search

mavlink / mavlink-devguide

MAVLink Developer Guide
http://mavlink.io
Other
110 stars 135 forks source link

Skydio add parachute protocol #546

Open vivian-zhou-skydio opened 1 month ago

vivian-zhou-skydio commented 1 month ago

adding parachute protocol microservice document and test suite

vivian-zhou-skydio commented 1 month ago

Hi @hamishwillee sorry for the delay - just want to add you as a reviewer for this! :)

hamishwillee commented 1 month ago

@vivian-zhou-skydio Thank you.

  1. Your branch is not allowing me to push subedits. Can you please either allow that, or fetch my branch https://github.com/mavlink/mavlink-devguide/tree/skydio-add-parachute-protocol and cherry-pick these two changes into it:

    pick 8832c63 prettier + add to sidebar
pick d76fd51 Reduce

    What these do is cut back this document quite significantly. There is nothing wrong with it, except there is no point duplicating the docs in the messages - so this makes it much more high level.

  2. Further, reading this has made me question some aspects of the naming in the mavlink PR, and the particular case of the drone trigger. Sorry to kick this off again, but that's what you get when you document stuff!

  3. The first "flaw" in this is that it ignores the existing MAVLink command to enable auto triggering - https://mavlink.io/en/messages/common.html#MAV_CMD_DO_PARACHUTE

    We need to clearly outline the way they interact, and what you've done here can't break the expectations on MAV_CMD_DO_PARACHUTE implementations.

    • If this is sent to disable ATS, what should that do for your parachute? Ditto if it triggers parachute immediately, but your parachute is configured to ignore the offboard control (IMO it should obey this anyway).
    • How do we know if this API is supported on a particular system? Perhaps it just supports MAV_CMD_DO_PARACHUTE, perhaps it supports both? Do we need a protocol bit to query this?
    • Perhaps this is actually two protocols - simple and advanced. That wouldn't change any of these questions, but it might change how we present it.
    • We know that a parachute might be attached to an FC via PWM, so connection should also consider the flight stack. If we have a flight stack then it might need to emit the message instead.