feature(battery): BATTERY_STATUS_V2 integration#14269
Open
hamishwillee wants to merge 13 commits into
Open
Conversation
hamishwillee
changed the title
Contributor
Build ResultsPlatform Status
Some builds failed. Pre-commit
Pre-commit hooks: 4 passed, 36 failed, 7 skipped. Artifact Sizes
Updated: 2026-04-13 04:46:25 UTC • Triggered by: Android |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for MAVLink BATTERY_STATUS_V2 and BATTERY_INFO in QGroundControl’s vehicle battery Fact model, including a post-connect negotiation to request the new streams and (optionally) suppress legacy BATTERY_STATUS. Also extends camera battery reporting to accept BATTERY_STATUS_V2.
Changes:
- Add
BATTERY_STATUS_V2handling inVehicle(message dispatch + charge-state announcement mapping). - Extend
BatteryFactGroup/JSON metadata to expose all new V2/INFO fields as Facts and add V2 negotiation logic. - Add
BATTERY_STATUS_V2plumbing throughQGCCameraManager→ camera control interfaces/implementations.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Vehicle/Vehicle.h | Declares new V2 battery handlers and shared charge-state announcement helper. |
| src/Vehicle/Vehicle.cc | Dispatches BATTERY_STATUS_V2, triggers negotiation after initial connect, and maps V2 flags to charge-state for audio alerts. |
| src/Vehicle/FactGroups/BatteryFactGroupListModel.h | Adds V2 negotiation API/state and new battery Facts for V2/INFO fields. |
| src/Vehicle/FactGroups/BatteryFactGroupListModel.cc | Implements negotiation (request streams/disable V1) and parses BATTERY_STATUS_V2 + BATTERY_INFO into Facts. |
| src/Vehicle/FactGroups/BatteryFact.json | Adds metadata entries for the new Facts (capacity/status flags/identity/spec fields). |
| src/FactSystem/FactGroupListModel.h | Makes handleMessageForFactGroupCreation virtual to support model-specific interception. |
| src/Camera/VehicleCameraControl.h | Adds camera battery handler for BATTERY_STATUS_V2. |
| src/Camera/VehicleCameraControl.cc | Updates camera battery percentage from BATTERY_STATUS_V2. |
| src/Camera/SimulatedCameraControl.h | Implements the new V2 camera battery handler as a no-op. |
| src/Camera/QGCCameraManager.h | Declares MAVLink handler for BATTERY_STATUS_V2. |
| src/Camera/QGCCameraManager.cc | Routes BATTERY_STATUS_V2 to camera controls. |
| src/Camera/MavlinkCameraControlInterface.h | Extends the camera control interface with handleBatteryStatusV2. |
…egotiationVehicle BATTERY_STATUS_V2 status_flags has no bits for MAV_BATTERY_CHARGE_STATE_LOW or _CRITICAL, so once V2 is active the existing voice alerts and charge-state fact could never reach those states. Fix by falling back to a percent-based derivation using the BatteryIndicatorSettings thresholds (threshold1/threshold2) when no fault flag is set — applied both in Vehicle::_handleBatteryStatusV2 (audio alerts) and BatteryFactGroup::_handleBatteryStatusV2 (chargeState fact). Also removes the _negotiationVehicle member which was written in three places but never read (dead state, flagged by Copilot review r3061146854). Addresses Copilot review comments: r3061146802, r3061146835 — missing LOW/CRITICAL in V2 path r3061146854 — unused _negotiationVehicle field Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
threshold1()/threshold2() are non-const Fact* accessors; using 'const auto *' on the settings pointer caused a const-qualification mismatch caught by -Werror. Remove the const qualifier. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Used BatteryIndicatorSettings.threshold1/threshold2 (defaults 80%/60%) as LOW/CRITICAL alarm triggers, which would fire voice alerts at 79% and 59% remaining — far too early. Replace with conventional alarm thresholds (25% LOW, 10% CRITICAL) that sit below the visual OK sub-colour range and match industry norms. Remove the now-unused BatteryIndicatorSettings.h includes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…elper Three bugs fixed from Copilot re-review: 1. V2 activation was triggered by any component sending BATTERY_STATUS_V2, including camera peripherals. A camera on a different compid could cause QGC to suppress BATTERY_STATUS from the autopilot (which was never sending V2). Fix: gate _v2StatusReceived / _activateV2 on message.compid matching vehicle->defaultComponentId(). 2. QString::fromLatin1(bi.name) scanned for a null terminator past the MAVLink fixed-length field boundary if a sender omitted it. Fix: bound with strnlen(bi.name, sizeof(bi.name)) for all three BATTERY_INFO string fields. 3. The charge-state derivation (faultMask + percent thresholds) was duplicated identically in BatteryFactGroup and Vehicle, creating a maintenance risk. Extracted into BatteryFactGroup::chargeStateFromV2(statusFlags, percent). This also resolves the previously noted CHARGING/EMERGENCY asymmetry: CHARGING is applied on top of the helper result in the UI path only; the audio path intentionally omits it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Seeded _lowestBatteryChargeStateAnnouncedMap with chargeState on first encounter. MAV_BATTERY_CHARGE_STATE_CHARGING = 7 is numerically the largest enum value, so if the first message for a battery carried CHARGING, all subsequent LOW (2) / CRITICAL (3) / EMERGENCY (4) / FAILED (5) checks failed `chargeState > 7` permanently — voice alerts were silently suppressed for the lifetime of the connection unless an OK state was received in between. Fix: seed the map at MAV_BATTERY_CHARGE_STATE_OK (1) unconditionally, which also ensures that connecting to a vehicle already in LOW/CRITICAL fires an immediate alert rather than swallowing the first occurrence. Pre-existing bug on master; surfaced by Copilot review of the V2 path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ting state If the vehicle ACKs SET_MESSAGE_INTERVAL as UNSUPPORTED but later starts streaming BATTERY_STATUS_V2 (e.g. an always-on stream not controlled by SET_MESSAGE_INTERVAL), the old guard kept _v2State at Unsupported and _activateV2 was never called — both V1 and V2 would update the same facts simultaneously and V1 was never suppressed. Changing the guard to != V2NegotiationActive means V2 activates on the first frame from the autopilot component regardless of prior negotiation state. _activateV2 sets the state to Active as its first action, so subsequent frames never re-trigger it and SET_MESSAGE_INTERVAL(147, -1) is sent exactly once. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…it case _handleBatteryStatusV2 was calling _announceBatteryChargeState without filtering on compid, so a camera peripheral sending BATTERY_STATUS_V2 would trigger voice battery alerts for the vehicle. Added the same `compid != _defaultComponentId` guard already used by _handleHomePosition and the V2 negotiation path. Also standardise the five new BATTERY_INFO voltage fact unit strings from "V" to "v" to match the existing "voltage" fact in BatteryFact.json. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ward-decl BatteryFact.json had a pre-existing outlier using "v" (lowercase) for the voltage fact while every other Fact metadata file in the codebase uses "V". Standardise the existing voltage fact and all new BATTERY_INFO voltage facts to uppercase "V". FactGroupListModel.h declared methods taking Vehicle* without a forward declaration, making the header rely on include-order to compile. Add the missing class Vehicle; forward declaration, consistent with FactGroup.h and ParameterManager.h in the same directory. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_handleBatteryStatus lacked the compid guard that was added to _handleBatteryStatusV2 in the previous commit. A camera peripheral streaming BATTERY_STATUS (V1) could trigger incorrect voice alerts for the autopilot battery. Add the same early-return pattern used by _handleSysStatus and _handleBatteryStatusV2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nitialConnectComplete If BATTERY_STATUS_V2 frames arrive before initialConnectComplete fires (e.g. autopilot already streaming at 0.5 Hz), _activateV2 is called from handleMessageForFactGroupCreation and _v2State becomes Active. The early return at the top of startV2Negotiation then skips the entire function, including the BATTERY_INFO sendMavCommand, leaving BATTERY_INFO unrequested even when _infoReceived is false. Scope the early-return to the V2 negotiation block only. The BATTERY_INFO request is independent and always runs when _infoReceived is still false. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
Author
|
@DonLakeFlyer Claude beat Copilot into submission. The fixes sound pretty sensible, but I'm not much of a programmer. It does "work" in that you can connect to my test code or PX4 and you get the same battery information whether BATTERY_STATUS or the _V2 variant are sent. QGC turns BATTERY_STATUS off in PX4 if it gets the V2 message following "usual paterns". The idea being that when QGC has had enough releases PX4 will turn off streaming the original message and switch to the new one. No changes should be needed in QGC at that point. |
Collaborator
Author
|
@copilot resolve the merge conflicts in this pull request |
Collaborator
|
Still no mavlink update for this... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This provides support for the BATTERY_STATUS_V2 and BATTERY_INFO messages. It corresponds to the PX4 implementation in PX4/PX4-Autopilot#25347
The change populates the battery % icon and the popup with the same information as
BATTERY_STATUS. In order to ease migration, QGC will requestBATTERY_INFOandBATTERY_STATUS_V2if it doesn't get them. If it does get them it will send a command to the flight stack to stop streamingBATTERY_STATUS.The integration doesn't do anything special with the new information in the new messages. I do however capture all fields as facts so that someone can update the UI later if they so wish.
The work was done by Claude.
I also had claude write a test script to check that the battery information was displayed, and that the session negotiation worked.
It works against the corresponding PX4 build.
Type of Change
Testing
I have tested this first with this MAVSDK test: https://github.com/hamishwillee/mavsdk_qgc_server_tests/tree/main/battery_tests
If you run it like
python3 battery_test_mavsdk.py --autoit sends BATTERY_STATUS messages initially, then will switch to BATTERY_STATUS_V2 and BATTERY_INFO when requested.I ran the PX4 version in SITL from feat(mavlink): Battery_Status_V2 MAVLink stream PX4/PX4-Autopilot#25347 - note, this has to be run as
make px4_sitl_mavlink-dev gz_x500because BATTERY_STATUS_V2 is in development. Can see that PX4 does the switch and turns off BATTERY_STATUS.Platforms Tested
SITL on WSL2 on Ubuntu 24
Flight Stacks Tested
Screenshots
Checklist
Related Issues
By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).