fix: change MANUAL_CONTROL throttle range from [-1000, 1000] to [0, 1000]

[pbreuer-ff]

manual_control_input uORB topic is populated from MANUAL_CONTROL (MAVLink) while expecting throttle to be in range [0, 1000]. Internally, the uORB manual_control_input/setpoint topics have throttle in [-1, 1]. Currently, the "MAVLink output side" produces MANUAL_CONTROL messages with throttle in [-1000, 1000], so there is an inconsistency between input and output handling. For context see here

@MaEtUgR
@dagar
@jeremyzff

[jeremyzff]

This brings the output such that the definition matches what PX4 expects as input. It also puts this back to what it was before.

We had a dangerous situation with an integrator because of this change in definition of the control ranges.

[dagar]

The "right" way to do this mapping might be debatable, but regardless we need to be consistent between MANUAL_CONTROL input and output. https://github.com/PX4/PX4-Autopilot/blob/9d9e088d02c8a20f286e60d6d2806259d5603c2a/src/modules/mavlink/mavlink_receiver.cpp#L2096-L2097

[MaEtUgR]

@pbreuer-ff @jeremyzff Thanks for making this pr 🙏 I refactored the MANUAL_CONTROL input processing to make it the exact matching inverse including the exemplary comments.

Apologies for the confusion around the MANUAL_CONTROL.z MAVLink field and the manual_control_setpoint.throttle uORB field. My mistake (https://github.com/PX4/PX4-Autopilot/pull/15949/files#r1025243016) stemmed from assuming the message output was purely telemetry of the internal state with a range of [-1, 1], while the input controlled vehicles via ground stations like QGroundControl, historically using [0, 1000] for most vehicle types. This mismatch broke things when forwarding messages through PX4.

The real issue lies in the ambiguous MAVLink MANUAL_CONTROL.z field description:
https://mavlink.io/en/messages/common.html#MANUAL_CONTROL

• "maximum being 1000 and minimum being -1000 on a joystick" is clear and aligns with what I used for output.
• "Positive values are positive thrust, negative values are negative thrust." is vague and doesn’t suit joystick positions. It wrongly implies the sender knows the joystick’s function and adjusts the range accordingly, contradicting the earlier definition.

I believe things diverged in 2015 with these commits:
mavlink/mavlink@1567c0a
mavlink/mavlink@5e6dc47
and the corresponding QGroundcontrol implementation to send [0, 1000] for multicopters and [-1000, 1000] for certain other vehicle types.

Currently, PX4 rejects negative values if the full [-1000, 1000] range is used for the input, while previous to #23649 they spilled into internal states as out-of-bounds values.

At this point we should for sure stay consistent between input and output and hence merge this pr.

Introducing a clearer replacement for MANUAL_CONTROL might be the best long-term solution, avoiding overloaded meanings and easing compatibility.

[hamishwillee]

@MaEtUgR FWIW Mavlink defin for Z is:

Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to a separate slider movement with maximum being 1000 and minimum being -1000 on a joystick and the thrust of a vehicle. Positive values are positive thrust, negative values are negative thrust.

If you think it is useful we can omit that last sentence?

Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to a separate slider movement with maximum being 1000 and minimum being -1000 on a joystick and the thrust of a vehicle.Positive values are positive thrust, negative values are negative thrust.

Also open to a new design - we'd quite like to separate the control of sliders and of buttons.

[MaEtUgR]

@hamishwillee Thanks for chiming in.

If you think it is useful we can omit that last sentence?

I think it would be BUT QGroundcontrol sends it with this strange positive negative thrust assumption per vehicle type. And I'm not sure if I can just change that without potentially unsafe repercussions.

Also open to a new design - we'd quite like to separate the control of sliders and of buttons.

Interesting. I mean buttons would not need to be streamed so change based vs. continuous input stream might make sense to separate. I agree with @jeremyzff on the wish to be able to assign inputs to effects on the vehicle depending on flight situation. There are different varieties of that so we certainly still need to discuss.

[jeremyzff]

this may be a problem either way, but one issue with not streaming the buttons is you can miss events easily if a packet is dropped over the air.

I've been whispering in @MaEtUgR 's ear about ways to commonize the input for controllers between RC in and MANUAL_CONTROL in. It's a struggle for us who need to use both types to do so seamlessly because its processed so differently.