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

Motor output ramp consistent for higher than minimum initial motor command #21690

Merged
merged 2 commits into from
Jun 10, 2023

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jun 6, 2023

Solved Problem

When integrating a product with PWM ESCs I found that the current PWM ramp is suboptimal:

Excpected: When the mixed motor command is exactly the minimum during the ramp time it ramps from the disarmed PWM value to the minimum PWM value.

Unexpected: When the mixed motor command is even just slightly higher than the minimum the ramp jumps up to a higher value than the disarmed PWM value and then ramps from there to the commanded throttle scaled between minimum PWM and maximum PWM. Additionally the first time a vehicle is armed after boot there is a 50 millisecond delay until the ramp actually starts.
ramp before

Solution

  • Replace the complicated ramp calculation with a simpler one that fades the current motor command in with a scale ramping from 0 to 1.
  • Remove the ancient init state of the ramp state machine that was introduced in cc45283#diff-53cf9e4cbf5fe3dc0c623b0be8c562859b714628e061dcfb548c4690311b18d3R186-R195 presumably as a workaround for some specific timing problem in the very very early days. And possibly just produced confusion later on at least for me.

As a result the ramp always starts immediately after arming from the disarmed PWM value and ends at the commanded throttle scaled between minimum PWM and maximum PWM. If the commanded throttle changes during the ramp then the scale and hence also end value of the ramp changes.
ramp after

Changelog Entry

For release notes:

Bugfix: Make arming motor output ramp consistent for higher than minimum initial motor command

Alternatives

I'm thinking about removing the output ramp on arming completely but this will come as a separate pr and requires more testing.

Test coverage

  • Check if the disarmed value can ever be not set. I saw that we rely on the disarmed value everywhere in the mixer_module and also the PWM drivers set it correctly even if there's just a default disarmed value for all channels.
  • Check functionality when the disarmed value is higher than the minimum value. It then ramps down instead of up but still works.
  • Check functionality with a reversed output. It ramps from the disarmed value (default 900) up to the reversed throttle value (close to 2000). If the disarmed value is customized to e.g. 2100 then it ramps down so even if I've never seen this case that works. My guess from reading the previous implementation is that this never worked despite extra case handling.
  • I bench-tested this solution on two vehicles with PWM ESCs.

Context

Since #20031 the throttle even with no roll and pitch torques is not necessarily zero when arming but directly starts at the configured minimum throttle in multicopter modes that have manual throttle control. Also #21010 can add additional non-zero motor commands while arming.

Before:
When the mixed throttle for the motor was exactly zero the ramp went
from the disarmed PWM value to the minimum PWM value.

When the throttle was even just slightly different from zero the ramp
made a jump up to the commanded throttle scaled between
disarmed PWM and maximum PWM, then ramped between
disarmed PWM and minimum PWM and at the end jumped up again to
the commanded throttle scaled between minimum PWM and maximum PWM.

After:
The ramp goes from disarmed PWM value to the the
commanded throttle scaled between minimum PWM and maximum PWM.
If the commanded throttle changes during the ramp then the scale and
hence also end value of the ramp changes.
That state only delayed the first arming by 50 miliseconds.
Was presumably a workaround for some issue very very early on.
See cc45283
@MaEtUgR MaEtUgR added the bug label Jun 6, 2023
@MaEtUgR MaEtUgR self-assigned this Jun 6, 2023
@MaEtUgR MaEtUgR changed the title Maetugr/pwm ramp fix Motor output ramp consistent for higher than minimum initial motor command Jun 6, 2023
@MaEtUgR MaEtUgR requested review from bkueng, tstastny and dagar June 8, 2023 13:41
@dagar dagar merged commit 94d2140 into main Jun 10, 2023
82 of 85 checks passed
@dagar dagar deleted the maetugr/pwm-ramp-fix branch June 10, 2023 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants