Skip to content
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

Remote ID in PX4 v1.14 #2627

Merged
merged 4 commits into from
Jul 20, 2023
Merged

Remote ID in PX4 v1.14 #2627

merged 4 commits into from
Jul 20, 2023

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Jul 12, 2023

This adds a page from Remote ID in PX4 v1.14. I wanted to capture that it is experimental, and exactly what you can expect right now.

I only added Cube ID as that is the only system I have had confirmed works with current main.

THoughts?

@mrpollo @junwoo091400 et al

PS Am requesting confirmation of Cube ID setup on https://discord.com/channels/1022170275984457759/1108437678871019530/1128846440702169188

en/SUMMARY.md Outdated Show resolved Hide resolved
Copy link
Contributor

@junwoo091400 junwoo091400 left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together!


### Enable Remote ID

Enable Remote ID by [setting](../advanced_config/parameters.md#conditional-parameters) parameter [COM_ARM_ODID](#COM_ARM_ODID) to `2` (it is disabled by default).
Copy link
Contributor

Choose a reason for hiding this comment

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

Little nuance: under the hood this parameter is only allowing arming prevention, but to the user this would be effectively turning Remote ID "on", so it seems fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I'll change it.

The UAV will then require `HEARTBEAT` messages from the Remote ID as a precondition for arming the UAV.
You can also set the parameter to `1` to warn but still allow arming without the Remote ID.

Once enabled, PX4 v1.14 streams these messages by default (in streaming modes: normal, onboard, usb, onboard low bandwidth):
Copy link
Contributor

Choose a reason for hiding this comment

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

We currently stream these messages regardless of whether the COM_ARM_ODID is set or not: https://github.com/PX4/PX4-Autopilot/blob/f000238987ce9d604bd7aa351d870d1c7c47464a/src/modules/mavlink/mavlink_main.cpp#L1518-L1519

Perhaps we should have either a generic remote ID enabler parameter or using COM_ARM_ODID, to not stream these messages when we aren't using Remote ID feature 🤔. I will leave a note on this in PX4/PX4-Autopilot#21777

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We certainly don't want to stream stuff if it is not needed. I know we have a pattern for streaming by default, and for requesting a stream. Do we have a pattern for enabling streaming based on a parameter or enabling a particular driver?

Most places we fly are going to need this, so default enabling probably not the end of the world.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated docs appropriately

The following message can be streamed on request (using [MAV_CMD_SET_MESSAGE_INTERVAL](https://mavlink.io/en/messages/common.html#MAV_CMD_SET_MESSAGE_INTERVAL)):

- [OPEN_DRONE_ID_BASIC_ID](https://mavlink.io/en/messages/common.html#OPEN_DRONE_ID_BASIC_ID) - UAV identity information (essentially a serial number)
- PX4 v1.14 specifies a serial number ([MAV_ODID_ID_TYPE_SERIAL_NUMBER](https://mavlink.io/en/messages/common.html#MAV_ODID_ID_TYPE_SERIAL_NUMBER)) but does not use the specified format (ANSI/CTA-2063 format).
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming Andrew's PR (PX4/PX4-Autopilot#21647) doesn't go into v1.14, this is a true statement.

Then, perhaps we could even elaborate further that we are using Board's GUID value, which is basically a unique device specifier like this (but if I'm not mistaken, for same CPUs, the same GUID will be shared, @davids5 do you have more info on this?) 🤔

Implementation details:
https://github.com/PX4/PX4-Autopilot/blob/f000238987ce9d604bd7aa351d870d1c7c47464a/src/modules/mavlink/streams/OPEN_DRONE_ID_BASIC_ID.hpp#L172-L174

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You said it wouldn't, and I believe you.

en/peripherals/remote_id.md Outdated Show resolved Hide resolved
@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-maintainers-call-july-18-2023/33189/1

@github-actions
Copy link

No flaws found

@hamishwillee
Copy link
Collaborator Author

@junwoo091400 Thank you for your review. I have updated so it is now accurate for current state of play. Of course if PX4/PX4-Autopilot#21647 goes in then it would require an update.

Merging now.

@hamishwillee hamishwillee merged commit ea307c9 into main Jul 20, 2023
1 check passed
@hamishwillee hamishwillee deleted the remoteit branch July 20, 2023 01:43
mrpollo pushed a commit that referenced this pull request Jul 27, 2023
* Remote ID in PX4 v1.14

* Make into standard format for easy extension

* Update en/peripherals/remote_id.md

Co-authored-by: Junwoo Hwang <[email protected]>

* Address feedback

---------

Co-authored-by: Junwoo Hwang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants