feat(mavlink): Battery_Status_V2 MAVLink stream

[Sayshara]

Solved Problem

Initial implementation for new mavlink message stream BATTERY_STATUS_V2.
It's base on BatteryStatus.msg, in a future we need to split this msg according to:
mavlink/rfcs#19
dronecan/DSDL#7
mavlink/mavlink#1846

Best to use similar structure to dsdl format of the battery messages (20004.BatteryInfoAux.uavcan, 20010.BatteryContinuous.uavcan, 20011.BatteryPeriodic.uavcan, 20012.BatteryCells.uavcan). Another question is why this is on ardupilot side, not common dsdl.

Solution

• added new stream for mavlink.

Changelog Entry

Alternatives

• On MAVLink side still missing kind of BATTERY_CELL_VOLTAGES.

Test coverage

Context

[hamishwillee]

@MaEtUgR What's the normal way we handle the case where we are migrating between messages for the MAVLINK_MODE_NORMAL case (where it is OTA).

My guess is that we'd initially stream both like this, but we might also update QGC to request the message type it wants and set the other not to stream?

[hamishwillee]

Did you test against QGC, and does QGC implement this?
If not, we might be able to make changes if needed.

---

Best to use similar structure to dsdl format of the battery messages (20004.BatteryInfoAux.uavcan, 20010.BatteryContinuous.uavcan, 20011.BatteryPeriodic.uavcan, 20012.BatteryCells.uavcan). Another question is why this is on ardupilot side, not common dsdl.

FWIW

We try match the most common definitions. I looked at all of them when we designed this message.

One thing I would consider is modifying this more like https://mavlink.io/en/messages/common.html#FUEL_STATUS
Specifically, it does a better job of reporting the level

The reported consumed_fuel and remaining_fuel must only be supplied if measured: they must not be inferred from the maximum_fuel and the other value. A recipient can assume that if these fields are supplied they are accurate. If not provided, the recipient can infer remaining_fuel from maximum_fuel and consumed_fuel on the assumption that the fuel was initially at its maximum (this is what battery monitors assume). Note however that this is an assumption, and the UI should prompt the user appropriately (i.e. notify user that they should fill the tank before boot).

maximum_fuel	float			Capacity when full. Must be provided.
consumed_fuel	float		invalid:NaN	Consumed fuel (measured). This value should not be inferred: if not measured set to NaN. NaN: field not provided.
remaining_fuel	float		invalid:NaN	Remaining fuel until empty (measured). The value should not be inferred: if not measured set to NaN. NaN: field not provided.

In other words the recipient knows what is measured and what is inferred in all cases and can tell that it is working with a system that trusts the values it outputs. This is better than what the battery message does, which is assume the battery was initially full, and use the consumed charge or battery voltage to work out what was consumed - then estimate the remaining.

Not sure we can though, because the approach comes from power modules, and I'm not sure we can "know" internally or from other sources what was calculated and what is measured.

[Sayshara]

Did you test against QGC, and does QGC implement this?
If not, we might be able to make changes if needed.

Tested, but Battery_Status_V2 is not implemented in QGC.

One thing I would consider is modifying this more like https://mavlink.io/en/messages/common.html#FUEL_STATUS
...

Changed the way of calculating capacity_remaining, now is based on remaining_capacity_wh/nominal_voltage. I'm not sure if this is the way which we should use, before was just remaining * capacity.

[hamishwillee]

Changed the way of calculating capacity_remaining, now is based on remaining_capacity_wh/nominal_voltage. I'm not sure if this is the way which we should use, before was just remaining * capacity.

Sorry, that was a side-note. This should match what it says on the mavlink spec, and we need to discuss with others if we want to modify the spec.

I commented because the way that the fuel spec does it works well because you can measure fuel in different ways. It needs more thinking to make a change like this for batteries though, since so many things assume certain behaviour. Upshot, revert please if the change doesn't match the spec.

[Sayshara]

Changed the way of calculating capacity_remaining, now is based on remaining_capacity_wh/nominal_voltage. I'm not sure if this is the way which we should use, before was just remaining * capacity.

Sorry, that was a side-note. This should match what it says on the mavlink spec, and we need to discuss with others if we want to modify the spec.

I commented because the way that the fuel spec does it works well because you can measure fuel in different ways. It needs more thinking to make a change like this for batteries though, since so many things assume certain behaviour. Upshot, revert please if the change doesn't match the spec.

I think after that change it's directly converts actual remaining energy to charge capacity using basic electrical relationship: https://www.inchcalculator.com/wh-to-ah-calculator/. The remaining * capacity approach would be more appropriate when only percentage-based remaining capacity is available. It's still according to mavlink spec.

Of course we could add different behavior depends on MAV_BATTERY_STATUS_FLAGS_CAPACITY_RELATIVE_TO_FULL, but this flag is not available yet in the code. Next PR I will add BatteryContinous/Periodic/Cells.

[hamishwillee]

1. There are lots of build errors. That's because MAVLINK_MSG_ID_BATTERY_STATUS_V2 is not being found. I think that is intended for non-SITL boards https://docs.px4.io/main/en/mavlink/#px4-and-mavlink but not sure how this is normally handled.

2. The changes look good to me, but I'm not a real programmer.

3. Of course we could add different behavior depends on MAV_BATTERY_STATUS_FLAGS_CAPACITY_RELATIVE_TO_FULL, but this flag is not available yet in the code.

   I don't think we need to change the behaviour here to compensate for MAV_BATTERY_STATUS_FLAGS_CAPACITY_RELATIVE_TO_FULL. This is something a smart battery would set to allow a GCS to know that the value for remaining capacity can be trusted, and internally you might use this also.

4. Next PR I will add BatteryContinous/Periodic/Cells.
   I'm not sure what these are. But you shouldn't add anything for cells until the new mavlink message is fully defined (that needs to be completed in the RFC and signed off).

   What I would look at next is perhaps moving some of the fields in https://docs.px4.io/main/en/msg_docs/BatteryStatus.html (the dynamic topic) to https://docs.px4.io/main/en/msg_docs/BatteryInfo.html (the static info) - at the moment the battery status has most of the fields.
   This would require updates to all the battery messages and the drivers that set the values.

   When you have a plan, consult @MaEtUgR to confirm he is happy with it please.

   Noe, I've pinged @MaEtUgR to look at this - he is currently on holiday though.

[farhangnaderi]

@Sayshara would you be able to continue on this PR?

[Sayshara]

@Sayshara would you be able to continue on this PR?

Yes, it's just waiting for review.

[hamishwillee]

@Sayshara I've asked @MaEtUgR for a review, but can you fix the build errors first.

[Sayshara]

@Sayshara I've asked @MaEtUgR for a review, but can you fix the build errors first.

Fixed, the only ones failing is flash usage for mavlink-dev.px4board and SITL tests with MAVSDK.

[github-actions]

This pull request has been automatically closed due to 120 days of inactivity. If you would like to continue, please feel free to reopen it or submit a new PR.

[dakejahl]

Should we get this in? Is there handling in QGC? We don't want to publish both messages, so we'll need to find a good solution for the migration... I don't know how we do this while keeping backwards compat.

[hamishwillee]

Should we get this in? Is there handling in QGC? We don't want to publish both messages, so we'll need to find a good solution for the migration... I don't know how we do this while keeping backwards compat.

@dakejahl We absolutely should get support for BATTERY_STATUS_V2. I believe that MAVSDK has support, but QGC does yet.

This was pending review and an answer on https://github.com/PX4/PX4-Autopilot/pull/25347/changes#r2255643302 - which was exactly your question "how do we manage the transition".

I don't know the "official patterns". My thinking is that QGC should request BATTERY_STATUS_V2 to be streamed if it isn't being received. If that succeeds it sould send a command to stop BATTERY_STATUS being streamed.
Once BATTERY_STATUS_V2 is supported by enough versions of QGC, PX4 could stream it by default.

[hamishwillee]

FYI, I have updated this in line with my suggestion - removing streaming by default. I also fixed up an indexing error in the battery id. Claude seems to think this is good, and so do I by inspection.

I have created mavlink/qgroundcontrol#14269 to request the new message if it is not detected, and display it. If you build this with make px4_sitl_mavlink-dev gz_x500 it works with that PR.

Note though, that PX4 only sends BATTERY_INFO if a serial number is set, even if it is requested. So I can't test the full path with PX4 of a smart battery.

[hamishwillee]

@dakejahl @MaEtUgR Can you review this? I have looked at it, checked it with claude, and tested it via QGC in mavlink/qgroundcontrol#14269 - see #25347 (comment) above

[dakejahl]

Not approving so @MaEtUgR can sign off, but nothing here looks merge-blocking. +1 on the stop-streaming-by-default migration via mavlink/qgroundcontrol#14269 — keeps the link budget sane and lets V2 land ahead of broad GCS support.

Three small inline nits on the V2 stream; all non-blocking.

[dakejahl]

Nit (same pattern on lines 82, 88): fabsf(x - (-1.0f)) > FLT_EPSILON is an unusual way to spell x != -1.0f for a deliberately set sentinel. FLT_EPSILON is ~1.19e-7, so this is effectively exact-equality — works, just reads strangely. x != -1.0f or x > -0.5f would be clearer.

[dakejahl]

remaining_capacity_wh has no @invalid sentinel in BatteryStatus.msg (unlike voltage_v=0, current_a=-1), so a dumb power module that leaves it at 0 produces 0 Ah remaining instead of NaN. Same for nominal_voltage — the isZero guard protects against div-by-zero but not against "not measured." No regression vs. current behavior; worth revisiting when smart-battery fields get proper invalid sentinels, or by keying validity off source == SOURCE_EXTERNAL.

[dakejahl]

Minor: FAULT_CELL_FAIL reasonably maps to MAV_BATTERY_STATUS_FLAGS_BAD_BATTERY — same "battery unusable" indicator you already set for FAULT_HARDWARE_FAILURE. One-line addition.