Open Drone ID Live GNSS and Arm status prototype #21647

dirksavage88 · 2023-05-28T13:27:05Z

-Changed operator position to Live GNSS: To test you will need a tablet/laptop with built-in GPS or a USB GPS receiver module. Using QGC daily (they have merged the Remote ID support) you will need to select remote ID in the settings -> mavlink/qgroundcontrol#10600

-Added prototype arm status through the health and arming checks module. This is just for testing purposes and the actual implementation can be different. If you have remote ID enabled but don't have a module plugged in, you will still be able to arm. However if you have a module plugged in and it is not receiving valid GPS or some other failure in operator ID or basic ID it will result in a preflight fail and not allow you to arm.

-Added mavlink receiver support for drone ID arm status

According to David Sastre we are trying to verify the following settings:

GPS indicator: If set to FIXED, it will go green if valid lat/long is introduced. If it is set to LIVE, it will go green if valid GPS is received, and latest GCS GPS info is received less than 5 seconds ago. It will go red otherwise. Also, if it is set to LIVE, but region is set to FAA, it will go red if GCS GPS doesn't provide altitude ( needed for FAA region ), but for EU this doesn't apply as altitude is not mandatory.

Basic ID: will go green if the armstatus message received is "GOOD_TO_ARM". It will go red if armstatus message reports an error string "missing basic_id message". This is not ideal, but it is the only way given the circumstances of detecting precisely when it is just Basic ID failure. A bitmask or something in armstatus message will be more appropriate, but these are the cards we can play with.

Arm status: will go green if armstatus message is good. If armstatus message contains errors it will go red.

Comms indicator: will go green as soon as we receive an armstatus message. Will go red as soon as the timeout ( 2.5 seconds ) expires with no message received.

Operator ID: will only check if the data set in QGC is valid, not empty and valid ID type.

julianoes · 2023-05-29T03:52:03Z

@dirksavage88 thanks! Make sure to rebase on top of main to fix the conflicts, and mark the PR is ready when I should review.

dagar · 2023-05-30T14:41:11Z

Can you revert the whitespace changes, it makes the overall PR huge and hard to review.

We also need to reconcile this with #21652.

src/modules/mavlink/streams/OPEN_DRONE_ID_SYSTEM.hpp

dirksavage88 · 2023-05-30T23:23:08Z

Can you revert the whitespace changes, it makes the overall PR huge and hard to review.

We also need to reconcile this with #21652.

I reverted files with large diffs.

Also rebased to main @julianoes

For reconciling with the other PR, I really intended this draft PR to be for people to experiment with the RID modules and get something working since there wasn't anything out there yet.

dirksavage88 · 2023-06-02T01:56:10Z

This PR is ready to be tested. I was able to successfully arm with the Dronebeacon DB200. All required ODID messages have been satisfied. Reverted some files to work with #21652
@mrpollo

hamishwillee · 2023-06-29T05:15:54Z

src/modules/mavlink/mavlink_main.cpp

@@ -1729,7 +1737,11 @@ Mavlink::configure_streams_to_default(const char *configure_single_stream)
 		configure_stream_local("MAG_CAL_REPORT", 1.0f);
 		configure_stream_local("MANUAL_CONTROL", 5.0f);
 		configure_stream_local("NAV_CONTROLLER_OUTPUT", 10.0f);
+		configure_stream_local("OPEN_DRONE_ID_ARM_STATUS", 1.f);


Why are we streaming OPEN_DRONE_ID_ARM_STATUS? Its inbound from the remoteID hardware. Should be used to indicate healthy/unhealthy for the remoteID.

I.e. an update to #21652

I'm probably missing something ...

The Bluemark module requires the stream to work correctly. Without it, the module health checks are not satisfied and arming is prevented. Perhaps Bluemark can clarify on why this is.

how much of this behaviour is defined in the spec vs left to the manufacturer to figure out?

@tridge may know more about the how the spec has been implemented. The vendor may have just an abstraction layer on top of ODID core library for their specific hardware. Or they may use proprietary software and just satisfy the mavlink ODID definitions and don’t use ardu (or ODID core library)

Yeah even in Ardupilot, the ARM_STATUS message isn't getting sent out, so I am puzzled as to why Bluemark module needs it 🤔

https://github.com/ArduPilot/ardupilot/blob/5fa8b887a2df8a85822f84f41eec64bef5066895/libraries/AP_OpenDroneID/AP_OpenDroneID.cpp#L266-L326

The Bluemark module requires the stream to work correctly. Without it, the module health checks are not satisfied and arming is prevented. Perhaps Bluemark can clarify on why this is.

Well in the standard Remote ID regulation, you need to make sure the Remote ID information is correct and there are no failures. So if the Remote ID module detect an issue, it should report it back to the GCS.

Note there are open PRs that needs to be merged into master of ArduRemoteID: PR#101 and PR#102 that will improve these checks or its status message.

@BluemarkInnovations I was going to sarcastically say where did you get that "requirement", but you're absolutely right - its in the MAVLink spec:

Similarly, the autopilot and GCS components must listen to the ARM_STATUS from the RID transmitter component(s) and not allow the UA to be airborne before the RID transmitter component(s) is ready. During flight, if the arm status indicates a failure, similar action must be taken as for a lack of HEARTBEAT messages.

The vehicle shouldn't start if it doesn't have this so what the GCS does is irrelevant. Though this is needed for monitoring the precise cause of arming failure, unless the flight stack were to emit an event (and none is standardized).

@dirksavage88 What is interesting here is that the status would also have to be updated in LOCATION (that is what appears to happen if the HEARTBEAT disappears).

I'll see if I can tidy the spec: mavlink/mavlink-devguide#511

FWIW I don't think this should be streamed by default. If there is no GPS then firing this off is a waste of bandwidth. perhaps it could be requested by GCS or enabled by a parameter.

Late to the party as I only started looking into Remote ID yesterday, but I had the same initial reaction as @hamishwillee back in June.

In my mind it's more logical if you just set up mavlink forwarding so that the GCS can subscribe directly to the topic from the Remote ID hardware without going through the "useless" steps of repackaging the incoming message and publishing it as uORB only to subscribe to the uORB message to republish a mavlink message with basically the same content as the initial message.

Maybe I am too naive and miss something essential here?

Yes, that would make sense, just forwarding it. However, for a module that talks DroneCAN or some other protocol, this might be required again.

hamishwillee · 2023-06-29T05:45:27Z

Is this planned for PX4 v1.14?

junwoo091400 · 2023-06-29T12:08:30Z

For reference, user testing report in discord: https://discord.com/channels/1022170275984457759/1038284900081614879/1115609787317628998

mrpollo · 2023-06-29T13:49:59Z

@hamishwillee not by design, but would make sense if its a clean cherry pick

junwoo091400 · 2023-06-30T06:55:54Z

@dirksavage88 Btw, you have merged the master branch into your branch (git merge master), but for rebase it is commonly done via rebase (git rebase master) to not create that merge commit.

It is not critical, but just for your information!

More info: https://medium.com/@harishlyadav/when-to-use-git-rebase-explained-3c8192cba5c7

msg/OpenDroneIdArmStatus.msg

src/modules/mavlink/streams/OPEN_DRONE_ID_OPERATOR_ID.hpp

src/modules/mavlink/streams/OPEN_DRONE_ID_SYSTEM.hpp

dirksavage88 · 2023-07-11T03:07:16Z

BASIC ID is now cached. @jnomikos will take a look at the system message. I still am not sure how to go about the serial number or the takeoff vs live vs fixed (and thus standard remote ID vs broadcast module add-on).

I can not arm with takeoff or fixed when FAA is selected: trying to select either in QGC does not work. If I select EU it allows to choose fixed/takeoff.

@Davidsastresas is this not implemented for the FAA option in QGC? I would like to be able to choose takeoff or fixed for FAA rules (under broadcast module exception).

Davidsastresas · 2023-07-12T23:03:41Z

Hello @dirksavage88,

back when we implemented this, for FAA it wasn't allowed the fixed/takeoff location, that is why it is only available for CE. Has it changed?

dirksavage88 · 2023-07-20T17:37:46Z

Hello @dirksavage88,

back when we implemented this, for FAA it wasn't allowed the fixed/takeoff location, that is why it is only available for CE. Has it changed?

This is the case for standard remote ID under FAA rules. I think the confusion is that some recreational users who assemble their own pixhawk drones will buy a broadcast module like the Bluemark one tested in this PR and not realize that it (currently) forces them to comply with standard remote ID, where they would probably be fine under the broadcast module rules under FAA which allowed takeoff position instead of live operator coordinates. Bluemark and other vendors make their own “broadcast rules” module that has zero interface with the autopilot and this can only be used recreationally (and with a built kit, not a fully assembled product) under FAA rules starting this September. This is my current interpretation and I was also confused with the broadcast module vs standard remote ID nuances as there is overlap.

DronecodeBot · 2023-08-02T15:21:40Z

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-community-q-a-august-02-2023/33434/1

ryanjAA · 2023-08-08T00:25:21Z

This should be merged.

dirksavage88 · 2023-08-08T00:29:01Z

This should be merged.

I am working on cleaning it up and rebasing. The plan is to merge in the next 2 weeks. Emergency status will be handled via a separate PR (still trying to figure out the QGC side of it). We have a RemoteID call this Thursday 10AM CST if you are interested. I will have a cube ID and the bluemark module in hand and can support if anyone has technical issues getting their module up and running.

jnomikos · 2023-08-09T15:17:54Z

This should be merged.

I am working on cleaning it up and rebasing. The plan is to merge in the next 2 weeks. Emergency status will be handled via a separate PR (still trying to figure out the QGC side of it). We have a RemoteID call this Thursday 10AM CST if you are interested. I will have a cube ID and the bluemark module in hand and can support if anyone has technical issues getting their module up and running.

Yep, I am currently working on getting Emergency Status working

ryanjAA · 2023-08-09T18:52:30Z

The aim would be for this to get into 1.14 before the release in a couple weeks but depends on how much more is needed.

dirksavage88 · 2023-08-11T01:43:56Z

The aim would be for this to get into 1.14 before the release in a couple weeks but depends on how much more is needed.

So the Bluemark module is working as intended with the latest on this PR.

I can even take out the system message stream and just forward the mavlink system message tream to the module from QGC (pending a push from my local branch). When I disconnect the ground station gps, the module reports operator location failure and sets arm status to 1 (fail). It also checks for the self ID, basic ID, and Operator ID and reports a failure if it doesn't see these messages. The Cube ID has no such logic. It does not have a built-in uas_id either.

The Cube ID does not have logic to check the basic RID requirements and then set the arm status to fail. We would have to implement that logic in PX4.

dirksavage88 · 2023-08-13T21:35:43Z

Rebased to main and squashed. I removed all streams that QGC currently handles: https://github.com/mavlink/qgroundcontrol/blob/1d59706e2b1d35465307bad03151a0bb15ae7b9c/src/Vehicle/RemoteIDManager.cc#L156

with these changes in place mavlink forwarding will have to be enabled on the telem port the RID is connected to @hamishwillee and for FAA rules a external gps or tablet/phone/UDP gps source is to be used for live operator position under standard remote ID rules.

The Bluemark module has internal logic to set arm status if it does not get valid location data or missing ID messages. The Cube ID from my testing does not have this logic built-in and PX4 does not handle setting arm status (nor does it do checks beyond reading arm status coming from the module).

Modules Tested to work:

[x ] BlueMark Db200
[x ] Cube ID*
[x ] Holybro

Modules I have not yet tested:

Mro
Wurzbach electronics

no internal logic to handle Arm status if not receiving the required ODID messages

@dagar I would like to know your ideas for getting open drone ID baked in at compile time instead of a parameter. It can be a separate PR if needed.

hamishwillee · 2023-08-21T22:31:22Z

Sounds cool. Just FMI, any work on "tamper proofness" of the IDs happened?

dirksavage88 · 2023-08-22T00:15:58Z

Sounds cool. Just FMI, any work on "tamper proofness" of the IDs happened?

@hamishwillee I didn’t look into tamper proofness as it could be it’s own PR with a more focused effort (e.g more architecture and resource constraints consideration). I’m open to ideas on how to implement remote ID enforced at compile time (maybe a module?) instead of a parameter change.

With this PR I reduced the streams so that QGC does most of the work.

hamishwillee · 2023-11-01T00:55:11Z

@hamishwillee I didn’t look into tamper proofness as it could be it’s own PR with a more focused effort (e.g more architecture and resource constraints consideration). I’m open to ideas on how to implement remote ID enforced at compile time (maybe a module?) instead of a parameter change.

FWIW My understanding is that QGC will not allow the parameter to be set/it will be hidden in the UI. Commercial vendors mostly seem to want to implement their own solutions for more than that. I guess we'll talk more about what might be done in the flight stack once FAA starts making more rulings on particular implementations.

With this PR I reduced the streams so that QGC does most of the work.

When this goes in, let's talk docs again. I presume what you mean here is that rather than streaming by default QGC makes requests for the things it expects the vehicle to emit? IF so, not sure about that - QGC might not be in the loop at setup time. But anyone :-).

Any progress on getting this in? It would be great to revisit the docs that currently mention this as being experimental.

hamishwillee · 2023-11-28T21:58:48Z

Is this ever going in?

ThomasRigi · 2023-11-30T10:31:54Z

src/modules/commander/HealthAndArmingChecks/checks/openDroneIDCheck.cpp

-		/* EVENT
-		 * @description
-		 * Open Drone ID system failed to report. Make sure it is setup and installed properly.
-		 *
-		 * <profile name="dev">
-		 * This check can be configured via <param>COM_ARM_ODID</param> parameter.
-		 * </profile>
-		 */
-		reporter.armingCheckFailure(affected_modes, health_component_t::open_drone_id,
-					    events::ID("check_open_drone_id_missing"),
-					    events::Log::Error, "Open Drone ID system missing");
+	if (_open_drone_id_arm_status_sub.copy(&odid_module_arm_status)) {

-		if (reporter.mavlink_log_pub()) {
-			mavlink_log_critical(reporter.mavlink_log_pub(), "Preflight Fail: Open Drone ID system missing");
+		if (_param_com_arm_odid.get() == 2) {
+			// disallow arming without the Open Drone ID system
+			affected_modes = NavModes::All;


Wouldn't it be better to switch the order here ? First check the parameter, and fail if _open_drone_id_arm_status_sub.copy fails ?

I'm not very familiar with the details of uORB, but don't we run the danger to not fail the check if we never received a open_drone_id_arm_status message in the first place ? I'm assuming that if it was never published then _open_drone_id_arm_status_sub.copy could fail.

More generally, I'm assuming that if it's in an if-clause it can fail. No questions asked why. And in this case you wouldn't have the arming prevention.

we are copying into a local variable (uorb 'struct') from the subscribed topic. We modify the copy and fill out the message data accordingly. Check any other commander check and the same logic applies.

Example:

PX4-Autopilot/src/modules/commander/HealthAndArmingChecks/checks/offboardCheck.cpp

Line 44 in 73c77f4

if (_offboard_control_mode_sub.copy(&offboard_control_mode)) {

I didn't see any advertise() of this uORB topic, and it's only published if you've received an incoming MAVLINK_MSG_ID_OPEN_DRONE_ID_ARM_STATUS. Seems like .copy is not working if it was never published: https://logs.px4.io/plot_app?log=fd3f182f-4703-415c-930e-b2aad8307eb9

I could arm with COM_ARM_ODID = 2 and definitely no RemoteID connected. I could arm fine with QGC, which I should NOT be able to do.

Strange, I remember testing removing the RID module and watching arm status prevent arming. What QGC version are you using?

you can add a px4_info in that if clause as a sanity check, but it should still prevent arming since wether the topic info is being advertised should be independent of the rid module presence (e.g you can still copy the struct empty or not):

PX4-Autopilot/src/modules/commander/HealthAndArmingChecks/checks/openDroneIDCheck.cpp

Line 57 in 73c77f4

if (!context.status().open_drone_id_system_present) {

I didn't remove the RID module, I never plugged it in in the first place. I'm using a daily build from a few months ago. (around 27.11.23). I could do the same thing without QGC though, so it shouldn't matter.

If we don't pass line 50 because open_drone_id_arm_status was never advertised (published) in the first place then it doesn't matter how we would refuse the check in line 57 as we never enter inside.

@ThomasRigi please try with the latest commit on the feature branch on my fork

ThomasRigi · 2023-11-30T13:59:26Z

I had a look at this PR and think that in general it's good to go in except for my comment on the ODID arming check sequence (#21647 (comment))

I also reopened an old discussion because I didn't quite understand / agree with it, but I might be biased from only using the Flarm Atom, who tell you in their manual to enable mavlink forwarding. If other units can be set up without mavlink forwarding, I guess that it makes sense to actively republish it from PX4, so I'm not going to insist on this point if consensus was clear that republishing is the only "safe enough" way.

src/modules/mavlink/streams/OPEN_DRONE_ID_ARM_STATUS.hpp

julianoes · 2024-04-04T01:27:32Z

Bump, should this still go in?

…. Removed all streams that QGC sends currently. Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Co-authored-by: Thomas Stauber <48206725+ThomasRigi@users.noreply.github.com>

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Signed-off-by: Julian Oes <julian@oes.ch>

julianoes · 2024-05-01T22:54:59Z

I've rebased the PR and force pushed the branch.
I've also fixed formatting.
And I've restored OPEN_DRONE_ID_BASIC_ID.hpp which was probably removed by accident.

That being said I'm not sure the changes are correct.

dirksavage88 · 2024-05-02T11:31:59Z

It needs to be tested again with the RID modules @julianoes

IIRC the basic ID file was removed and mavlink forwarding was enabled (forward basic ID) to save resources on the FC.

julianoes self-requested a review May 29, 2023 03:51

This was referenced May 29, 2023

Remote ID showing failed GCS GPS for Live GNSS mavlink/qgroundcontrol#10654

Closed

Add Open Drone ID arming check #21652

Merged

dagar reviewed May 30, 2023

View reviewed changes

src/modules/mavlink/streams/OPEN_DRONE_ID_SYSTEM.hpp Outdated Show resolved Hide resolved

dirksavage88 force-pushed the remote_id_arm branch from 0bf6b4e to 77341fa Compare May 30, 2023 23:21

dirksavage88 marked this pull request as ready for review May 31, 2023 01:58

junwoo091400 added the Feature: Remote ID label Jun 1, 2023

hamishwillee reviewed Jun 29, 2023

View reviewed changes

hamishwillee mentioned this pull request Jun 29, 2023

Add Remote ID docs to PX4 v1.14 level PX4/PX4-user_guide#2605

Open

junwoo091400 mentioned this pull request Jun 29, 2023

Remote ID Development Coordination #21777

Open

10 tasks

junwoo091400 reviewed Jun 30, 2023

View reviewed changes

msg/OpenDroneIdArmStatus.msg Show resolved Hide resolved

junwoo091400 reviewed Jun 30, 2023

View reviewed changes

src/modules/mavlink/streams/OPEN_DRONE_ID_OPERATOR_ID.hpp Outdated Show resolved Hide resolved

hamishwillee reviewed Jul 5, 2023

View reviewed changes

src/modules/mavlink/streams/OPEN_DRONE_ID_SYSTEM.hpp Outdated Show resolved Hide resolved

jnomikos mentioned this pull request Jul 6, 2023

{Sponsored by Intelliterra} OPEN_DRONE_ID_SYSTEM, OPEN_DRONE_ID_SELF_ID, and OPEN_DRONE_ID_OPERATOR_ID values getting cached within PX4 dirksavage88/Firmware#1

Merged

junwoo091400 mentioned this pull request Jul 13, 2023

Remote ID in PX4 v1.14 PX4/PX4-user_guide#2627

Merged

dirksavage88 force-pushed the remote_id_arm branch from 923e4a8 to 3745a94 Compare August 13, 2023 21:30

ThomasRigi reviewed Nov 30, 2023

View reviewed changes

ThomasRigi reviewed Dec 4, 2023

View reviewed changes

src/modules/mavlink/streams/OPEN_DRONE_ID_ARM_STATUS.hpp Show resolved Hide resolved

ryanjAA mentioned this pull request Jan 26, 2024

Operator id issue ArduPilot/ArduRemoteID#138

Open

dirksavage88 and others added 4 commits May 2, 2024 10:32

Open Drone ID basic serial support via mavlink forwarding and streams…

d52da5f

…. Removed all streams that QGC sends currently. Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Update src/modules/mavlink/streams/OPEN_DRONE_ID_ARM_STATUS.hpp

621431d

Co-authored-by: Thomas Stauber <48206725+ThomasRigi@users.noreply.github.com>

fix to missing remote ID module handling

be02029

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Fix formatting

a33d48d

Signed-off-by: Julian Oes <julian@oes.ch>

julianoes force-pushed the remote_id_arm branch from 048517a to a33d48d Compare May 1, 2024 22:49

mavlink: restore OPEN_DRONE_ID_BASIC_ID

9ff0c4f

Signed-off-by: Julian Oes <julian@oes.ch>

Open Drone ID Live GNSS and Arm status prototype #21647

Are you sure you want to change the base?

Open Drone ID Live GNSS and Arm status prototype #21647

Conversation

dirksavage88 commented May 28, 2023 • edited

julianoes commented May 29, 2023

dagar commented May 30, 2023

dirksavage88 commented May 30, 2023 • edited

dirksavage88 commented Jun 2, 2023 • edited

hamishwillee Jun 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamishwillee Jul 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamishwillee commented Jun 29, 2023

junwoo091400 commented Jun 29, 2023

mrpollo commented Jun 29, 2023

junwoo091400 commented Jun 30, 2023 • edited

dirksavage88 commented Jul 11, 2023 • edited

Davidsastresas commented Jul 12, 2023

dirksavage88 commented Jul 20, 2023

DronecodeBot commented Aug 2, 2023

ryanjAA commented Aug 8, 2023

dirksavage88 commented Aug 8, 2023

jnomikos commented Aug 9, 2023

ryanjAA commented Aug 9, 2023

dirksavage88 commented Aug 11, 2023 • edited

dirksavage88 commented Aug 13, 2023 • edited

hamishwillee commented Aug 21, 2023

dirksavage88 commented Aug 22, 2023

hamishwillee commented Nov 1, 2023 • edited

hamishwillee commented Nov 28, 2023

Choose a reason for hiding this comment

dirksavage88 Mar 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirksavage88 Mar 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirksavage88 Mar 12, 2024 • edited

Choose a reason for hiding this comment

ThomasRigi commented Nov 30, 2023

julianoes commented Apr 4, 2024

julianoes commented May 1, 2024

dirksavage88 commented May 2, 2024

dirksavage88 commented May 28, 2023 •

edited

dirksavage88 commented May 30, 2023 •

edited

dirksavage88 commented Jun 2, 2023 •

edited

hamishwillee Jun 29, 2023 •

edited

hamishwillee Jul 5, 2023 •

edited

junwoo091400 commented Jun 30, 2023 •

edited

dirksavage88 commented Jul 11, 2023 •

edited

dirksavage88 commented Aug 11, 2023 •

edited

dirksavage88 commented Aug 13, 2023 •

edited

hamishwillee commented Nov 1, 2023 •

edited

dirksavage88 Mar 11, 2024 •

edited

dirksavage88 Mar 12, 2024 •

edited

dirksavage88 Mar 12, 2024 •

edited