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
Adding DSHOT1200 and 900 #2033
Adding DSHOT1200 and 900 #2033
Conversation
@@ -30,11 +30,27 @@ typedef enum { | |||
PWM_TYPE_DSHOT600, | |||
PWM_TYPE_DSHOT300, | |||
PWM_TYPE_DSHOT150, | |||
PWM_TYPE_DSHOT900, | |||
PWM_TYPE_DSHOT1200, | |||
PWM_TYPE_MAX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small point, but can we have these in ascending order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, and implemented it at first - but then it requires a configurator change at the same time :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should bear the pain now, rather than forever have to live with an out of order list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can toss it into 1.86 before release if you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me the existing list with DShot600 first always looked out of order.
We can simple remap them for older versions in the configurator right when we receive / send them, and all will be fine
@@ -334,7 +334,7 @@ static const char * const lookupTableSuperExpoYaw[] = { | |||
static const char * const lookupTablePwmProtocol[] = { | |||
"OFF", "ONESHOT125", "ONESHOT42", "MULTISHOT", "BRUSHED", | |||
#ifdef USE_DSHOT | |||
"DSHOT600", "DSHOT300", "DSHOT150" | |||
"DSHOT600", "DSHOT300", "DSHOT150", "DSHOT900", "DSHOT1200", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also these in ascending order to match changed enums.
Wow which ESC support this? |
KISS24 :) |
@@ -27,6 +27,8 @@ typedef enum { | |||
PWM_TYPE_ONESHOT42, | |||
PWM_TYPE_MULTISHOT, | |||
PWM_TYPE_BRUSHED, | |||
PWM_TYPE_DSHOT1200, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to be a pain, but can we have these in ascending order? Ie
typedef enum {
PWM_TYPE_STANDARD = 0,
PWM_TYPE_ONESHOT125,
PWM_TYPE_ONESHOT42,
PWM_TYPE_MULTISHOT,
PWM_TYPE_BRUSHED,
PWM_TYPE_DSHOT150,
PWM_TYPE_DSHOT300,
PWM_TYPE_DSHOT600,
PWM_TYPE_DSHOT900,
PWM_TYPE_DSHOT1200,
PWM_TYPE_MAX
} motorPwmProtocolTypes_e;
that follows the convention as per the "shot" protocols which has slowest first. It also means we could add a DSHOT1500 in future while preserving the order (unlikely, I know).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@martinbudden done :) Might as well get it how we want if it is dependent on a configurator change
Not yet tested on actual hardware