Fix PWM/Oneshot calibration #21711

MaEtUgR · 2023-06-09T16:34:44Z

Solved Problem

When testing #21513 I found (again) that PWM calibration does not do what I expect and leads to it almost being impossible to configure PWM outputs consistently once a calibration was performed. That's why I opened #21634 and want to fix that first.

Fixes #21634

Solution

Make PWM calibration range consistent 1000-2000us (which maps to 125-250us for Oneshot) independent of minimum and maximum PWM configuration. These parameters represent the PWM value where the motors start spinning and stops going faster and producing more thrust. These definitions are separate from the calibration and do not match the same values for any ESC I've ever used. This was also discussed for example here: Send PWM_MIN value after a PWM_MAX value while calibrating ESCs #11663 (comment)
Make sure the motor output is always released and reverts to normal operation after a calibration even if it fails!
Change the timing of the ESC calibration to mostly give more time for the calibration. It's worth to give the ESC enough time for the steps compared to having some ESCs fail or calibrate to wrong values. In my experience the ESC's timeouts are pretty long because the calibration process is originally a manual one.
I added a safety check where an increase in measured current upon applying the high PWM value leats to an immediate abortion. I did this because on my test setup, the battery detection doesn't work reliably and this is an additional layer of preventing a screw-up.

Changelog Entry

For release notes:

Bugfix: Fix PWM/Oneshot calibration to consistent range
Documentation: Need to clarify https://docs.px4.io/main/en/advanced_config/esc_calibration.html#esc-calibration

Alternatives

I want to also allow PWM calibration when the battery detection is not possible e.g. because the user has no power module or it needs to stay connected to power the autopilot. I suggest we take this PR without additional functionality together with #21513 first for the 1.14 release and deal with the manual calibration with the corresponding UI as a next step. The problem right now is the tight integration with the QGC UI being very limited. The first step would be to at least have a backward-compatible MAVLink shell way of doing that.

EDIT: I allowed to calibrate without battery detection because of the following reasons:

There are many setups where at the time of ESC calibration the user has either no power module or it needs to stay connected to power the autopilot.
Without this the ESC calibration aborts after a timeout if battery detection doesn't work. The problem is if the user still connects the battery as he gets instructed to and the calibration aborts then the ESCs are in calibration mode and after abortion just calibrate to the wrong value.
There's no additional safety by aborting the calibration if the battery cannot be detected during the timeout because a pixhawk board without power module will report a battery status from the ADC driverand in it that no battery is connected which is the best it can do. In this situation the motors will still spin if the ESCs are powered.

So the only disadvantage is that the UI does not show when the user did actually not plug the battery which is much less bad than not allowing a calibration at all and aborting in the worst moment which makes the ESC calibrate to the wrong value. So I suggest this as the in-between step and fixing the UI next.

Test coverage

Hardware testing of PWM and Oneshot on Holybro QAV250 with pixhawk 4 mini. I get extremely consistent calibrations with well better tolerances than what's needed for useful vehicle operation.

I'm testing with more ESCs now.

to make them easy to search for and check the arguments.

- Change timings for a more reliable calibration. - Make sure there's an error message when battery measurement is not available also when it gets disabled after system boot in the power just above the calibration button. - Safety check if measured electrical current goes up after issuing the high calibration value for the case the user did not unplug power and the detection either fails or is not enforced.

Some ESCs e.g. Gaui enter the menu relatively quickly if the signal is high for too long. To solve that it's kept high shorter. Also all tested ESCs detect the low signal within a shorter time so no need to wait longer.

Before this the ESC calibration aborts if battery detection doesn't work. The problem is if the user still connects the battery as he gets instructed and the calibration aborts then the ESCs are in calibration mode and after abortion calibrate to the wrong value. Also I realized there's no additional safety by aborting the calibration if the battery cannot be detected during the timeout because a pixhawk board without power module will report a battery status from the ADC driverand in it that no battery is connected which is the best it can do. In this situation the motors will still spin if the ESCs are powered.

MaEtUgR · 2023-06-13T14:05:10Z

I did testing on 5 different PWM ESCs and adjusted the timing to work with all of them. Also I allowed ESC calibration without battery detection see my edit in the "Alternatives" section above.

DronecodeBot · 2023-06-13T15:17:17Z

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-maintainers-call-june-13-2023/32587/1

src/modules/commander/esc_calibration.cpp

bkueng · 2023-06-15T09:28:27Z

src/modules/commander/esc_calibration.cpp

+		battery_status_sub.update();
+
+		if ((now - timeout_start) < 1_s && (battery_status_sub.get().current_a > current_before_calibration + 1.f)) {
+			// Safety termination, current rises immediately, user didn't unplug power before


Did you test this case?

Yes I did with multiple setups. If the current is picked up by the power module it reacts very fast, aborts and the motor(s) don't have time to spin up. It also triggered with a single small motor and no propeller attached. I made it only trigger in the first second because there can be a 1A current spike measured during a normal calibration later on which makes finding a safe threshold much harder.

It can't provide a guarantee because the user could be powering the ESC independently of the power module and then no current is measured. But in my eyes it's a worthwhile safety layer because it would have caught point 1. in #21634.

Here's a plot from one of my tests. The current spike leads to an immediate abortion and basically just a stutter of the motors:

I should probably change the timeout check for this one too then ⚙️

Co-authored-by: Beat Küng <beat-kueng@gmx.net>

MaEtUgR · 2023-06-15T14:35:26Z

Thanks a lot for the review @bkueng ! I think I don't completely understand the timestamp wrapping. Let me know if it's better now with the last commit.

bkueng

I think I don't completely understand the timestamp wrapping.

It's only a problem in certain cases, because unsigned int is used: if you have a-b and b>a, it wraps. Specifically it happens when taking the current time, then doing an orb copy which was published after the current time. That can't happen in your case, but there's various places where it can, and so it's better to be consistent everywhere.

For the 1.14 release because we already changed the actuator configuration and users will need to go through their setups I also corrected the PWM calibration and the default PWM values. See PX4/PX4-Autopilot#21711 and PX4/PX4-Autopilot#21513 This is the documentation change with it. And needs to be referenced by the release notes.

MaEtUgR · 2023-06-27T18:26:10Z

Here's the documentation change for this together with the new PWM defaults (#21513): PX4/PX4-user_guide#2599

For the 1.14 release because we already changed the actuator configuration and users will need to go through their setups I also corrected the PWM calibration and the default PWM values. See PX4/PX4-Autopilot#21711 and PX4/PX4-Autopilot#21513 This is the documentation change with it. And needs to be referenced by the release notes.

MaEtUgR requested a review from dagar June 9, 2023 16:34

MaEtUgR self-assigned this Jun 9, 2023

MaEtUgR mentioned this pull request Jun 9, 2023

[WIP] PWM calibration problem #21634

Closed

MaEtUgR added 6 commits June 13, 2023 15:51

Unify MixingOutput constructor calls

6c1ec9c

to make them easy to search for and check the arguments.

actuator_test: condition order refactoring

42c6dd4

mixer_module: consistent PWM/oneshot calibration range

ae31ffb

esc_calibration: adjust timing to work with all tested ESCs

9f5efd8

Some ESCs e.g. Gaui enter the menu relatively quickly if the signal is high for too long. To solve that it's kept high shorter. Also all tested ESCs detect the low signal within a shorter time so no need to wait longer.

MaEtUgR force-pushed the maetugr/pwm-calibration branch from 69ea5be to e83363f Compare June 13, 2023 13:51

bkueng previously approved these changes Jun 15, 2023

View reviewed changes

MaEtUgR dismissed bkueng’s stale review via 67bc737 June 15, 2023 14:10

esc_calibration: handle timeout wraps better

24c9ee0

Co-authored-by: Beat Küng <beat-kueng@gmx.net>

MaEtUgR force-pushed the maetugr/pwm-calibration branch from 67bc737 to 24c9ee0 Compare June 15, 2023 14:33

bkueng approved these changes Jun 16, 2023

View reviewed changes

MaEtUgR merged commit 053d302 into main Jun 16, 2023

MaEtUgR deleted the maetugr/pwm-calibration branch June 16, 2023 10:05

This was referenced Jun 16, 2023

[1.14] Fix PWM/Oneshot calibration #21726

Merged

Default motor PWM configuration #21513

Merged

MaEtUgR mentioned this pull request Jun 27, 2023

esc_calibration: document latest functionality PX4/PX4-user_guide#2599

Merged

2 tasks

This was referenced Aug 7, 2023

Send PWM_MIN value after a PWM_MAX value while calibrating ESCs #11663

Closed

ESC calibration not calibrating low end #12759

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PWM/Oneshot calibration #21711

Fix PWM/Oneshot calibration #21711

MaEtUgR commented Jun 9, 2023 •

edited

Loading

MaEtUgR commented Jun 13, 2023

DronecodeBot commented Jun 13, 2023

bkueng Jun 15, 2023

MaEtUgR Jun 15, 2023

MaEtUgR commented Jun 15, 2023

bkueng left a comment

MaEtUgR commented Jun 27, 2023 •

edited

Loading

Fix PWM/Oneshot calibration #21711

Fix PWM/Oneshot calibration #21711

Conversation

MaEtUgR commented Jun 9, 2023 • edited Loading

Solved Problem

Solution

Changelog Entry

Alternatives

Test coverage

MaEtUgR commented Jun 13, 2023

DronecodeBot commented Jun 13, 2023

bkueng Jun 15, 2023

Choose a reason for hiding this comment

MaEtUgR Jun 15, 2023

Choose a reason for hiding this comment

MaEtUgR commented Jun 15, 2023

bkueng left a comment

Choose a reason for hiding this comment

MaEtUgR commented Jun 27, 2023 • edited Loading

MaEtUgR commented Jun 9, 2023 •

edited

Loading

MaEtUgR commented Jun 27, 2023 •

edited

Loading