Skip to content

Conversation

@Claudio-Chies
Copy link
Member

@Claudio-Chies Claudio-Chies commented Dec 17, 2025

Fixes device ID handling in UAVCAN bridges.

Solution

Refactor device ID logic across multiple UAVCAN sensor bridges to use a unified method for generating device IDs.

Changelog Entry

Bugfix: Improved device ID logic for UAVCAN sensors.

Test coverage

@oystub
Copy link
Contributor

oystub commented Dec 17, 2025

Nice, I like doing this properly!

However, note that some of the bridges that haven't been modified still leave the bus id as 0, for example rangefinder. Up until now, the "convention" has been to just leave bus id as 0 for everything DroneCAN. If we change that, we should be careful to update it everywhere.

There is also some discussion if we actually want to separate DroneCAN buses, or treat them as as one large network. (Are two devices with the same DroneCAN ID on two different buses considered the "same" device, or two different ones?)

I did something similar in an old draft, but haven't followed it up properly:
#22880

@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 17, 2025

This pr is related to #26126

@oystub Nice, thanks for chiming in. Let's combine these two changes and bring in consistent Device IDs for UAVCAN 👍

@Claudio-Chies
Copy link
Member Author

Took inspiration from #22880, and expanded it to more drivers, thanks @oystub

@dakejahl
Copy link
Contributor

@Claudio-Chies looks like there are some merge conflicts. Also try to avoid force pushing after review has started (if possible), it makes it difficult for reviewers to see what has changed between commits.

Copy link
Contributor

@dakejahl dakejahl left a comment

Choose a reason for hiding this comment

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

Looking good! Stoked about this, thank you!

@dakejahl
Copy link
Contributor

dakejahl commented Jan 5, 2026

CI failure is real, otherwise LGTM

@github-actions
Copy link

github-actions bot commented Jan 6, 2026

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 376 byte (0.02 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +376  +0.0%    +376    .text
 -97.9%     +60 -97.9%     +60    [17 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()
   +13%     +28   +13%     +28    UavcanBarometerBridge::air_pressure_sub_cb()
   +11%     +20   +11%     +20    UavcanDifferentialPressureBridge::air_sub_cb()
   +12%     +16   +12%     +16    UavcanAccelBridge::init_driver()
  +7.1%     +16  +7.1%     +16    UavcanFlowBridge::flow_sub_cb()
  +1.0%     +16  +1.0%     +16    UavcanGnssBridge::process_fixx<>()
   +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()
  +5.4%      +8  +5.4%      +8    UavcanHygrometerBridge::UavcanHygrometerBridge()
  +4.3%      +8  +4.3%      +8    UavcanMagnetometerBridge::UavcanMagnetometerBridge()
  -0.0%     -12  -0.0%     -12    [section .text]
+0.0%     +57  [ = ]       0    .debug_abbrev
+0.0%     +12  [ = ]       0    .debug_frame
+0.0% +7.92Ki  [ = ]       0    .debug_info
+0.0% +1.92Ki  [ = ]       0    .debug_line
  +500%      +5  [ = ]       0    [Unmapped]
  +0.0% +1.91Ki  [ = ]       0    [section .debug_line]
+0.0% +1.73Ki  [ = ]       0    .debug_loclists
+0.1%    +605  [ = ]       0    .debug_rnglists
  [NEW]      +3  [ = ]       0    [Unmapped]
  +0.1%    +602  [ = ]       0    [section .debug_rnglists]
+0.1% +2.96Ki  [ = ]       0    .debug_str
-0.4%      -1  [ = ]       0    .shstrtab
+0.0%      +1  [ = ]       0    .strtab
  +1.9%      +1  [ = ]       0    UavcanSensorBridgeBase::get_channel_for_node()
  +100%     +16  [ = ]       0    __memset_veneer
 -41.0%     -16  [ = ]       0    __nxsem_trywait_veneer
-3.8%    -376  [ = ]       0    [Unmapped]
+0.0% +15.2Ki  +0.0%    +376    TOTAL

px4_fmu-v6x [Total VM Diff: 384 byte (0.02 %)]
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%    +384  +0.0%    +384    .text
 -97.6%     +68 -97.6%     +68    [17 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()
   +13%     +28   +13%     +28    UavcanBarometerBridge::air_pressure_sub_cb()
   +11%     +20   +11%     +20    UavcanDifferentialPressureBridge::air_sub_cb()
   +12%     +16   +12%     +16    UavcanAccelBridge::init_driver()
  +7.1%     +16  +7.1%     +16    UavcanFlowBridge::flow_sub_cb()
  +1.0%     +16  +1.0%     +16    UavcanGnssBridge::process_fixx<>()
   +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()
  +5.4%      +8  +5.4%      +8    UavcanHygrometerBridge::UavcanHygrometerBridge()
  +4.3%      +8  +4.3%      +8    UavcanMagnetometerBridge::UavcanMagnetometerBridge()
  -0.0%     -12  -0.0%     -12    [section .text]
+0.0%     +57  [ = ]       0    .debug_abbrev
+0.0%     +12  [ = ]       0    .debug_frame
+0.0% +7.92Ki  [ = ]       0    .debug_info
+0.0% +1.92Ki  [ = ]       0    .debug_line
  +500%      +5  [ = ]       0    [Unmapped]
  +0.0% +1.91Ki  [ = ]       0    [section .debug_line]
+0.0% +1.69Ki  [ = ]       0    .debug_loclists
+0.1%    +602  [ = ]       0    .debug_rnglists
+0.1% +2.96Ki  [ = ]       0    .debug_str
+1.3%      +3  [ = ]       0    .shstrtab
+0.0%      +1  [ = ]       0    .strtab
  +1.9%      +1  [ = ]       0    UavcanSensorBridgeBase::get_channel_for_node()
-4.9%    -384  [ = ]       0    [Unmapped]
+0.0% +15.1Ki  +0.0%    +384    TOTAL

Updated: 2026-01-06T18:13:22

@dakejahl
Copy link
Contributor

dakejahl commented Jan 6, 2026

I was about to hit merge, noticed a merge conflict and fixed it, but then noticed the target branch isn't main
image

@Claudio-Chies
Copy link
Member Author

@dakejahl yea this was intended, as i needed to get the other PR over the line faster than this, and these changes were dependent on the other changes.
when the targeted PR gets merged into main, the target branch of this changes to main aswell.

@Claudio-Chies Claudio-Chies merged commit 8e3881a into pr-uavcan_gnss_prio Jan 7, 2026
68 of 70 checks passed
@Claudio-Chies Claudio-Chies deleted the pr-uavcan_dev_id branch January 7, 2026 09:23
dakejahl added a commit that referenced this pull request Jan 7, 2026
* UAVCAN: extent SENS_GPS_PRIME usage to UAVCAN GNSS devices

* use convenience function

Co-authored-by: Matthias Grob <[email protected]>

* Update src/drivers/uavcan/sensors/gnss.cpp

Co-authored-by: Øyvind Taksdal Stubhaug <[email protected]>

* Apply suggestion from @MaEtUgR

Co-authored-by: Matthias Grob <[email protected]>

* Fix type casting in GPS prime range check

* reverted parameter default

* UAVCAN: fix and improve device_id logic (#26135)

* UAVCAN: extent SENS_GPS_PRIME usage to UAVCAN GNSS devices

* use convenience function

Co-authored-by: Matthias Grob <[email protected]>

* Update src/drivers/uavcan/sensors/gnss.cpp

Co-authored-by: Øyvind Taksdal Stubhaug <[email protected]>

* Apply suggestion from @MaEtUgR

Co-authored-by: Matthias Grob <[email protected]>

* Fix type casting in GPS prime range check

* UAVCAN: fix and improve device_id logic

* Added bus information to more UAVCAN drivers

* Fix device_id registration in UavcanBarometerBridge

---------

Co-authored-by: Matthias Grob <[email protected]>
Co-authored-by: Øyvind Taksdal Stubhaug <[email protected]>
Co-authored-by: Jacob Dahl <[email protected]>

---------

Co-authored-by: Matthias Grob <[email protected]>
Co-authored-by: Øyvind Taksdal Stubhaug <[email protected]>
Co-authored-by: Jacob Dahl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants