Skip to content

Adds dshot reverse commands support #157

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Alissonsz
Copy link
Contributor

@Alissonsz Alissonsz commented Feb 15, 2025

❗ WORK IN PROGRESS ❗

@rtlopez
Copy link
Owner

rtlopez commented Feb 15, 2025

yesterday I disovered serious bug, that cause heap corruption, unfortunately it is in master branch. Within few days I should be able to provide fix. In the meantime you might try to rebase to release-0.2 branch which is stable and well tested.

@Alissonsz
Copy link
Contributor Author

Ok, thanks for the reply!!

@rtlopez
Copy link
Owner

rtlopez commented Feb 15, 2025

I've rolledback bad changes and pushed fix to master. Please check if it still occurs. If you next time get this error, uncoment line monitor_filters = esp32_exception_decoder in platformio.ini file, then connect with serial monitor, and try some commands like reboot or save. You should get the backtrace.

@Alissonsz Alissonsz force-pushed the dshot-reverse branch 5 times, most recently from 5fb7ff3 to 72f5db8 Compare February 17, 2025 22:12
@@ -114,6 +114,22 @@ int IRAM_ATTR EscDriverEsp32::write(size_t channel, int pulse)
return 1;
}

int EscDriverEsp32::reverseMotor(size_t channel, bool reverse)
Copy link
Contributor Author

@Alissonsz Alissonsz Feb 17, 2025

Choose a reason for hiding this comment

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

@rtlopez How would you call this function from the CLI? I tried some ways but everything felt too ugly 😢

Copy link
Owner

Choose a reason for hiding this comment

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

_model.state.mixer.escMotor->reverseMotor(), but check first if pointer is initialized.

@Alissonsz Alissonsz force-pushed the dshot-reverse branch 2 times, most recently from 1d95f0f to 1c284d2 Compare February 17, 2025 22:16
Copy link
Owner

@rtlopez rtlopez left a comment

Choose a reason for hiding this comment

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

I added few more comments in advance, to save your time :)

@@ -114,6 +114,22 @@ int IRAM_ATTR EscDriverEsp32::write(size_t channel, int pulse)
return 1;
}

int EscDriverEsp32::reverseMotor(size_t channel, bool reverse)
Copy link
Owner

Choose a reason for hiding this comment

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

_model.state.mixer.escMotor->reverseMotor(), but check first if pointer is initialized.

#include "EscDriver.h"

// The official DShot Commands
typedef enum dshot_cmd_e
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer camel case notation for type names. Also typedef is not required here. Just enum DshotCommand

@@ -99,7 +102,8 @@ class EscDriverEsp32: public EscDriverBase
void transmitAll();
void readTelemetry();
void writeAnalogCommand(uint32_t channel, int32_t pulse);
void writeDshotCommand(uint32_t channel, int32_t pulse);
void writeDshotThrottleCommand(uint32_t channel, int32_t pulse);
Copy link
Owner

Choose a reason for hiding this comment

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

don't rename this method name, this cause changes in many places that you haven't explored yet, like unittests or differrent platforms. Just add new method with unique name like writeDshotCustomCommand

@Alissonsz Alissonsz force-pushed the dshot-reverse branch 2 times, most recently from 8c271e9 to 644f4e8 Compare February 21, 2025 20:43
@Alissonsz
Copy link
Contributor Author

@rtlopez could you test it in one ESC that you ideally knows that support the dshot revert command? Maybe you also can spot something I'm missing for that to work. Technically I should:

  1. set throttle to 0.
  2. send the command some times with a small delay.
  3. save the settings.

but its not working and tried a lot of stuff but there's no comprehensive way to debug what the ESC is doing 😢

@Alissonsz Alissonsz closed this Feb 25, 2025
@Alissonsz
Copy link
Contributor Author

Alissonsz commented Feb 25, 2025

Closing it since I couldn't make it work and lost hope.
Also it's very easy to revert the motors using esc-configurator website.

@rtlopez
Copy link
Owner

rtlopez commented Feb 25, 2025

This code is quite promising, It can also be used to control leds on esc's or to implement beeper. I'll reopen it to not forget and continue some day, thanks.

@rtlopez rtlopez reopened this Feb 25, 2025
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.

2 participants