From 30ab623f89286f7c82145ae68f55df05356e7b84 Mon Sep 17 00:00:00 2001
From: James Rich <2199651+jamesarich@users.noreply.github.com>
Date: Tue, 9 Jun 2026 15:47:49 -0500
Subject: [PATCH] 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) <noreply@anthropic.com>
---
 src/Power.cpp                         | 12 +++---------
 src/nimble/NimbleBluetooth.cpp        | 11 +++++++++++
 src/platform/nrf52/NRF52Bluetooth.cpp | 17 +++++++++++++----
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/src/Power.cpp b/src/Power.cpp
index 4272746fd..42bdf3ce4 100644
--- a/src/Power.cpp
+++ b/src/Power.cpp
@@ -964,15 +964,9 @@ void Power::readPowerStatus()
     }
     newStatus.notifyObservers(&powerStatus2);
 
-    // Mirror battery level to the BLE Battery Service (0x2A19), pushing only on change.
-    if (hasBattery == OptTrue) {
-        static int lastBleBatteryPercent = -1;
-        int pct = powerStatus2.getBatteryChargePercent();
-        if (pct != lastBleBatteryPercent) {
-            lastBleBatteryPercent = pct;
-            updateBatteryLevel(pct);
-        }
-    }
+    // 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 13ad9b981..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;
 
@@ -719,6 +720,7 @@ void NimbleBluetooth::deinit()
 
     BLEDevice::deinit(true);
     BatteryCharacteristic = nullptr; // freed by deinit; clear so updateBatteryLevel() won't touch it
+    lastBatteryLevel = -1;
 #endif
 }
 
@@ -859,7 +861,10 @@ void NimbleBluetooth::setupService()
     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();
 }
 
@@ -869,6 +874,12 @@ void updateBatteryLevel(uint8_t level)
     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())
diff --git a/src/platform/nrf52/NRF52Bluetooth.cpp b/src/platform/nrf52/NRF52Bluetooth.cpp
index 3aaa7078a..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,8 +357,15 @@ 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()
-        blebas.write(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()
 {