[Sponsored by Cubepilot and Holybro] Cube ID via DroneCAN

[julianoes]

This adds support for Cubepilot's Cube ID modules via DroneCAN, so that users are not limited to the Cube ID serial variant.

Thanks to dirksavage88, I borrowed quite a bit of code from #21647.

Tested using QGC master and DroneScanner.
QGC v4.4.0 doesn't properly implement the "Fixed GPS GCS" setting (see mavlink/qgroundcontrol#11673).

Initial work for CubeID was sponsored by CubePilot.
Additionally, testing/fixup for the Holybro Remote ID was sponsored by Holybro.

[PetervdPerk-NXP]

Awesome, you managed to update the libuavcan and dsdl dependencies so that we can generate latest data types?

[julianoes]

@PetervdPerk-NXP I just updated it and nothing seemed to break, so I'm not sure I did it right...

[henrykotze]

We definitely need to test it on hardware, since I have updated previously to support newer data types, but there were issues translating the messages between FMU and Cannodes. I specify remember that the Servo Array Command didnt work as expected.

I will see if I can find the Issue or some internal doc which describes the issue.

[julianoes]

@henrykotze or @AlexKlimaj any tips how I test whether the DroneCAN update breaks anything?

[dagar]

Add a parameter to enable/disable this in UAVCAN? Only allocating UavcanRemoteIDController at init if enabled would suffice, then this doesn't cost anything.

[julianoes]

@dagar what param name do you suggest, given this has subscriptions as well as publications? UAVCAN_EN_RID?

[julianoes]

This is working on my bench, I'd say this is on par with the serial implementation and we should be able to merge it once reviewed and flash issues are resolved.

[julianoes]

@dagar let me know what param name you want, and what else you need me to do to merge this.

[julianoes]

@henrykotze or @AlexKlimaj, please speak up how I have to test this, otherwise I'd like to get this in.

[AlexKlimaj]

I can test in a few days. @dakejahl may be able to test sooner.

[dakejahl]

I don't have the CAN version of the CubeID

[julianoes]

@dakejahl

I don't have the CAN version of the CubeID

That's what I have tested already.

However, I was told that other CAN devices such as CANnodes with servo array might be broken with the update.

[julianoes]

Updated and extended after testing with Holybro's Remote ID module as well which is running ArduRemoteID v1.14.

[julianoes]

FYI @BluemarkInnovations and @dirksavage88

[dirksavage88]

FYI @BluemarkInnovations and @dirksavage88

@julianoes I can test the holybro, but I don't have the Cube ID CAN either

[dagar]

This largely looks good, I'm just not sure about some of the message flow specifics given the range of different use cases (more convoluted than you might expect).

For example rather than just copying around OPEN_DRONE_ID_ARM_STATUS into uORB and then Mavlink streaming it out verbatim, shouldn't PX4 itself be generating OPEN_DRONE_ID_ARM_STATUS based on its overall ability to actually arm which INCLUDES consuming a remote id module's OPEN_DRONE_ID_ARM_STATUS?

[dagar]

Another case is OPEN_DRONE_ID_SYSTEM. It some cases it needs to be forwarded through from the GCS (with GPS) to the remote id module. In other cases it can be generated by PX4 itself using the takeoff location for operator_latitude/operator_longitude. It needs to be one or the other, but definitely not both right?

[julianoes]

Thanks for the review and comments:

For example rather than just copying around OPEN_DRONE_ID_ARM_STATUS into uORB and then Mavlink streaming it out verbatim, shouldn't PX4 itself be generating OPEN_DRONE_ID_ARM_STATUS based on its overall ability to actually arm which INCLUDES consuming a remote id module's OPEN_DRONE_ID_ARM_STATUS?

The way it seems to work is that the remote ID module sends this message out and PX4 parses it, and sends it out to the GCS via MAVLink.

Are you saying PX4 should modify the message?

Another case is OPEN_DRONE_ID_SYSTEM. It some cases it needs to be forwarded through from the GCS (with GPS) to the remote id module. In other cases it can be generated by PX4 itself using the takeoff location for operator_latitude/operator_longitude. It needs to be one or the other, but definitely not both right?

That's how I did it for the DroneCAN ones. If the GCS has sent a system message, then it is just forwarded. If not, then it will fall back to the takeoff location.

If you want we can add a param to explicitly configure that.

[julianoes]

@dagar: regarding the last commit:

I did not manage to properly extract the open_drone_id MAVLink helpers into its own library. At build time there seemed to be a race condition between the MAVLink headers having been generated and the library requiring it.

Making the library depend on mavlink_c_generate seemed to work but made the IO boards fail which don't have MAVLink obviously.

Using ifs to avoid the open_drone_id files to be built for IO failed, as well as making it dependent on the MAVLink module. I assume that's because it is a circular dependency.

Instead I went with just moving the helpers into the MAVLink module and including the file in the UAVCAN driver as well.

Any other idea how I ought to do that?

[julianoes]

@mrpollo it would be good not to cancel all other builds when one fails. Otherwise, I have to fix one board at a time and then try again, just to find the next one that fails.

[hamishwillee]

@julianoes I've added the new uORB messages in PX4/PX4-user_guide#3370

Are there any docs implications to https://docs.px4.io/main/en/peripherals/remote_id.html ?

[julianoes]

I think we already updated the docs @hamishwillee.

[AlexKlimaj]

I actually need sd bench in the ARV6X build. I use it on the production line to test the SD card.

[julianoes]

Oh, I'm sorry! Anything else you can remove instead?

Although, you could consider having a separate test image that gets reflashed with the production image once all tested.

[AlexKlimaj]

It looks like it all fits just adding it back in. I want to avoid having to flash multiple times in production. The time it takes really adds up.

[julianoes]

Ok good. Again, sorry about that but I find it super hard to get anything merged as any feature is going to hit the flash limit on at least some boards, and they all come with a slightly different set of modules and drivers installed, so it's impossible to know what's actually needed on each of them. The PR had been dragging for weeks, so I just took things in my own hands, disabled what I thought was "safe" and merged it. In an idea world I'd wait for everyone to give a thumbs up. Next time I know to ping you when I mess with an ARK board.

[dakejahl]

Looks like the libuavcan update breaks CAN FW updating. I just tested the commits before and after this PR and confirmed it.
#23727