Battery: use the standard BLE charging flag when a device exposes it

The companion protocol reports only a battery voltage, so charging is a
voltage-trend inference. Some devices could instead expose the standard
GATT Battery Service (0x180F) whose Battery Level Status characteristic
(0x2BED, Battery Service 1.1) carries a real, firmware-reported charging
flag. Dumping the GATT table shows no current MeshCore build implements
it — the BLE profile is just the Nordic UART pipe plus DFU — but wire it
up now so it lights up automatically the day one does.

get_hw_charging() reads 0x2BED best-effort over BLE (reaching the bleak
client the forced-teardown path already uses) and prefers its flag; any
failure — service absent, read refused, short value, non-BLE transport —
is a quiet None, so the poller falls back to the inference and every
current device behaves exactly as before. Verified against a live device:
the read resolves and returns a clean, steady None. The 0x2BED bitfield
decode is a pure function unit-tested against the spec's Charge State enum
so the parse is correct ahead of any hardware that populates it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

meshterm/core/connection.py

@@ -58,6 +58,45 @@ _MOCK_MONITOR_INTERVAL_S = 0.05
 #: safety net for firmware that doesn't reliably push ``MESSAGES_WAITING`` (seconds).
 _MESSAGE_POLL_INTERVAL_S = 3.0
 
+#: Standard Bluetooth GATT Battery Service and its Battery Level Status characteristic (GATT
+#: Specification Supplement, "Battery Level Status", 0x2BED — added in Battery Service 1.1).
+#: Where a companion exposes these, the status characteristic carries a *firmware-reported*
+#: charging flag — a ground truth the MeshCore companion protocol itself never provides (it
+#: reports only a battery voltage). MeshCore firmware does not implement them today: its BLE
+#: profile is just the Nordic UART pipe plus DFU, confirmed by dumping the GATT table. So this
+#: path lies dormant behind a fail-safe fallback and lights up automatically only if a future
+#: device ships the service. See :func:`charging_from_battery_level_status`.
+_BATTERY_SERVICE_UUID = "0000180f-0000-1000-8000-00805f9b34fb"
+_BATTERY_LEVEL_STATUS_UUID = "00002bed-0000-1000-8000-00805f9b34fb"
+
+
+def charging_from_battery_level_status(data: bytes) -> Optional[bool]:
+    """Decode the charging state from a GATT *Battery Level Status* value (0x2BED).
+
+    Per the Bluetooth GATT Specification Supplement the characteristic opens with a 1-byte
+    flags field followed by a 16-bit little-endian *Power State* word; the two bits at offset
+    5 are the **Charge State** enum — 0 unknown, 1 charging, 2 discharging (active), 3
+    discharging (inactive). The fields after the word (identifier, battery level, additional
+    status) are optional and unread here, so only the first three bytes are required.
+
+    Args:
+        data: The raw characteristic value as read over GATT.
+
+    Returns:
+        ``True`` when the pack reports it is charging, ``False`` when it reports discharging,
+        or ``None`` when the value is too short or the state is *unknown* — in which case the
+        caller should fall back to the voltage-trend inference.
+    """
+    if len(data) < 3:
+        return None
+    power_state = int.from_bytes(data[1:3], "little")
+    charge_state = (power_state >> 5) & 0b11
+    if charge_state == 1:
+        return True
+    if charge_state in (2, 3):
+        return False
+    return None
+
 #: Per-``get_msg`` timeout in the message pump, so a missing device reply can't wedge the
 #: drain loop (seconds).
 _MESSAGE_GET_TIMEOUT_S = 5.0
@@ -546,6 +585,18 @@ class Device(ABC):
             ``used_kb``/``total_kb``. Empty when the read is unsupported.
         """
 
+    async def get_hw_charging(self) -> Optional[bool]:
+        """Return a firmware-reported charging flag, or ``None`` when the device has none.
+
+        The companion protocol carries only a battery voltage, so almost every device answers
+        ``None`` and callers fall back to inferring charge from the voltage trend (see
+        :meth:`~meshterm.services.battery_service.BatteryService._charging`). A transport that
+        can read a real charging flag — a BLE device exposing the standard Battery Level Status
+        characteristic (0x2BED) — overrides this to return it. Best-effort by contract: it
+        never raises and never meaningfully blocks, so a caller may await it every poll.
+        """
+        return None
+
     @abstractmethod
     async def get_stats(self) -> dict:
         """Return the firmware's core/radio/packet statistics, merged into one dict.
@@ -779,6 +830,9 @@ class MeshCoreDevice(Device):
         # the first response and one caller silently gets the other's channel. Holding this lock
         # keeps at most one channel read outstanding, so the response is unambiguously ours.
         self._channel_read_lock = asyncio.Lock()
+        #: Set once we've noted a device exposing the standard BLE Battery Service, so the
+        #: "using its charging flag" log fires a single time per session rather than each poll.
+        self._logged_bas = False
 
     @property
     def transport(self) -> str:
@@ -1830,6 +1884,36 @@ class MeshCoreDevice(Device):
         event = self._ok(await self._require().commands.get_bat())
         return dict(getattr(event, "payload", {}) or {})
 
+    async def get_hw_charging(self) -> Optional[bool]:  # noqa: D102 - inherited docstring
+        # BLE only, and only when a device exposes the standard Battery Level Status
+        # characteristic — no MeshCore firmware does today, so this returns None on every
+        # current device and the battery poller falls back to its voltage-trend inference. It
+        # reaches the raw bleak client the way the forced-teardown path does (through
+        # ``connection_manager.connection``) and is wrapped whole: any failure — attribute
+        # path moved, service absent, read refused, short value — is a quiet None, never a
+        # disrupted poll.
+        if self._transport != "ble" or self._mc is None:
+            return None
+        try:
+            client = self._mc.connection_manager.connection.client
+            service = client.services.get_service(_BATTERY_SERVICE_UUID)
+            if service is None:
+                return None
+            char = service.get_characteristic(_BATTERY_LEVEL_STATUS_UUID)
+            if char is None or "read" not in getattr(char, "properties", ()):
+                return None
+            value = await client.read_gatt_char(char)
+        except Exception as exc:  # noqa: BLE001 - a best-effort probe must never disrupt polling
+            _log.debug("hardware charging read failed: %s", exc)
+            return None
+        if not self._logged_bas:
+            self._logged_bas = True
+            _log.info(
+                "device exposes a standard BLE Battery Service (0x180F); using its "
+                "charging flag over the voltage-trend inference — verify the reading"
+            )
+        return charging_from_battery_level_status(bytes(value))
+
     async def get_stats(self) -> dict:  # noqa: D102 - inherited docstring
         mc = self._require()
         stats: dict = {}

meshterm/services/battery_service.py

@@ -13,14 +13,17 @@ Two device facts shape what it can report:
   the reading is turned into a state-of-charge estimate against a single-cell LiPo
   discharge curve (:func:`battery_percent`). Devices with no battery gauge answer with no
   usable level; those are reported as *absent* so the header shows nothing for them.
-* Firmware exposes **no charging flag at all**. Charging is therefore *inferred* from the
-  terminal voltage — but a single-cell LiPo sags tens of millivolts under each LoRa
+* The companion protocol exposes **no charging flag at all**. Charging is therefore *inferred*
+  from the terminal voltage — but a single-cell LiPo sags tens of millivolts under each LoRa
   transmission and springs back when the radio idles, so the raw sample trend is mostly load
   noise, not charge. The inference reads it robustly (the median of the sample window's older
   half against its newer half, which discards those transient spikes) and calls charge only
   on a large, *sustained* rise, dropping straight back the moment the pack goes flat. It is
-  still an estimate — the best the firmware allows — but one a discharging or resting pack no
-  longer trips.
+  still an estimate — the best the companion protocol allows — but one a discharging or
+  resting pack no longer trips. Where a device *does* expose a firmware charging flag over the
+  standard BLE Battery Level Status characteristic (no MeshCore build does today), that ground
+  truth is preferred and the inference only fills in for the rest — see
+  :meth:`~meshterm.core.connection.Device.get_hw_charging`.
 """
 
 from __future__ import annotations
@@ -197,10 +200,15 @@ class BatteryService:
             return
         now = time.monotonic()
         self._history.append((now, mv))
+        # Prefer a firmware-reported charging flag when the device exposes one (the standard
+        # BLE Battery Level Status characteristic); only where it can't — every MeshCore device
+        # today — fall back to inferring charge from the voltage trend.
+        hw_charging = await device.get_hw_charging()
+        charging = hw_charging if hw_charging is not None else self._charging(now)
         self._reading = BatteryReading(
             millivolts=mv,
             percent=battery_percent(mv),
-            charging=self._charging(now),
+            charging=charging,
         )
 
     def _charging(self, now: float) -> bool:

tests/test_battery.py

@@ -8,8 +8,10 @@ charging inference are all assertable without a device or a terminal.
 from __future__ import annotations
 
 import asyncio
+import time
 from types import SimpleNamespace
 
+from meshterm.core.connection import charging_from_battery_level_status
 from meshterm.services.battery_service import (
     _BATTERY_PRESENT_FLOOR_MV,
     POLL_S,
@@ -85,15 +87,22 @@ def test_battery_cell_charging_sweeps_bottom_to_full_holding_the_percent() -> No
 # --- the poller ---------------------------------------------------------------------------
 
 
-def _service(levels) -> BatteryService:
-    """A ``BatteryService`` over a fake context whose device answers ``levels`` in turn."""
+def _service(levels, hw_charging=None) -> BatteryService:
+    """A ``BatteryService`` over a fake context whose device answers ``levels`` in turn.
+
+    ``hw_charging`` is what the device's firmware charging flag reports each poll — ``None``
+    (the default, and every real MeshCore device) means "no flag", so the poller infers.
+    """
     seq = list(levels)
 
     async def get_battery() -> dict:
         return {"level": seq.pop(0)} if seq else {}
 
+    async def get_hw_charging():
+        return hw_charging
+
     async def device() -> SimpleNamespace:
-        return SimpleNamespace(get_battery=get_battery)
+        return SimpleNamespace(get_battery=get_battery, get_hw_charging=get_hw_charging)
 
     ctx = SimpleNamespace(
         is_connected=True,
@@ -175,3 +184,53 @@ def test_battery_poller_charging_ignores_load_sag_and_flat_or_falling_packs() ->
     svc._history.clear()
     svc._history.extend(_history_of(now, [3800, 3900]))
     assert svc._charging(now) is False
+
+
+# --- the standard BLE charging flag (GATT Battery Level Status, 0x2BED) --------------------
+
+
+def _level_status(charge_state: int, *, flags: int = 0, extra: bytes = b"") -> bytes:
+    """A Battery Level Status value: flags byte, the Power State word, then optional tail."""
+    power_state = (charge_state & 0b11) << 5
+    return bytes([flags]) + power_state.to_bytes(2, "little") + extra
+
+
+def test_charging_flag_decodes_the_charge_state_field() -> None:
+    """The 2-bit Charge State enum maps to charging / not / unknown per the GSS."""
+    assert charging_from_battery_level_status(_level_status(1)) is True   # charging
+    assert charging_from_battery_level_status(_level_status(2)) is False  # discharging (active)
+    assert charging_from_battery_level_status(_level_status(3)) is False  # discharging (inactive)
+    assert charging_from_battery_level_status(_level_status(0)) is None   # unknown → infer
+
+
+def test_charging_flag_reads_only_the_power_state_word() -> None:
+    """Other Power State bits and the optional identifier/level tail don't sway the verdict."""
+    # Battery present + wired external power + charging, with identifier & level bytes trailing.
+    value = _level_status(1, flags=0b011, extra=b"\xab\xcd\x50")
+    assert charging_from_battery_level_status(value) is True
+    # A short value (no room for the Power State word) is unknown, never a guess.
+    assert charging_from_battery_level_status(b"\x00") is None
+    assert charging_from_battery_level_status(b"") is None
+
+
+def test_battery_poller_prefers_the_hardware_charging_flag_over_inference() -> None:
+    """A firmware charging flag wins over the voltage-trend guess, in both directions."""
+    now = time.monotonic()
+    rising = [3700, 3712, 3724, 3736, 3748, 3760, 3772, 3784]
+
+    # Baseline: with no hardware flag the rising trend is *inferred* as charging.
+    svc = _service([3900], hw_charging=None)
+    svc._history.extend(_history_of(now, rising))
+    asyncio.run(svc._poll())
+    assert svc.reading().charging is True
+
+    # Same rising trend, but the hardware reports NOT charging → the flag overrides the guess.
+    svc = _service([3900], hw_charging=False)
+    svc._history.extend(_history_of(now, rising))
+    asyncio.run(svc._poll())
+    assert svc.reading().charging is False
+
+    # And a hardware "charging" wins with no trend at all, where inference would say not.
+    svc = _service([3900], hw_charging=True)
+    asyncio.run(svc._poll())
+    assert svc.reading().charging is True