Skip to content

feat(uavcan): support for new Continious, Periodic and Cell Battery messages - #27464

Open
Claudio-Chies wants to merge 4 commits into
mainfrom
pr-uavcan_battery
Open

feat(uavcan): support for new Continious, Periodic and Cell Battery messages#27464
Claudio-Chies wants to merge 4 commits into
mainfrom
pr-uavcan_battery

Conversation

@Claudio-Chies

@Claudio-Chies Claudio-Chies commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

Add UAVCAN bridge support for the multi-message smart-battery protocol (BatteryContinuous / BatteryPeriodic / BatteryCells) and a host-runnable gtest harness that exercises the UAVCAN→uORB path end-to-end through libuavcan's loopback transport.

Problem

PX4's UAVCAN battery bridge only understood the legacy BatteryInfo/BatteryInfoAux and CBAT messages. Newer style smart batteries split their data across a BatteryContinuous (high-rate) + BatteryPeriodic (low-rate) + BatteryCells trio, which the bridge ignored. Separately, the bridge had no automated test
coverage at all: verifying a change meant wiring real hardware on a CAN bus and watching listener battery_status. The driver is also gated on PLATFORM_NUTTX, so it isn't compiled in the px4_sitl_test build that make tests uses, leaving no path to test it in CI.

Solution

Add a BatteryDataType::Multi mode that claims a node on the first multi-message frame and suppresses the legacy BatteryInfo/BatteryInfoAux/CBAT handlers for that node to avoid double-publishing.

  • BatteryContinuous
    maps voltage/current/SoC/temperature/fault flags and charging state;

  • BatteryPeriodic
    maps cell count,
    nominal voltage, capacity (with full_charge_capacitydesign_capacityBAT${i}_CAPACITY param fallback), cycle count, SoH and serial number into battery_info;

  • BatteryCells
    fills per-cell voltages and max_cell_voltage_delta. The delta computation is extracted into a reusable static

For testing, a BUILD_TESTING-gated standalone CMake target (src/drivers/uavcan/sensors/test/) compiles libuavcan core + DSDL + the bridge sources without CONFIG_DRIVERS_UAVCAN, so the bridge links into a functional gtest. A generic UavcanBridgeTestFixture spins N loopback-paired libuavcan nodes — index 0 hosts the
bridge, the rest publish synthetic frames — and is reusable for other UAVCAN sensor bridges. BatteryBridgeTest covers field mapping, battery_info serial-number/id propagation, multi-instance separation, capacity param fallback, and edge cases (zero nominal voltage → NaN, missing capacity, UINT*_MAX "not provided"
sentinels, charging/fault flag handling). Runs under the existing make tests / checks.yml job — no new CI workflow.

Related PR

@Claudio-Chies
Claudio-Chies requested a review from dakejahl May 26, 2026 11:22

@dakejahl dakejahl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add some instructions to your CLAUDE.md for prettier PR descriptions?

This is what I use, maybe we should add this to the PX4 project CLAUDE.md

CLAUDE.md

@github-actions github-actions Bot added kind:feature Request or change that adds new functionality. kind:test Adds or improves tests. scope:build-system CMake, Kconfig, board config, or build tooling. scope:drivers Device drivers and hardware interfaces. scope:middleware DDS, ROS 2, Cyphal/UAVCAN, zenoh, or bridge layers. scope:testing Unit, integration, fuzzing, or test data. labels May 27, 2026
@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 5352 byte (0.25 %)]
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.3% +5.10Ki  +0.3% +5.10Ki    .text
     +84% +1.35Ki   +84% +1.35Ki    uavcan::GenericSubscriber<>::TransferForwarder::handleIncomingTransfer()
    [NEW]    +524  [NEW]    +524    UavcanBatteryBridge::battery_periodic_sub_cb()
    [NEW]    +496  [NEW]    +496    UavcanBatteryBridge::battery_continuous_sub_cb()
    +7.8%    +420  +7.8%    +420    uavcan::GenericSubscriber<>::checkInit()
    +4.7%    +384  +4.7%    +384    uavcan::GlobalDataTypeRegistry::registerDataType<>()
     +11%    +306   +11%    +306    uavcan::GenericSubscriber<>::~GenericSubscriber()
     +75%    +272   +75%    +272    UavcanBatteryBridge::init()
     +46%    +220   +46%    +220    UavcanBatteryBridge::UavcanBatteryBridge()
    +8.8%    +168  +8.8%    +168    uavcan::Subscriber<>::handleReceivedDataStruct()
    [NEW]    +156  [NEW]    +156    UavcanBatteryBridge::battery_cells_sub_cb()
    +0.1%    +132  +0.1%    +132    [section .text]
    +7.7%    +120  +7.7%    +120    uavcan::GenericSubscriber<>::TransferForwarder::~TransferForwarder()
    +8.6%    +114  +8.6%    +114    uavcan::Subscriber<>::~Subscriber()
    [NEW]    +100  [NEW]    +100    Battery::computeMaxCellVoltageDelta()
    +7.5%     +72  +7.5%     +72    uavcan::GenericSubscriber<>::TransferForwarder
    +5.7%     +72  +5.7%     +72    uavcan::MethodBinder<>::operator bool()
    +7.5%     +60  +7.5%     +60    uavcan::GenericSubscriber<>
    +8.8%     +60  +8.8%     +60    uavcan::Subscriber<>
   -99.4%     +58 -99.4%     +58    [10 Others]
     +11%     +56   +11%     +56    _GLOBAL__sub_I__ZN19IUavcanSensorBridge8make_allERN6uavcan5INodeER4ListIPS_EP17NodeInfoPublisher
     +32%     +48   +32%     +48    _GLOBAL__sub_I__ZN19UavcanBatteryBridge4NAMEE
  [ = ]       0  +0.1%    +128    .bss
    [ = ]       0  +5.4%    +120    uavcan::GlobalDataTypeRegistry::registerDataType<>()::entry
    [ = ]       0   +67%      +8    _bdshot_channel_mask
  +0.0%    +136  [ = ]       0    .debug_abbrev
  +0.2%    +360  [ = ]       0    .debug_aranges
  +0.3% +1.32Ki  [ = ]       0    .debug_frame
  +0.2% +66.0Ki  [ = ]       0    .debug_info
  +0.2% +10.9Ki  [ = ]       0    .debug_line
   -66.7%      -2  [ = ]       0    [Unmapped]
    +0.2% +10.9Ki  [ = ]       0    [section .debug_line]
  +0.4% +16.4Ki  [ = ]       0    .debug_loclists
  +0.4% +2.64Ki  [ = ]       0    .debug_rnglists
     +50%      +1  [ = ]       0    [Unmapped]
    +0.4% +2.64Ki  [ = ]       0    [section .debug_rnglists]
  +1.7% +58.5Ki  [ = ]       0    .debug_str
  +0.4%      +1  [ = ]       0    .shstrtab
  +1.3% +8.60Ki  [ = ]       0    .strtab
    [NEW]     +45  [ = ]       0    Battery::computeMaxCellVoltageDelta()
    [NEW]    +133  [ = ]       0    UavcanBatteryBridge::battery_cells_sub_cb()
    [NEW]    +143  [ = ]       0    UavcanBatteryBridge::battery_continuous_sub_cb()
    [NEW]    +139  [ = ]       0    UavcanBatteryBridge::battery_periodic_sub_cb()
     +15%     +16  [ = ]       0    ___ZN39ControlAllocationSequentialDesaturation23computeDesaturationGainERKN6matrix6VectorIfLj16EEES4__veneer
   -18.0%     -16  [ = ]       0    ___ZN7sensors22VehicleAngularVelocity21FilterAngularVelocityEiPfi_veneer
    +8.0%    +335  [ = ]       0    uavcan::GenericSubscriber<>
    +7.9%    +392  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder
    +7.8%    +527  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::handleIncomingTransfer()
    +7.9% +1.16Ki  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::~TransferForwarder()
    +8.0%    +362  [ = ]       0    uavcan::GenericSubscriber<>::checkInit()
    +8.0%   +1014  [ = ]       0    uavcan::GenericSubscriber<>::~GenericSubscriber()
    +5.9%    +446  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()
    +5.8%    +946  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()::entry
    +6.3%    +434  [ = ]       0    uavcan::MethodBinder<>::operator bool()
     +18%    +169  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
    +9.3%    +488  [ = ]       0    uavcan::Subscriber<>
    +9.2%    +572  [ = ]       0    uavcan::Subscriber<>::handleReceivedDataStruct()
    +9.3% +1.44Ki  [ = ]       0    uavcan::Subscriber<>::~Subscriber()
  +0.4% +2.12Ki  [ = ]       0    .symtab
    [NEW]     +48  [ = ]       0    Battery::computeMaxCellVoltageDelta()
    [NEW]     +32  [ = ]       0    UavcanBatteryBridge::battery_cells_sub_cb()
    [NEW]     +48  [ = ]       0    UavcanBatteryBridge::battery_continuous_sub_cb()
    [NEW]     +48  [ = ]       0    UavcanBatteryBridge::battery_periodic_sub_cb()
   -91.1%    +560  [ = ]       0    [4 Others]
     +67%     +32  [ = ]       0    ___ZN39ControlAllocationSequentialDesaturation23computeDesaturationGainERKN6matrix6VectorIfLj16EEES4__veneer
   -25.0%     -16  [ = ]       0    ___ZN4uORB7Manager17get_device_masterEv_veneer
   -40.0%     -32  [ = ]       0    ___ZN7sensors22VehicleAngularVelocity21FilterAngularVelocityEiPfi_veneer
     +33%     +16  [ = ]       0    __net_unlock_veneer
   -33.3%     -16  [ = ]       0    __nxsig_pendingset_veneer
     +50%     +16  [ = ]       0    __pthread_mutex_unlock_veneer
    +7.5%     +96  [ = ]       0    uavcan::GenericSubscriber<>
    +7.5%     +96  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder
    +6.9%     +80  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::handleIncomingTransfer()
    +7.5%    +288  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::~TransferForwarder()
    +9.1%    +192  [ = ]       0    uavcan::GenericSubscriber<>::checkInit()
    +1.5%     +16  [ = ]       0    uavcan::GenericSubscriber<>::handleIncomingTransfer()
    +7.4%    +288  [ = ]       0    uavcan::GenericSubscriber<>::~GenericSubscriber()
    +5.3%    +144  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()
    +5.3%    +192  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()::entry
    +4.1%     +48  [ = ]       0    uavcan::MethodBinder<>::operator bool()
 -10.0% -1.10Ki  [ = ]       0    [Unmapped]
  +0.4%  +170Ki  +0.3% +5.23Ki    TOTAL

px4_fmu-v6x [Total VM Diff: 5376 byte (0.26 %)]
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.3% +5.12Ki  +0.3% +5.12Ki    .text
     +84% +1.35Ki   +84% +1.35Ki    uavcan::GenericSubscriber<>::TransferForwarder::handleIncomingTransfer()
    [NEW]    +524  [NEW]    +524    UavcanBatteryBridge::battery_periodic_sub_cb()
    [NEW]    +496  [NEW]    +496    UavcanBatteryBridge::battery_continuous_sub_cb()
    +7.8%    +420  +7.8%    +420    uavcan::GenericSubscriber<>::checkInit()
    +4.7%    +384  +4.7%    +384    uavcan::GlobalDataTypeRegistry::registerDataType<>()
     +11%    +306   +11%    +306    uavcan::GenericSubscriber<>::~GenericSubscriber()
     +75%    +272   +75%    +272    UavcanBatteryBridge::init()
     +46%    +220   +46%    +220    UavcanBatteryBridge::UavcanBatteryBridge()
    +8.8%    +168  +8.8%    +168    uavcan::Subscriber<>::handleReceivedDataStruct()
    [NEW]    +156  [NEW]    +156    UavcanBatteryBridge::battery_cells_sub_cb()
    +0.1%    +144  +0.1%    +144    [section .text]
    +7.7%    +120  +7.7%    +120    uavcan::GenericSubscriber<>::TransferForwarder::~TransferForwarder()
    +8.6%    +114  +8.6%    +114    uavcan::Subscriber<>::~Subscriber()
    [NEW]    +100  [NEW]    +100    Battery::computeMaxCellVoltageDelta()
    +7.5%     +72  +7.5%     +72    uavcan::GenericSubscriber<>::TransferForwarder
    +5.7%     +72  +5.7%     +72    uavcan::MethodBinder<>::operator bool()
   -99.3%     +70 -99.3%     +70    [9 Others]
    +7.5%     +60  +7.5%     +60    uavcan::GenericSubscriber<>
    +8.8%     +60  +8.8%     +60    uavcan::Subscriber<>
     +11%     +56   +11%     +56    _GLOBAL__sub_I__ZN19IUavcanSensorBridge8make_allERN6uavcan5INodeER4ListIPS_EP17NodeInfoPublisher
     +32%     +48   +32%     +48    _GLOBAL__sub_I__ZN19UavcanBatteryBridge4NAMEE
  [ = ]       0  +0.1%    +128    .bss
    [ = ]       0  +5.4%    +120    uavcan::GlobalDataTypeRegistry::registerDataType<>()::entry
    [ = ]       0   +12%      +8    [section .bss]
  +0.0%    +136  [ = ]       0    .debug_abbrev
  +0.2%    +360  [ = ]       0    .debug_aranges
  +0.3% +1.32Ki  [ = ]       0    .debug_frame
  +0.2% +65.8Ki  [ = ]       0    .debug_info
  +0.2% +10.9Ki  [ = ]       0    .debug_line
    +100%      +3  [ = ]       0    [Unmapped]
    +0.2% +10.9Ki  [ = ]       0    [section .debug_line]
  +0.5% +16.4Ki  [ = ]       0    .debug_loclists
  +0.5% +2.63Ki  [ = ]       0    .debug_rnglists
   -33.3%      -1  [ = ]       0    [Unmapped]
    +0.5% +2.64Ki  [ = ]       0    [section .debug_rnglists]
  +1.7% +58.5Ki  [ = ]       0    .debug_str
  +0.4%      +1  [ = ]       0    .shstrtab
  +1.3% +8.60Ki  [ = ]       0    .strtab
    [NEW]     +45  [ = ]       0    Battery::computeMaxCellVoltageDelta()
    [NEW]    +133  [ = ]       0    UavcanBatteryBridge::battery_cells_sub_cb()
    [NEW]    +143  [ = ]       0    UavcanBatteryBridge::battery_continuous_sub_cb()
    [NEW]    +139  [ = ]       0    UavcanBatteryBridge::battery_periodic_sub_cb()
    +8.0%    +335  [ = ]       0    uavcan::GenericSubscriber<>
    +7.9%    +392  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder
    +7.8%    +527  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::handleIncomingTransfer()
    +7.9% +1.16Ki  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::~TransferForwarder()
    +8.0%    +362  [ = ]       0    uavcan::GenericSubscriber<>::checkInit()
    +8.0%   +1014  [ = ]       0    uavcan::GenericSubscriber<>::~GenericSubscriber()
    +5.9%    +446  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()
    +5.8%    +946  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()::entry
    +6.3%    +434  [ = ]       0    uavcan::MethodBinder<>::operator bool()
     +18%    +169  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
    +9.3%    +488  [ = ]       0    uavcan::Subscriber<>
    +9.2%    +572  [ = ]       0    uavcan::Subscriber<>::handleReceivedDataStruct()
    +9.3% +1.44Ki  [ = ]       0    uavcan::Subscriber<>::~Subscriber()
  +0.4% +2.12Ki  [ = ]       0    .symtab
    [NEW]     +48  [ = ]       0    Battery::computeMaxCellVoltageDelta()
    [NEW]     +32  [ = ]       0    UavcanBatteryBridge::battery_cells_sub_cb()
    [NEW]     +48  [ = ]       0    UavcanBatteryBridge::battery_continuous_sub_cb()
    [NEW]     +48  [ = ]       0    UavcanBatteryBridge::battery_periodic_sub_cb()
    +7.5%     +96  [ = ]       0    uavcan::GenericSubscriber<>
    +7.5%     +96  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder
    +6.9%     +80  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::handleIncomingTransfer()
    +7.5%    +288  [ = ]       0    uavcan::GenericSubscriber<>::TransferForwarder::~TransferForwarder()
    +9.1%    +192  [ = ]       0    uavcan::GenericSubscriber<>::checkInit()
    +1.5%     +16  [ = ]       0    uavcan::GenericSubscriber<>::handleIncomingTransfer()
    +7.4%    +288  [ = ]       0    uavcan::GenericSubscriber<>::~GenericSubscriber()
    +5.3%    +144  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()
    +5.3%    +192  [ = ]       0    uavcan::GlobalDataTypeRegistry::registerDataType<>()::entry
    +4.1%     +48  [ = ]       0    uavcan::MethodBinder<>::operator bool()
     +15%     +32  [ = ]       0    uavcan::ReceivedDataStructure<>::safeget<>()
    +8.8%     +96  [ = ]       0    uavcan::Subscriber<>
    +8.4%    +144  [ = ]       0    uavcan::Subscriber<>::handleReceivedDataStruct()
    +8.8%    +288  [ = ]       0    uavcan::Subscriber<>::~Subscriber()
   +73% +2.88Ki  [ = ]       0    [Unmapped]
  +0.4%  +174Ki  +0.3% +5.25Ki    TOTAL

Updated: 2026-06-02T09:25:06

@hamishwillee hamishwillee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that MAVLink follows the same model - BATTERY_INFO is the slow message, BATTERY_STATUS_V2 is the fast message - planned to go in very soon #25347

The battery RFC stripped out the cell information message from BATTERY_STATUS when inventing BATTERY_STATUS_V2 as "diagnostic" - there's a proposal that it be retained in a separate message, but no one has defined its form.

EDIT, I guess my point is that it is worth looking at the form of the messages because BATTERY_STATUS_V2 can still change, and if there are significant differences we should look at them now.

@Claudio-Chies

Copy link
Copy Markdown
Member Author

@dakejahl i've also added CI tests for the can battery driver. open for discussion if this adds too much clutter, which i guess it might be the case.

@dakejahl dakejahl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we're going to add tests shouldn't they cover the callbacks? Maybe not necessary or that useful for this module though 🤷

@Claudio-Chies

Copy link
Copy Markdown
Member Author

@dakejahl which callbacks do you mean? i am testing the callbacks of the three messages which i'm implementing by comparing against the uorb messages.

@Claudio-Chies
Claudio-Chies marked this pull request as ready for review June 1, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:feature Request or change that adds new functionality. kind:test Adds or improves tests. scope:build-system CMake, Kconfig, board config, or build tooling. scope:drivers Device drivers and hardware interfaces. scope:middleware DDS, ROS 2, Cyphal/UAVCAN, zenoh, or bridge layers. scope:testing Unit, integration, fuzzing, or test data.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants