989 namespace drivetrain configurations #994
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- There is one configuration for choosing the drivetrain. - However, each drivetrain has it's own lengthy set of configurations. - Some of these are shared among several drivetrains OR to avoid that they are prefixed and so identifiers become very long. - This commit moves each drivetrain's configuration into a python dict, effectively namespacing the configuration. - This allowed me to shorten the names of some configs since a prefix is not needed to disambiguate them. So some config names have changed - The bigger issue for users is that now, if they want to change any drivetrain config, they will need to uncomment the entire dictionary block, not just a single line of configuration. That should be obvious when looking at the code, but it could trick a newbie that does not understand python. I would say that is fine, because they will learn something important from it. - If you look at the resulting config.py you will see that it is much cleaner.
- so I updated the configuration to reflect this - removed the DRIVETRAIN_TYPE configuration, since it is not used - removed the other drivetrain configurations which are not used
|
This branch still needs have all the updated drivetrains tested. |
DocGarbanzo
reviewed
Mar 11, 2022
donkeycar/templates/cfg_complete.py
Outdated
| # and glide to a stop. | ||
| # - both pwms are full duty cycle (100% HIGH) to brake | ||
| # | ||
| HBRIDGE_PIN_FWD = 18, ß# provides forward duty cycle to motor |
There was a problem hiding this comment.
Here is an unintentional beta as well as some unintentional commas here and below.
There was a problem hiding this comment.
Thank you for catching that.
DocGarbanzo
pushed a commit
that referenced
this pull request
Apr 4, 2022
* Use python dict to namespace the various drivetrain configurations - There is one configuration for choosing the drivetrain. - However, each drivetrain has it's own lengthy set of configurations. - Some of these are shared among several drivetrains OR to avoid that they are prefixed and so identifiers become very long. - This commit moves each drivetrain's configuration into a python dict, effectively namespacing the configuration. - This allowed me to shorten the names of some configs since a prefix is not needed to disambiguate them. So some config names have changed - The bigger issue for users is that now, if they want to change any drivetrain config, they will need to uncomment the entire dictionary block, not just a single line of configuration. That should be obvious when looking at the code, but it could trick a newbie that does not understand python. I would say that is fine, because they will learn something important from it. - If you look at the resulting config.py you will see that it is much cleaner. * path_follow.py only support I2C_SERVO drivetrain - so I updated the configuration to reflect this - removed the DRIVETRAIN_TYPE configuration, since it is not used - removed the other drivetrain configurations which are not used * Change webui info log to debug log * Put drivetrain config in dictionary to address isssue #989 * remove spurious beta character and extra commas. * version="4.3.6" Co-authored-by: Ezward <Ezward@github.com> (cherry picked from commit cffec78)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses Issue #989
The PWM_STEERING_PIN configuration values are used in several DRIVETRAIN_TYPE configurations, each with a different default value. If you just set the DRIVETRAIN_TYPE = "PWM_STEERING_THROTTLE", but don't uncomment the PWM_STEERING_PIN value, then the default of a different drivetrain get's used.
So a workaround is to just uncomment PWM_STEERING_PIN in the PWM_STEERING_THROTTLE sections.
This fix changes the newer drivetrain configuration into python dictionaries so all configuration are namespaced.