MANUAL_CONTROL: Redefine the use of z (throttle) to be independent to vehicle's thrust direction
#1922
Conversation
Previously, the `z` component definition relied on having a knowledge of which direction the thrust would result in, depending on it's value. However this is a bad practice as the ManaulControl message itself should be agnostic to a vehicle type (either reverse-thrustable ones like Rover, or ones with only positive thrust like conventional quadcopters). With this new definition, the MANUAL_CONTROL message will not be vehicle-specific and truly represent the Joystick / RC Transmitter's throttle position
There was a problem hiding this comment.
Most importantly remove:
Positive values are positive thrust, negative values are negative thrust.
That part causes ambiguity and confusion because what happens if the vehicle does not have any negative thrust? In practice, this message was designed and is broadly used to report joystick positions over MAVLink. But with this definition, it instead results in the MAVLink-enabled remotes needing to understand the thrust situation of the vehicle just to be able to report the throttle stick or slider position.
It's sad that the original easier to understand definition was abandoned in https://github.com/mavlink/mavlink/pull/64/files
but
https://github.com/mavlink/mavlink/pull/464/files
really started the confusion in my eyes.
I'd vote for a very clear definition like this:
https://github.com/PX4/PX4-Autopilot/blob/7977ba3217422b042a6848ef529d9ea61ba0b686/msg/ManualControlSetpoint.msg#L24-L27
but for this message, I think the ship has sailed and a new one would need to be added for backwards compatibility.
| @@ -5448,7 +5448,7 @@ | |||
| <field type="uint8_t" name="target">The system to be controlled.</field> | |||
| <field type="int16_t" name="x" invalid="INT16_MAX">X-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to forward(1000)-backward(-1000) movement on a joystick and the pitch of a vehicle.</field> | |||
| <field type="int16_t" name="y" invalid="INT16_MAX">Y-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to left(-1000)-right(1000) movement on a joystick and the roll of a vehicle.</field> | |||
| <field type="int16_t" name="z" invalid="INT16_MAX">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.</field> | |||
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to throttle high (1000) - throttle low (-1000) on a joystick. Centered throttle should result in value of 0 (center).</field> | |||
There was a problem hiding this comment.
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to throttle high (1000) - throttle low (-1000) on a joystick. Centered throttle should result in value of 0 (center).</field> | |
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Joystick or slider fully down corresponds to -1000, fully up to 1000, the center position to 0.</field> |
There was a problem hiding this comment.
Not sure about this. Before we were saying its another slider, now we're enforcing that the alignment of that slider is "up/down". I think the desired change is just to remove the emphasis on the positive/negative thrust. Something like.
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to throttle high (1000) - throttle low (-1000) on a joystick. Centered throttle should result in value of 0 (center).</field> | |
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally the axis is mapped to vehicle thrust. Vehicles that support only positive thrust map the range from maximum thrust (1000) to mininimum (-1000). Vehicles that support negative thrusts map the range from maximum positive thrust (1000) to maximum negative thrust (-1000), with 0 (centred) implying no thrust).</field> |
There was a problem hiding this comment.
Before we were saying its another slider
Conceptually it should represent an analog input axis be it slider, stick axis or whatever else someone would like to map. In most cases, it will be a stick axis which might be worth mentioning but certainly not enforcing.
now we're enforcing that the alignment of that slider is "up/down"
My wording is not general enough. I do not want to enforce that. The axis just needs to have a minimum and maximum if that's up, down, sideways or diagonal is up to the user. Again in most cases, I'm pretty sure it's up and down.
Generally the axis is mapped to vehicle thrust.
That is exactly the part that is in my eyes only half the story. Manual up, forward, gas is the first thing that comes to mind but often it's mapped to steer vertical or even horizontal speed. I'd try to avoid mentioning an exact use because even though it's true in certain scenarios it can be very confusing in others.
Vehicles that support only positive thrust map the range from maximum thrust (1000) to mininimum (-1000). Vehicles that support negative thrusts map the range from maximum positive thrust (1000) to maximum negative thrust (-1000), with 0 (centred) implying no thrust).
Very good wording. Whatever the thrust, speed or other desired quantity range is, should be mapped to the full available input range -1000 to 1000.
There was a problem hiding this comment.
Whatever the thrust, speed or other desired quantity range is, should be mapped to the full available input range -1000 to 1000.
@MaEtUgR No, that's good wording ^^^ :-)
Perhaps we should make it that simple?
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. Generally corresponds to throttle high (1000) - throttle low (-1000) on a joystick. Centered throttle should result in value of 0 (center).</field> | |
| <field type="int16_t" name="z" invalid="INT16_MAX">Z-axis, normalized to the range [-1000,1000]. A value of INT16_MAX indicates that this axis is invalid. The value may be mapped to thrust, speed, throttle, or any other property. The range of possible property values should be mapped across the full movement range of the axis.</field> |
Similar changes could be made to the other axes
|
As the wording is now "Positive values are positive thrust, negative values are negative thrust.". That implies that on a vehicle with only positive thrust you have to have a deadzone from the low position of the axis range to the centre. I bet that no one actually does that. This is conceptually a breaking change from a MAVLink perspective but I think it makes sense, if we can get the wording right. I will take to the dev call this evening.
|
Oddly, I'm looking at doing the opposite in (non-Sub) ArduPilot (ArduPilot/ardupilot#22259) - actually taking action based on the buttons field in Turns out that MissionPlanner actually already does what you're suggesting here - makes mavlink calls rather than passing through the button bits (a small impediment to my little project). |
Sounds daft unless the functions you want don't have corresponding mavlink messages, or can't do so for some reason. I understand that in Canberra they call you "Mad Peter" :-) Currently
The send-a-mavlink-message approach seems much smarter and more flexible.
|
|
On Tue, 29 Nov 2022, Hamish Willee wrote:
The send-a-mavlink-message approach seems much smarter and more flexible.
* The GCS does not need to know anything about the flight stack than it already knows - what commands are supported. (and even those can be in many cases checked without side effects). It doesn't need to
know what parameters the flight stack uses to map button presses or what functions are called.
* The flight stack doesn't have to have special code paths to process instructions it is already processing through mavlink.
* The manual control message doesn't have to cope with the possiblity of joysticks with more buttons than we expose.
That's all true, and we did note the first two when we discussed.
In this case one of our partners wanted to have fence enable/disable on a
button. MissionPlanner didn't permit binding that to a button, and would
need patching and updating to support the functionality. Having the
function instantly available was what I was after - as soon as it was
available in ArduPilot it could be configured.
I would *like* MissionPlanner to use our DO_AUX_FUNC list (I was mistaken
in my initial assesment there; it has specialised code for each function
it offers). Addition of an arbitrary-number-combo-box interface into the
relevant configuration page might fix things.
Problem with that last is that now *every* GCS has to have this interface.
The list of valid functions will be different autopilot-to-autopilot....
and while ArduPilot has a convenient list of functions that can be
activated by MAVLink (I might actually generate a aux_func.xml at some
stage), other autopilots may not...
There are advantages both ways for storing the configuration in the GCS
and on the vehicle; one allows you to switch laptops and know the config
will be the same. The other allows the interface to change between
laptops based on input devices....
|
|
@peterbarker There was an orthogonal discussion here #1848 which looks at the use case of a MAVLink joystick. This is a joystick that does not need to be connected to a GCS when used. The idea was that a joystick would have a component metadata mechanism to configure its buttons a lot like the mavlink camera API - i.e. you'd query the joystick for its axes and buttons and get metadata back on what is supported. The buttons would have set options that map to parameter values - i.e. button 1 might have param_button_1 on the joystick with options "enable fence", "kill switch". My point is that the sending mavlink command mechanism is more flexible here too. Component information helps with the problem of knowing what the joystick can do, and means that the autopilot doesn't need to have any extra config to work. It's just an idea at this time. |
|
Anyway, appreciate that you're probably making a pragmatic decision based on what mission planner does now (and PX4). Nothing is going to change in any sort of short timeframe so you can do what you like. But if you go with sending mavlink commands that will work whatever is done in future with the MANUAL_CONTROL message. |
Description
Previously, the
zcomponent definition relied on having a knowledge of which direction the thrust would result in, depending on its value, which is tied to the vehicle's setup (e.g. Rover, which can take in reverse-throttle commands vs. conventional quadcopter with only positive throttle).This is a bad practice because:
MANUAL_CONTROLmessage for each different type of vehicle to use the 'correct' range ofz. So having a GCS with a Joystick input will have to know which range of throttle would result in positive/negative thrust for each connected vehicle, which doesn't make sense.With this new definition, the MANUAL_CONTROL message will not be vehicle-specific and truly represent the Joystick / RC Transmitter's throttle position.
Alternatives
We could keep the convention as is 馃
Discussion Points
Context
This issue was discussed in a PX4 PR trying to standardize the range for throttle internally: PX4/PX4-Autopilot#15949 (comment)