Skip to content

Add support for generic payloads #3

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.

Sign up for GitHub

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

Open
vivian-zhou-skydio wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for generic payloads #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions message_definitions/v1.0/development.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,11 @@
<param index="6" reserved="true" default="0"/>
<param index="7" reserved="true" default="NaN"/>
</entry>
<entry value="404" name="MAV_CMD_GENERIC_PAYLOAD_FUNCTIONS_CONTROL" hasLocation="false" isDestination="false">
<description>Allows for enable/disable control over function(s) of a generic payload.</description>
<param index="1" label="Index" minValue="0" increment="1">Index of Function</param>
Copy link

@tal-zaitsev-skydio tal-zaitsev-skydio Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there also be a maxValue of 65535? function_count is a uint16_t

Copy link

@amy-chen-skydio amy-chen-skydio Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Parameters are sent as float and could have a value bigger than 65535

<param index="2" label="Enable" minValue="0" maxValue="1" increment="1">0: Disable, 1: Enable</param>
Copy link

@thomas-watters-skydio thomas-watters-skydio May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<param index="2" label="Enable" minValue="0" maxValue="1" increment="1">0: Disable, 1: Enable</param>
<param index="2" label="Activate" minValue="0" maxValue="1" increment="1">0: Disable, 1: Activate</param>

</entry>
<entry value="550" name="MAV_CMD_SET_AT_S_PARAM" hasLocation="false" isDestination="false">
<description>Allows setting an AT S command of an SiK radio.
</description>
Expand Down Expand Up @@ -567,6 +572,20 @@
<description>RAIM integrity check failed.</description>
</entry>
</enum>
<enum name="GENERIC_PAYLOAD_ERROR_FLAGS">
<entry value="1" name="GENERIC_PAYLOAD_ERROR_FLAGS_SOFTWARE_ERROR">
<description>There is an error with the generic payload's software.</description>
</entry>
<entry value="2" name="GENERIC_PAYLOAD_ERROR_FLAGS_HARDWARE_ERROR">
<description>There is an error with the generic payload's hardware.</description>
</entry>
<entry value="4" name="GENERIC_PAYLOAD_ERROR_FLAGS_OVERTEMP_FAULT">
<description>A specified limit has been reached by the generic paylod's temperature sensor. This only applies if the generic payload monitors temperature.</description>
</entry>
<entry value="8" name="GENERIC_PAYLOAD_ERROR_FLAGS_CUSTOM">
<description>Generic payload custom failure, see custom error flag bitmask for details.</description>
</entry>
</enum>
</enums>
<messages>
<!-- Transactions for parameter protocol -->
Expand Down Expand Up @@ -672,6 +691,24 @@
<field type="float" name="temperature" units="K" invalid="NaN">Fuel temperature. NaN: field not provided.</field>
<field type="uint32_t" name="fuel_type" enum="MAV_FUEL_TYPE">Fuel type. Defines units for fuel capacity and consumption fields above.</field>
</message>
<message id="398" name="GENERIC_PAYLOAD_STATUS">
<description>Generic payload status.</description>
<field type="uint32_t" name="uptime_ms" units="ms">Time since the start-up of the generic payload in ms</field>
<field type="char[32]" name="name">Generic payload name to be used in UI. This is a NULL terminated string. If it is exactly 32 characters long, add a terminating NULL. If this string is empty, a generic name is shown to the user.</field>
Copy link

@tal-zaitsev-skydio tal-zaitsev-skydio Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"If it is exactly 32 characters long, add a terminating NULL" is confusing. imo it's clearer to say something like "stores null terminated string if less than 32 characters, or 32 characters without null termination." Similarly to the language used elsewhere in this file

Copy link

@amy-chen-skydio amy-chen-skydio Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was copied from existing example STORAGE_TYPE enum

Copy link

@thomas-watters-skydio thomas-watters-skydio Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Textual storage name to be used in UI (microSD 1, Internal Memory, etc.) This is a NULL terminated string. If it is exactly 32 characters long, add a terminating NULL. If this string is empty, the generic type is shown to the user.

from STORAGE_INFORMATION

it is a bit weirdly worded though. because it sounds like they want 33bytes for a string that is exactly 32chars long.. but that would not work in the packing. unless oddly the packing doesn't check the length but simply looks for a null character, and then verifies the length afterwards

<field type="uint16_t" name="function_count">Total number of functions on the generic payload that can be controlled.</field>
<field type="uint32_t" name="error_flags" enum="GENERIC_PAYLOAD_ERROR_FLAGS" display="bitmask">Errors</field>
<field type="uint32_t" name="custom_error_flags" display="bitmask">Bitmap used to show custom error flags.</field>
<field type="float" name="power_draw" units="W" invalid="NaN">The power draw of the generic payload. NaN: field not provided</field>
<field type="uint16_t" name="weight" units="g" invalid="0">Generic payload weight. 0: field not provided.</field>
Copy link

@tal-zaitsev-skydio tal-zaitsev-skydio Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this expected to change dynamically for things like droppers? If not, should this then reflect the max weight of the payload?

<field type="float" name="temp_c" invalid="NaN">Temperature in Celsius. NaN: field not provided.</field>
</message>
<message id="399" name="GENERIC_PAYLOAD_FUNCTION_STATUS">
Copy link

@thomas-watters-skydio thomas-watters-skydio May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's reason to expand this to include something that specifies the control scheme ie :

enum ControlSchemeFlags {
  LATCHING = 1,
  NON_LATCHING = 2,
  BINARY = 4, 
  ANALOG = 8, 
  PWM = 16,
};

The non-latching idea was motivated by the dropper needing to "reset" after actuation. However in this case the current UI only sends on USER_ACTION to deploy, not reset. (Could be updated to send RESET maybe).

But non-latching would still be interpreted by attachment, ie it could latch for a time period, then reset on its own (in the case of the dropper), or it fires and resets immediately (one shot gpio0 on/off)

<description>Generic payload function status.</description>
Copy link

@thomas-watters-skydio thomas-watters-skydio Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<description>Generic payload function status.</description>
<description>Generic payload function status. Use [MAV_CMD_REQUEST_MESSAGE](https://mavlink.io/en/messages/common.html#MAV_CMD_REQUEST_MESSAGE).param2 to indicate the index/id of requested function: 0 for all, 1 for first, 2 for second, etc.</description>

<field type="char[32]" name="name">Generic payload function name to be used in UI. This is a NULL terminated string. If it is exactly 32 characters long, add a terminating NULL. If this string is empty, a generic name is shown to the user.</field>
<field type="uint16_t" name="index">Index of this function on the generic payload.</field>
<field type="uint8_t" name="enable">0: Disable, 1: Enable</field>
<field type="uint16_t" name="function_count">Total number of functions on the generic payload that can be controlled.</field>
Copy link

@tal-zaitsev-skydio tal-zaitsev-skydio Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this in both messages?

Copy link

@thomas-watters-skydio thomas-watters-skydio Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vivian-zhou-skydio what was the reasoning here, i feel like I had convinced myself it was useful

</message>
<message id="414" name="GROUP_START">
<description>Emitted during mission execution when control reaches MAV_CMD_GROUP_START.</description>
<field type="uint32_t" name="group_id">Mission-unique group id (from MAV_CMD_GROUP_START).</field>
Expand Down
Loading