-
Notifications
You must be signed in to change notification settings - Fork 320
Update MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN to use enum and have power o… #422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Update MAV_CMD_PREFLIGHT_REBOOT_SHUTDOWN to use enum and have power o… #422
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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated
| <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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Sync
This is
FYI @peterbarker