Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

analog control fix #79

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

kozinalexey
Copy link

steer control from analog bug fix
add new style controll "tank mode" forward/reverse from one stick
#ifdef CONTROL_ADC_TANKSTYLE

steer control  from analog bug fix
add new style controll "tank mode"  forward/reverse from one stick
#ifdef CONTROL_ADC_TANKSTYLE
@Jana-Marie
Copy link
Collaborator

Hi,
Thank you for the contribution, but I don't think that will work.
Please move the line 205 to 208 in the main.c so that it is only calculated when the "Tank Style" is active.
Please also add a commented out define at a useful place in config.h.

@Jana-Marie
Copy link
Collaborator

And did you test the code? Is there a sample video?

@kozinalexey
Copy link
Author

kozinalexey commented Feb 12, 2019

hi, i make changes, code compiled and tested by my friend. i asked him video.
of course i missed to add help/description for #define CONTROL_ADC_TANKSTYLE

@kozinalexey
Copy link
Author

kozinalexey commented Feb 12, 2019

control forward-backward right-left from one joystick in "tank mode"
demo video
https://youtu.be/uBMAFsj5K08

@p-h-a-i-l
Copy link
Contributor

Could you rename "CONTROL_ADC_TANKSTYLE"? maybe something like adc, zero or bias would be more clear for me. For me "tank-style" is the default steering method already active.

@Jana-Marie
Copy link
Collaborator

Jana-Marie commented Feb 12, 2019

Likewise, this is not a fix, but an extension (of an ADC offset). Could you please name this accordingly and adjust the code so that the offset can be set in the config.h?

@kozinalexey
Copy link
Author

kozinalexey commented Feb 12, 2019

Likewise, this is not a fix, but an extension (of an ADC offset). Could you please name this accordingly and adjust the code so that the offset can be set in the config.h?
offset no have affect to reverse command

cmd2 = CLAMP(adc_buffer.l_rx2 - ADC2_MIN, 0, ADC2_MAX) / (ADC2_MAX / 1000.0f); // ADC2
clamp limited output cmd1 (or cmd2) value between 0 and ADC MAX value so cmd cant to be in range from -1000 to 1000

after
cmd = cmd -500; range 0-1000 changed to -500 ... +500
so
cmd = (cmd -500) * 2; range 0-1000 changed to -1000 ... +1000

Copy link
Contributor

@p-h-a-i-l p-h-a-i-l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal how to implement.
Need to do changes to config.h

// ###### CONTROL VIA TWO POTENTIOMETERS ######
// ADC-calibration to cover the full poti-range: connect potis to left sensor board cable (0 to 3.3V) (do NOT use the red 15V wire in the cable!). see <How to calibrate>. turn the potis to minimum position, write value 1 to ADC1_MIN and value 2 to ADC2_MIN. turn to maximum position and repeat it for ADC?_MAX. make, flash and test it.
#define CONTROL_ADC               // use ADC as input. disable DEBUG_SERIAL_USART2!
#define ADC1_MIN         0        // min ADC1-value while poti at minimum-position (0 - 4095)
#define ADC1_ZERO     1500        // ADC1-value while poti at zero-position (0 - 4095)
#define ADC1_MAX      4095        // max ADC1-value while poti at maximum-position (0 - 4095)
#define ADC1_MULT_NEG  500.0f     // Use 1000.0f to calibrate form MIN to MAX
#define ADC1_MULT_POS 1500.0f     // Use 1000.0f to calibrate form MIN to MAX

#define ADC2_MIN         0        // min ADC2-value while poti at minimum-position (0 - 4095)
#define ADC2_ZERO     2000        // ADC2-value while poti at zero-position (0 - 4095)
#define ADC2_MAX      4095        // max ADC2-value while poti at maximum-position (0 - 4095)
#define ADC2_MULT_NEG  300.0f     // Use 1000.0f to calibrate form MIN to MAX
#define ADC2_MULT_POS  300.0f     // Use 1000.0f to calibrate form MIN to MAX

(Taken from my own pull request, small subset :) )

Src/main.c Outdated Show resolved Hide resolved
Src/main.c Outdated Show resolved Hide resolved
Src/main.c Outdated Show resolved Hide resolved
Src/main.c Outdated Show resolved Hide resolved
Src/main.c Outdated Show resolved Hide resolved
Src/main.c Outdated Show resolved Hide resolved
Src/main.c Outdated Show resolved Hide resolved
@Jana-Marie
Copy link
Collaborator

@kozinalexey No this is not a fix, the ADC is not supposed to have negative values and the code is not a "out-of-the-box" code for everything, it is a codebase. Most parts in the config.h should be considered as "example" adding more will obfuscate the file even more. Therefore I don't want to merge your pull request, at least not by myself. I will get in touch with @NiklasFauth.

@p-h-a-i-l Thank you for your contributions and I'm sorry that your own pull request is not merged yet, I will review it as well later.

p-h-a-i-l and others added 7 commits February 13, 2019 12:49
Co-Authored-By: kozinalexey <[email protected]>
Co-Authored-By: kozinalexey <[email protected]>
Co-Authored-By: kozinalexey <[email protected]>
Co-Authored-By: kozinalexey <[email protected]>
Co-Authored-By: kozinalexey <[email protected]>
Co-Authored-By: kozinalexey <[email protected]>
Co-Authored-By: kozinalexey <[email protected]>
@p-h-a-i-l
Copy link
Contributor

@Jan--Henrik no worries, I know that my pull request is quite large and time is always limited.
You also need to decide what your strategy is for this repo: Code to get stuff running as a foundation for others or a constantly evolving codebase with nice but more complex features.

@kozinalexey you might want to rebase my code change proposals, doesn't look nice to have 7 tiny commits without commit message

@kozinalexey
Copy link
Author

kozinalexey commented Feb 13, 2019

@p-h-a-i-l im sorry. i did non clone the repository localy, im only using github visual tools for quick fix, githib visual tools have not rebase function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants