From 6b3f975ba500667a509dfa17ca8a3c7d7f9750e7 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Tue, 9 Jun 2026 18:58:26 -0500 Subject: [PATCH] fix(ble): reliably expose and update BLE battery level (BAS) (#10622) * fix(ble): reliably expose and update BLE battery level (BAS) The Battery Service (0x180F / 0x2A19) is now wired up per the Bluetooth BAS spec: the Battery Level characteristic always holds a valid 0-100 value and is pushed on change. - NimBLE: seed an initial level at setup and cache the value on every update so a READ returns the current level even while disconnected; only notify when a client is connected. - Power: mirror the battery level to the Battery Service from readPowerStatus() on change, so it updates independent of GPS/position events (previously the only push path was MeshService). Also fixes two regressions the above would otherwise introduce: - NimBLE use-after-free: BLEDevice::deinit(true) frees the GATT objects but left the global BatteryCharacteristic dangling. Several AdminModule paths (e.g. serial-config entry) deinit BLE while config.bluetooth.enabled stays true, so the periodic push would deref freed memory. Null the pointer in deinit(). - NRF52: guard blebas.write() on the nrf52Bluetooth instance so the new periodic push can't call it before the Battery Service is begun in setup(). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(ble): clamp BAS battery level to 0-100 and skip redundant updates Address review feedback on the Battery Level characteristic (0x2A19): - Clamp the value to the BAS-mandated 0-100 range at the platform write boundary (NimBLE seed + update, NRF52 update), so a misbehaving battery backend can't put an out-of-range value on the characteristic. - Skip the write/notify when the level is unchanged, so repeated callers (e.g. MeshService refresh paths) don't emit redundant notifications. - Simplify Power.cpp to a direct guarded call now that clamping and de-duplication live at the boundary, which also removes the implicit int->uint8_t narrowing. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Ben Meadors --- src/Power.cpp | 5 +++++ src/nimble/NimbleBluetooth.cpp | 24 +++++++++++++++++++++--- src/platform/nrf52/NRF52Bluetooth.cpp | 14 ++++++++++++-- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/Power.cpp b/src/Power.cpp index 43e9f1059..42bdf3ce4 100644 --- a/src/Power.cpp +++ b/src/Power.cpp @@ -14,6 +14,7 @@ * For more information, see: https://meshtastic.org/ */ #include "power.h" +#include "BluetoothCommon.h" #include "MessageStore.h" #include "NodeDB.h" #include "PowerFSM.h" @@ -962,6 +963,10 @@ void Power::readPowerStatus() lastLogTime = millis(); } newStatus.notifyObservers(&powerStatus2); + + // Mirror battery level to the BLE Battery Service (0x2A19); the platform layer clamps and dedupes. + if (hasBattery == OptTrue) + updateBatteryLevel(powerStatus2.getBatteryChargePercent()); #ifdef DEBUG_HEAP if (lastheap != memGet.getFreeHeap()) { // Use stack-allocated buffer to avoid heap allocations in monitoring code diff --git a/src/nimble/NimbleBluetooth.cpp b/src/nimble/NimbleBluetooth.cpp index eb4bd018f..5722a6691 100644 --- a/src/nimble/NimbleBluetooth.cpp +++ b/src/nimble/NimbleBluetooth.cpp @@ -40,6 +40,7 @@ constexpr uint16_t kPreferredBleTxTimeUs = (kPreferredBleTxOctets + 14) * 8; BLECharacteristic *fromNumCharacteristic; BLECharacteristic *BatteryCharacteristic; +static int lastBatteryLevel = -1; // last value written to 0x2A19, to skip redundant writes/notifies BLECharacteristic *logRadioCharacteristic; BLEServer *bleServer; @@ -718,6 +719,8 @@ void NimbleBluetooth::deinit() #endif BLEDevice::deinit(true); + BatteryCharacteristic = nullptr; // freed by deinit; clear so updateBatteryLevel() won't touch it + lastBatteryLevel = -1; #endif } @@ -856,16 +859,31 @@ void NimbleBluetooth::setupService() BatteryCharacteristic = batteryService->createCharacteristic( // 0x2A19 is the Battery Level characteristic) (uint16_t)0x2a19, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); BatteryCharacteristic->addDescriptor(batteryLevelDescriptor); + // Seed an initial 0-100 level so an early read of 0x2A19 returns a valid value. + uint8_t initialLevel = (powerStatus && powerStatus->getHasBattery()) ? powerStatus->getBatteryChargePercent() : 0; + if (initialLevel > 100) + initialLevel = 100; + BatteryCharacteristic->setValue(&initialLevel, 1); + lastBatteryLevel = initialLevel; batteryService->start(); } /// Given a level between 0-100, update the BLE attribute void updateBatteryLevel(uint8_t level) { - if ((config.bluetooth.enabled == true) && nimbleBluetooth && nimbleBluetooth->isConnected()) { - BatteryCharacteristic->setValue(&level, 1); + if (!config.bluetooth.enabled || !BatteryCharacteristic) + return; + + if (level > 100) // 0x2A19 must stay within the BAS 0-100 range + level = 100; + if (level == lastBatteryLevel) + return; + lastBatteryLevel = level; + + // Cache the value so a READ works without a subscriber; notify only when connected. + BatteryCharacteristic->setValue(&level, 1); + if (nimbleBluetooth && nimbleBluetooth->isConnected()) BatteryCharacteristic->notify(); - } } void NimbleBluetooth::clearBonds() diff --git a/src/platform/nrf52/NRF52Bluetooth.cpp b/src/platform/nrf52/NRF52Bluetooth.cpp index 4af8b41f6..c877829db 100644 --- a/src/platform/nrf52/NRF52Bluetooth.cpp +++ b/src/platform/nrf52/NRF52Bluetooth.cpp @@ -15,8 +15,9 @@ static BLECharacteristic fromRadio = BLECharacteristic(BLEUuid(FROMRADIO_UUID_16 static BLECharacteristic toRadio = BLECharacteristic(BLEUuid(TORADIO_UUID_16)); static BLECharacteristic logRadio = BLECharacteristic(BLEUuid(LOGRADIO_UUID_16)); -static BLEDis bledis; // DIS (Device Information Service) helper class instance -static BLEBas blebas; // BAS (Battery Service) helper class instance +static BLEDis bledis; // DIS (Device Information Service) helper class instance +static BLEBas blebas; // BAS (Battery Service) helper class instance +static int lastBatteryLevel = -1; // last value written to BAS, to skip redundant writes/notifies #ifndef BLE_DFU_SECURE static BLEDfu bledfu; // DFU software update helper service #else @@ -336,6 +337,7 @@ void NRF52Bluetooth::setup() LOG_INFO("Init the Battery Service"); blebas.begin(); blebas.write(0); // Unknown battery level for now + lastBatteryLevel = 0; // Setup the Heart Rate Monitor service using // BLEService and BLECharacteristic classes LOG_INFO("Init the Mesh bluetooth service"); @@ -355,6 +357,14 @@ void NRF52Bluetooth::resumeAdvertising() /// Given a level between 0-100, update the BLE attribute void updateBatteryLevel(uint8_t level) { + if (!nrf52Bluetooth) // skip until the Battery Service has been begun in setup() + return; + + if (level > 100) // BAS battery level must stay within 0-100 + level = 100; + if (level == lastBatteryLevel) + return; + lastBatteryLevel = level; blebas.write(level); } void NRF52Bluetooth::clearBonds()