-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fixes #13934: Fix motor(PWM protocol) spin while fc reset. #13937
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
Conversation
Do you want to test this code? You can flash it directly from Betaflight Configurator:
WARNING: It may be unstable. Use only for testing! |
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.
motorShutdown() already contains 1.5ms delay and is only point that calls motorDevice->vTable.shutdown(); . Increasing that delay may be enough. And it will prevent 'hidden' delay in virtual function,
@@ -35,6 +35,10 @@ | |||
|
|||
#include "pg/motor.h" | |||
|
|||
#ifndef PWM_SHUTDOWN_DELAY | |||
#define PWM_SHUTDOWN_DELAY 600 |
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.
This interval may be too short. You need at least 2ms for standard PWM pulse ( just for pulse to end, PWM interval may be 20ms)
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 tested a few more values: 200ms, 300ms, 490ms, none of these values can fix the problem. But value greater than or equal to 500ms can fix the issue.
The reason is after 500ms low level, ESC will in disarm state (ESC determine signal loss), in disarm state any signal interference will not considered valid. ESC need receive initialization sequence to re-enter arm state again.
So value greater or equal 500ms is OK.
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, i confused us and ms.
.6 s is noticeable time, but esc disarm is reasonable strategy. Maybe delay can be moved outside of this function (there is already delay after shutdown call). I'll look into it
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 think it's ok to move delay to motorShutdown, but's not all protocols require such a long delay.
The delay value can be set based on the ESC protocol currently in use.
We are on holiday now, I'll change and test it later.
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.
You can return minimum guard time before reboot from motorDevice->vTable.shutdown();
. Then extend delay in motorShutdown(); if necessary.
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.
Approved conditional on the changes requested by @ledvinap
…etaflight#13937) * Fix motor(PWM protocol) spin while fc reset. * move delay out to motorShutdown
Fixes #13934
Tested targets: SPEEDYBEEF405V4, SPEEDYBEEF405AIO and ARGUSF7AIO.
Before change, the PWM output may in high level while reset:

After add delay, PWM output keep zero duty before fc reset:
