Extend SENS_GPS_PRIME usage for UAVCAN GNSS devices

[Claudio-Chies]

Solved Problem

When using multiple UAVCAN GNSS modules, users were not able to set the primary one as they would be able to with serial modules

Solution

With this i extend the usage of the SENS_GPS_PRIME to GNSS devices, whereby the parameter now works as follows:

-1 : Auto (equal priority for all instances)
0 : Main serial GPS instance
1 : Secondary serial GPS instance
2-127 : UAVCAN module node ID

also changed the default behavior to be automatic. because a default value of 0 with an all UAVCAN setup is not ideal.

Changelog Entry

For release notes:

Feature: Extended SENS_GPS_PRIME for UAVCAN GNSS devices

Limitations

the only limiation is that the GNSS module cant have static NodeID 1, which is a valid NodeID bus is usualy the autopilot

Test coverage

• Tested with 2 Serial GNSS Modules
• Tested with 1 Serial, 1 UAVCAN module, where the Serial had priority
• Tested with 1 Serial, 1 UAVCAN module, where the UAVCAN had priority
• Tested with 2 UAVCAN Modules, where both were connected at start
• Tested with 2 UAVCAN Modules, where the one with priority was connected a few seconds after boot.

[github-actions]

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 552 byte (0.03 %)]

    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +552  +0.0%    +552    .text
 -99.9%     +92 -99.9%     +92    [23 Others]
  +5.7%     +88  +5.7%     +88    UavcanGnssBridge::process_fixx<>()
   +18%     +44   +18%     +44    UavcanSensorBridgeBase::publish()
   +37%     +40   +37%     +40    UavcanHygrometerBridge::hygro_sub_cb()
   +28%     +36   +28%     +36    UavcanGnssRelativeBridge::rel_pos_heading_sub_cb()
   +12%     +36   +12%     +36    sensors::VehicleGPSPosition::Run()
   +13%     +28   +13%     +28    UavcanBarometerBridge::air_pressure_sub_cb()
   +11%     +20   +11%     +20    UavcanDifferentialPressureBridge::air_sub_cb()
  +0.0%     +20  +0.0%     +20    g_cromfs_image
   +36%     +20   +36%     +20    uavcan::ReceivedDataStructure<>::safeget<>()
   +12%     +16   +12%     +16    UavcanAccelBridge::init_driver()
  +7.1%     +16  +7.1%     +16    UavcanFlowBridge::flow_sub_cb()
   +12%     +16   +12%     +16    UavcanGyroBridge::init_driver()
   +12%     +16   +12%     +16    UavcanMagnetometerBridge::init_driver()
   +12%     +16   +12%     +16    UavcanRangefinderBridge::init_driver()
  +5.4%      +8  +5.4%      +8    UavcanAccelBridge::UavcanAccelBridge()
  +4.1%      +8  +4.1%      +8    UavcanBarometerBridge::UavcanBarometerBridge()
  +5.4%      +8  +5.4%      +8    UavcanDifferentialPressureBridge::UavcanDifferentialPressureBridge()
  +5.4%      +8  +5.4%      +8    UavcanFlowBridge::UavcanFlowBridge()
  +5.4%      +8  +5.4%      +8    UavcanGnssRelativeBridge::UavcanGnssRelativeBridge()
  +5.4%      +8  +5.4%      +8    UavcanGyroBridge::UavcanGyroBridge()
+0.0%    +378  [ = ]       0    .debug_abbrev
+0.0%     +16  [ = ]       0    .debug_aranges
+0.0%     +44  [ = ]       0    .debug_frame
+0.0% +9.57Ki  [ = ]       0    .debug_info
+0.0% +2.20Ki  [ = ]       0    .debug_line
+0.0% +1.73Ki  [ = ]       0    .debug_loclists
+0.1%    +524  [ = ]       0    .debug_rnglists
   +50%      +1  [ = ]       0    [Unmapped]
  +0.1%    +523  [ = ]       0    [section .debug_rnglists]
+0.1% +2.96Ki  [ = ]       0    .debug_str
+0.0%    +292  [ = ]       0    .strtab
  +1.9%      +1  [ = ]       0    UavcanSensorBridgeBase::get_channel_for_node()
 -37.2%     -16  [ = ]       0    __arm_switchcontext_veneer
  +100%     +16  [ = ]       0    __memset_veneer
   +46%    +291  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
+0.0%     +64  [ = ]       0    .symtab
 -50.0%     -16  [ = ]       0    UavcanAccelBridge::imu_sub_cb()
   +33%     +16  [ = ]       0    UavcanAccelBridge::init_driver()
 -50.0%     -16  [ = ]       0    UavcanGyroBridge::imu_sub_cb()
   +33%     +16  [ = ]       0    UavcanGyroBridge::init_driver()
   +33%     +16  [ = ]       0    UavcanRangefinderBridge::init_driver()
 -50.0%     -16  [ = ]       0    UavcanRangefinderBridge::range_sub_cb()
   +33%     +16  [ = ]       0    ___ZNK3Ekf21updateAidSourceStatusI24estimator_aid_source3d_sN6matrix7Vector3IfEES4_EEvRT_RKyRKT1_RKT0_SE_SE_f.isra.0_veneer
 -40.0%     -32  [ = ]       0    __arm_switchcontext_veneer
   +67%     +32  [ = ]       0    __memset_veneer
 -25.0%     -16  [ = ]       0    __nxsem_wait_irq_veneer
 -33.3%     -16  [ = ]       0    __nxsig_lowest_veneer
   +50%     +16  [ = ]       0    __stm32_i2c_setclock_veneer
 -33.3%     -16  [ = ]       0    sensors::VehicleGPSPosition::ParametersUpdate()
  +100%     +16  [ = ]       0    sensors::VehicleGPSPosition::Start()
 -20.0%     -16  [ = ]       0    uavcan::DynamicArrayBase<>::validateRange()
   +62%     +80  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
-5.6%    -552  [ = ]       0    [Unmapped]
+0.0% +17.7Ki  +0.0%    +552    TOTAL

px4_fmu-v6x [Total VM Diff: 560 byte (0.03 %)]

    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +560  +0.0%    +560    .text
  +5.7%     +88  +5.7%     +88    UavcanGnssBridge::process_fixx<>()
 -99.9%     +88 -99.9%     +88    [22 Others]
   +18%     +44   +18%     +44    UavcanSensorBridgeBase::publish()
   +37%     +40   +37%     +40    UavcanHygrometerBridge::hygro_sub_cb()
   +28%     +36   +28%     +36    UavcanGnssRelativeBridge::rel_pos_heading_sub_cb()
   +12%     +36   +12%     +36    sensors::VehicleGPSPosition::Run()
  +0.0%     +32  +0.0%     +32    g_cromfs_image
   +13%     +28   +13%     +28    UavcanBarometerBridge::air_pressure_sub_cb()
   +11%     +20   +11%     +20    UavcanDifferentialPressureBridge::air_sub_cb()
   +36%     +20   +36%     +20    uavcan::ReceivedDataStructure<>::safeget<>()
   +12%     +16   +12%     +16    UavcanAccelBridge::init_driver()
  +7.1%     +16  +7.1%     +16    UavcanFlowBridge::flow_sub_cb()
   +12%     +16   +12%     +16    UavcanGyroBridge::init_driver()
   +12%     +16   +12%     +16    UavcanMagnetometerBridge::init_driver()
   +12%     +16   +12%     +16    UavcanRangefinderBridge::init_driver()
  +5.4%      +8  +5.4%      +8    UavcanAccelBridge::UavcanAccelBridge()
  +4.1%      +8  +4.1%      +8    UavcanBarometerBridge::UavcanBarometerBridge()
  +5.4%      +8  +5.4%      +8    UavcanDifferentialPressureBridge::UavcanDifferentialPressureBridge()
  +5.4%      +8  +5.4%      +8    UavcanFlowBridge::UavcanFlowBridge()
  +5.4%      +8  +5.4%      +8    UavcanGnssRelativeBridge::UavcanGnssRelativeBridge()
  +5.4%      +8  +5.4%      +8    UavcanGyroBridge::UavcanGyroBridge()
+0.0%    +378  [ = ]       0    .debug_abbrev
+0.0%     +16  [ = ]       0    .debug_aranges
+0.0%     +44  [ = ]       0    .debug_frame
+0.0% +9.57Ki  [ = ]       0    .debug_info
+0.0% +2.20Ki  [ = ]       0    .debug_line
+0.0% +1.70Ki  [ = ]       0    .debug_loclists
+0.1%    +521  [ = ]       0    .debug_rnglists
  [DEL]      -2  [ = ]       0    [Unmapped]
  +0.1%    +523  [ = ]       0    [section .debug_rnglists]
+0.1% +2.96Ki  [ = ]       0    .debug_str
+0.0%    +292  [ = ]       0    .strtab
  +1.9%      +1  [ = ]       0    UavcanSensorBridgeBase::get_channel_for_node()
   +46%    +291  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
+0.0%     +64  [ = ]       0    .symtab
 -50.0%     -16  [ = ]       0    UavcanAccelBridge::imu_sub_cb()
   +33%     +16  [ = ]       0    UavcanAccelBridge::init_driver()
 -50.0%     -16  [ = ]       0    UavcanGyroBridge::imu_sub_cb()
   +33%     +16  [ = ]       0    UavcanGyroBridge::init_driver()
   +33%     +16  [ = ]       0    UavcanRangefinderBridge::init_driver()
 -50.0%     -16  [ = ]       0    UavcanRangefinderBridge::range_sub_cb()
 -33.3%     -16  [ = ]       0    sensors::VehicleGPSPosition::ParametersUpdate()
  +100%     +16  [ = ]       0    sensors::VehicleGPSPosition::Start()
 -20.0%     -16  [ = ]       0    uavcan::DynamicArrayBase<>::validateRange()
   +62%     +80  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
 +90% +3.45Ki  [ = ]       0    [Unmapped]
+0.0% +21.7Ki  +0.0%    +560    TOTAL

Updated: 2026-01-07T09:31:08

[MaEtUgR]

Why aren't all GPS instances published as they are available and the sensor module selects like how it is done for every sensor and GNSS modules until now? This pr scrambles the responsibility of selecting the primary GNSS module over sensors module and UAVCAN driver by making assumptions about the uORB instance order which is not guaranteed depending on advertising order which can be seen from the lengthy error messages in the code.

I assume sensor_gps.device_id contains the UAVCAN node ID in some way and that's how e.g. magnetometers get prioritized. Why do it completely different for GNSS receivers?

[Claudio-Chies]

@MaEtUgR Thanks for the feedback and pointers, i've rewrote it into a much nicher implementation

[dakejahl]

Thanks for the fix. Going forward we should try to move away from using "instances" of things. We should instead favor device_id, and then we don't need to map between the two.

[MaEtUgR]

Much much nicer!! Thanks for trying out the suggested architecture 🙏 🙏
The only thing I'd still improve is to only read the parameter and set the primary GPS in one place. It's now not only on parameter changes that the function setPrimaryInstance() can get called and it just increases complexity to do that in two places making assumptions about the range. But everything is at a much higher level now 🙏

[oystub]

Btw, your PR also fixes this old bug that I have forgotten to follow up 👍

BUG: When using multiple DroneCAN GNSS sensors, they all get the same device id, as the id is stored in a variable shared by all sensor_gps channels.

#22880

[oystub]

We should probably also clearly specify the behavior if the parameter is set to an ID that isn't connected to the system. For example if a GNSS receiver is swapped so that there are two instances, but none match the specified id. I would believe this to be a pretty common situation.

Ideally, this should at least provide some sort of warning?

[github-actions]

No flaws found

[dakejahl]

@Claudio-Chies looks like you've got some of the latest commits from main tangled up in here. For syncing with main I would suggest using the rebase command git rebase main which will apply each of your commits onto main and prompt you to resolve merge conflicts per-commit. If you have a lot of commits on a branch this can quickly become totally ineffective. In that case you should first squash your commits into a single commit .using git reset HEAD~N where N is the number of commits and then git add . && git commit -m "blah". After you squash you can then run git rebase main and you will only have to resolve merge conflicts once. Alternatively you can do git merge main and avoid force pushing, this mixes your commits in with main but that's okay because we squash-and-merge anyway.

[Claudio-Chies]

@dakejahl yea i hate the update branch functionality from the WebUI, it tends to mess things up afterwards.
should be clean now

[Claudio-Chies]

@dakejahl should be ready to merge now, had to fix a casting issue.
i agree, purely device_id might be nicer in the long run, but lets leave it at this for the moment