Remove RC Throttle trim calibration value getting hard-set to Minimum value

[junwoo091400]

Descrption

A hotfix commit from 7 years ago was forcing RC transmitters to always have a trim value set to 'Min', effectively only allowing the [0, 1000] range.

While for legacy reasons & to bypass cases of spring-loaded transmitters, this may be ok, it is certainly not true regarding reversible thrust vehicles (e.g. Boat / Rovers), and it is confusing why only the throttle would be in the range [0, 1], and not [-1 ,1], as defined in the MANUAL_CONTROL MAVLink message definition.

Changes

Removed this hotfix, and allows setting the trim values automatically in the state machine (need to check logic once again)

Context

The discussion that discovered this flaw is here: PX4/PX4-Autopilot#15949 (comment)

We need to also clearly define the point about the definition of MANUAL_CONTROL's throttle (z) definition. As it doesn't really make sense to have throttle vector's orientation interfere with the RC transmitter's setpoint values (it should be agnostic to vehicle's behavior to be basic).

Also, PX4 will be able to handle this change using this logic (although some legacy parameter translations / user awarenesses would be needed): PX4/PX4-Autopilot#15949

And we need to make sure for ArduPilot this will work out as well. Judging from the way Steer / Throttle Manual override isn't differentiated in Rover code, I guess ArduPilot already uses full range (-1000 ~ +1000) definition for the internal RC throttle data, @peterbarker. Did you not have problem with QGC, regarding this point?

TODOs

• I am still unclear how the rcTrim value would then get set for throttle, need to check the state machine logic to be sure.

[DonLakeFlyer]

I don't quite understand what the original code that was removed was actually doing. That said as long as spring loaded throttle still works fine.

But this comment is concering: "although some legacy parameter translations / user awarenesses would be needed"

In general there is pretty much no user awareness on anything.

[MaEtUgR]

In short:

• PX4 currently has [0,1] throttle range for almost all use cases
• QGC hardcoded sets the trim value here such that the resulting throttle range is [0,1]
• I change the range in Scale manual control setpoint up/down stick -1 to 1 PX4/PX4-Autopilot#15949 because:
  • the axes should have consistent scaling
  • there's a lot of confusion around misuse of the current interface in -1 being out of range but still used for reverse thrust, movement
• These problems sadly carry over to (or come from?) MAVLink, see MANUAL_CONTROL: Redefine the use of z (throttle) to be independent to vehicle's thrust direction mavlink#1922
• I had to detect this hardcoded trim hotfix of the QGC RC calibration and correct the calibration to a consistent scale for all axes in PX4/PX4-Autopilot@d5e2573

We have to be careful with:

• backward compatibility, if the trim is set to the center value and it's used with an older PX4 version then the lower half of the stick input will get used for negative thrust which is almost nowhere in the code. So for most cases, the lower half would just get capped and be unusable without resetting the trim value.
• what the trim value gets even set to if it's not hardcoded to the minimum. Is it the center of the stick? When is that measured? Or is it just the position of the stick when the calibration starts (what I assume).

An alternative would be to replace the calibration with an autopilot-based one compared to the ground station doing all the logic based on the telemetry of the autopilot. That way compatibility on the exact ranges and values becomes less of an issue and e.g. also MAVSDK could easier trigger a calibration and just follow the steps. Just a suggestion.

That said as long as spring loaded throttle still works fine.

@DonLakeFlyer With compatible PX4 that's not an issue. From PX4 perspective it currently doesn't matter if the throttle is spring-loaded or not. It has its full range and in certain modes a spring-loaded throttle just makes more sense than in others. We could add this information in the future to possibly enhance UX in some cases but the pure control input is independent.

[DronecodeBot]

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/driving-backwards-with-a-boat-rover-in-manual-mode/25714/6

[mrpollo]

Should we get this in?

[DronecodeBot]

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-06-2023/32473/1