Update MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN to use enum and have power o…#422
Update MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN to use enum and have power o…#422hamishwillee wants to merge 2 commits intoArduPilot:masterfrom
Conversation
| <entry value="4" name="CAMERA_TRACKING_STATUS_FLAGS_MTI"> | ||
| <wip/> | ||
| <description>Camera Moving Target Indicators (MTI) are active</description> | ||
| </entry> | ||
| <entry value="8" name="CAMERA_TRACKING_STATUS_FLAGS_COASTING"> | ||
| <wip/> | ||
| <description>Camera tracking target is obscured and is being predicted</description> | ||
| </entry> |
| <param index="3">WIP: 0: Do nothing for camera, 1: Reboot onboard camera, 2: Shutdown onboard camera, 3: Reboot onboard camera and keep it in the bootloader until upgraded</param> | ||
| <param index="4">WIP: 0: Do nothing for mount (e.g. gimbal), 1: Reboot mount, 2: Shutdown mount, 3: Reboot mount and keep it in the bootloader until upgraded</param> |
There was a problem hiding this comment.
Did @julianoes say changing this was OK?
So we have a problem here. ArduPilot uses these fields for things complete unrelated to the purposes given in these fields. Some very creative things, some very useful things and completely contrary to this standard :-(
I think we can make this safe and happy and be compliant if we can add some magic values to the new enumeration values - that might work well. And kind-of standardise our weird crap.
Why would you ask the autopilot to reboot a mavlink node rather than asking the node to do it itself, BTW? With the field values as they were you'd be working with a camera regardless of whether it was mavlink-aware or not, now you must be able to map from a mavlink component ID to a camera to make this work if it turns out to be the autopilot's job? I expect this is related to the "gimbal IDs 1-6 are not mavlink IDs" stuff we recently added?
There was a problem hiding this comment.
Thanks for the ping @peterbarker.
I think I agree that it doesn't make much sense to reboot gimbals, cameras, etc. from a command addressed at the autopilot. Given it's WIP, let's remove it. MAVSDK doesn't implement it, and neither does QGC from what I can tell.
There was a problem hiding this comment.
The idea is that you have something in control of the power or reboot status for some other component. That would normally be a companion computer, but there is no reason that it could not be an autopilot. So you send the command to the autopilot and it can control the thing connected. This makes more sense in some cases than others. For example, it makes sense that you would turn something on by controlling its power elsewhere, because when something is off it can't get your command to turn itself on.
I'm OK with enums to make this work. I know that we had the idea that you could overwrite parts of commands etc, in a dialect but IMO this should never ever happen. All params that are unused should be treated as reserved going forward.
|
Closing in favour of #476 which doesn't have unrelated changes in it |
4 participants
Sync
This is
FYI @peterbarker