ArduPilot / ArduRemoteID

RemoteID support using OpenDroneID
GNU General Public License v2.0
90 stars 45 forks source link

Session ID Support? #124

Open kc2rxo opened 1 year ago

kc2rxo commented 1 year ago

This is a question I ask after experimenting with this code and a particular behavior made me question how Session IDs would be supported with this firmware (without modifications with my current understanding).

The current code checks if their is Basic ID data persisted in the parameters and pulls from this to fill in the UAS data structure. This can be filled by hard-coding it at compile time or be filled in dynamically with the first Basic ID message obtained via DroneCAN or MAVLink (only when the parameter is empty to begin with). Note this dynamic fill only happens with Basic ID 1, not Basic ID 2 which seems to have its parameter settings only be hard-coded.

From F3586-22:

A unique Session ID (UTM Assigned universally unique identifier (UUID) or Specific Session ID) as described in Specification F3411, Table 1 and Subsection 5.4.5.6, which can be resolved to the corresponding serial number, with access limited to authorized parties only, through a system and process accepted by the FAA.

If a user is to use a Session ID then the Serial Number should, at least by my reading of this statement, never be broadcast as it is considered something that has "access limited to authorized parties only, through a system and process accepted by the FAA".

With the current code setup the first Session ID seen over external transport would be filled in permanently and be re-used until the system is re-flashed or have its parameter cleared (only possible via DroneCAN with strings).

Is there a desired way to accomplish the goal of F3586-22 without having to modify the code or have the external system connect and clear the parameter?

BluemarkInnovations commented 1 year ago

You are right, with the current code it is not possible.

My idea is to submit a PR where there is a flag in the option parameter that indicates whether you want to store the received Basic ID data as parameter (or not). If it is set, it would only broadcast the received basic ID. With this PR it would be possible.

I aim to get it implemented before the end of next week. (I had this idea already for some time, too busy to have it done.)

kc2rxo commented 1 year ago

That would be my solution as well.

Perhaps a solution where flag(s) could be set to signal the configuration of the Basic IDs independently?

That way a manufacturer could set Basic ID 2 to be the Serial Number of aircraft and locking it (its not affected by the whole dynamic fill) and could fulfill the anti-tamper requirement of the US MOC. This would also provide a good fallback if no Session ID is set/found in Basic ID 1 to a Serial Number burned in to Basic ID 2. I'm sure the CAAs would rather have something broadcast rather than nothing.

Japan is the only CAA that I am aware of that requires two UAS IDs to be sent (Serial Number and CAA Assigned). Wonder how that will work out if Session IDs are allowed in the NAS.

BluemarkInnovations commented 1 year ago

So, I finally implemented the PR: https://github.com/ArduPilot/ArduRemoteID/pull/125

W.r.t. to Basic ID 2 and Basic ID 1, although it is implemented as parameters using 1 and 2, from a standard perspective, a receiver can receive Basic ID 2 as 1 and vice versa.

That way a manufacturer could set Basic ID 2 to be the Serial Number of aircraft and locking it (its not affected by the whole dynamic fill) and could fulfill the anti-tamper requirement of the US MOC. This would also provide a good fallback if no Session ID is set/found in Basic ID 1 to a Serial Number burned in to Basic ID 2. I'm sure the CAAs would rather have something broadcast rather than nothing.

So here you can lock Basic ID 1, but use the streamed BasicID info as Basic ID 2. This is already the current functionality.

If the option is set in this PR, it will use the received Basic ID info only. This can be 1 or 2 different BasicID info.

kc2rxo commented 1 year ago

So here you can lock Basic ID 1, but use the streamed BasicID info as Basic ID 2. This is already the current functionality.

If the option is set in this PR, it will use the received Basic ID info only. This can be 1 or 2 different BasicID info.

I realized this after a bit more experimentation with our setup. Your PR looks good.

Now I just need to figure out why half of my Session ID is being displayed as "0"s.

BluemarkInnovations commented 1 year ago

Now I just need to figure out why half of my Session ID is being displayed as "0"s.

Do you use Mission Planner? If so, please try QGC. In the past in some occasions, Mission Planner considered entered basicID data as integer and not as string.

kc2rxo commented 1 year ago

I'm actually using the Python3 library directly to build the ODID messages and sending them over serial to the ESP32.

kc2rxo commented 1 year ago

So what I found is rather interesting and figured I'd share before re-beating my head against my cubical wall.

If I send to the C3 the Session ID of "01656667686970717273747576788980818283" all works wonderfully. If I send "FF2001013456000000000000000000000000000000" again no problems. If I send "FF2001003456000000000000000000000000000000" all I get over the air is "FF2001000000000000000000000000000000000000".

Adding some debug prints I find that t.basic_id.uas_id is exactly what I sent over from Python minus the last byte. But then after the ODID_STR_COPY() call its what I see over the air.

I have done some permutations and as long as the the 4th byte in the Session ID is not 0 everything works. I am greatly disturbed and mythed.

Edit: adding some more confusion looks like if there is a 0x00 anywhere the UAS ID is terminated there - what could be causing it to terminate on 0 since we are using strncpy?

kc2rxo commented 1 year ago

I found the issue. Looks like ODID uses a strncpy when encoding the Basic ID UAS ID field. For Type 1 (Serial) and Type 2 (CAA Assigned) IDs this is fine, but Type 3 (UUID) and Type 4 (Specific Session IDs) this is a problem as they are defined as bytes in the standard not a string.

This strncpy will truncate when it encounters the first 0x00 byte while encoding the UAS ID. I suspect that the decode side also has a problem but have yet to check.

kc2rxo commented 11 months ago

Just a quick update here for this with my findings thus far.

The latest of ODID (a64611d) now has the required fix that treats ID Types properly. It uses memcpy when ID Type is not 1 or 2.

In this quest I also found that Parameters::set_by_name_string which calls either Parameters::Param::set_char64 or Parameters::Param::set_char20 both use strncpy. This exhibits the same problem as above when the UAS ID is stored in the parameters.

As such any time a UAS ID is handled strncpy should be avoided (when handling Types other than 1 and 2). The the main places I have found to be troublesome are where Parameters::set_by_name_string or ODID_STR_COPY that are called in RemoteIDModule.ino.

Should some changes be rolled into #125 to correct this?