From b4a4f85e78f2078f4c60da7bfa870308395e8e0e Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Wed, 8 Jul 2026 23:59:02 +0800 Subject: [PATCH 1/2] refactor(power): centralize PowerState derivation into PowerStatus StatusLEDModule re-derived charge/power state (discharging/charging/ charged/critical) from getHasUSB()/getIsCharging()/getBatteryChargePercent() independently of other consumers of the same signals. Move it to PowerStatus::getPowerState() so there's one source of truth; StatusLEDModule now just reads it. LED behavior unchanged. Also hoists the pre-existing power_state != Charging && power_state != Charged guard (duplicated across the LED_HEARTBEAT #if/#else branches) into a single local, since both copies needed the new qualified enum name anyway. Hardened PowerState itself: explicit numbering with Unknown = 0 as the default/unset value (was unnumbered, no Unknown), and getPowerState() now returns Unknown before the first real reading arrives instead of guessing from all-default fields. StatusLEDModule's default member initializer moves from Discharging to Unknown to match - runOnce() only branches on Charging/Charged/Critical, so anything else (including Unknown) still falls through to the same heartbeat default; no LED behavior change. This gives a future is_charging-style protobuf field a stable, ready-made source to map onto directly. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong --- src/PowerStatus.h | 17 +++++++++++++++++ src/modules/StatusLEDModule.cpp | 28 +++++++++------------------- src/modules/StatusLEDModule.h | 4 +--- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/PowerStatus.h b/src/PowerStatus.h index fe4543e08d2..f3864da5319 100644 --- a/src/PowerStatus.h +++ b/src/PowerStatus.h @@ -11,6 +11,11 @@ namespace meshtastic */ enum OptionalBool { OptFalse = 0, OptTrue = 1, OptUnknown = 2 }; +/// Coarse charge/power state, derived from PowerStatus's own fields. +/// Explicitly numbered, with Unknown=0 as the default/unset value, so this can be wire-mapped to a future +/// protobuf enum (e.g. an admin/telemetry is_charging-style field) without the numbering shifting under it. +enum class PowerState { Unknown = 0, Discharging = 1, Charging = 2, Charged = 3, Critical = 4 }; + /// Describes the state of the Power system. class PowerStatus : public Status { @@ -56,6 +61,18 @@ class PowerStatus : public Status bool getIsCharging() const { return isCharging == OptTrue; } + /// PowerState::Charged means "powered and battery reads full" - StatusLEDModule keys its solid-LED + /// state off this value directly. Returns Unknown before the first real reading arrives (e.g. cold + /// boot, before Power::readPowerStatus() has run) rather than guessing from all-default fields. + PowerState getPowerState() const + { + if (!isInitialized()) + return PowerState::Unknown; + if (getHasUSB() || getIsCharging()) + return getBatteryChargePercent() >= 100 ? PowerState::Charged : PowerState::Charging; + return getBatteryChargePercent() > 5 ? PowerState::Discharging : PowerState::Critical; + } + int getBatteryVoltageMv() const { return batteryVoltageMv; } /** diff --git a/src/modules/StatusLEDModule.cpp b/src/modules/StatusLEDModule.cpp index 5c6f849423d..4d49dfd58fd 100644 --- a/src/modules/StatusLEDModule.cpp +++ b/src/modules/StatusLEDModule.cpp @@ -48,18 +48,7 @@ int StatusLEDModule::handleStatusUpdate(const meshtastic::Status *arg) { switch (arg->getStatusType()) { case STATUS_TYPE_POWER: { - if (powerStatus->getHasUSB() || powerStatus->getIsCharging()) { - power_state = charging; - if (powerStatus->getBatteryChargePercent() >= 100) { - power_state = charged; - } - } else { - if (powerStatus->getBatteryChargePercent() > 5) { - power_state = discharging; - } else { - power_state = critical; - } - } + power_state = powerStatus->getPowerState(); break; } case STATUS_TYPE_BLUETOOTH: { @@ -111,13 +100,13 @@ int32_t StatusLEDModule::runOnce() { my_interval = 1000; - if (power_state == charging) { + if (power_state == meshtastic::PowerState::Charging) { #ifndef POWER_LED_HARDWARE_BLINKS_WHILE_CHARGING CHARGE_LED_state = !CHARGE_LED_state; #endif - } else if (power_state == charged) { + } else if (power_state == meshtastic::PowerState::Charged) { CHARGE_LED_state = LED_STATE_ON; - } else if (power_state == critical) { + } else if (power_state == meshtastic::PowerState::Critical) { if (POWER_LED_starttime + 30000 < millis() && !doing_fast_blink) { doing_fast_blink = true; POWER_LED_starttime = millis(); @@ -138,9 +127,10 @@ int32_t StatusLEDModule::runOnce() CHARGE_LED_state = LED_STATE_OFF; #endif } -// If we want a LED to be dedicated to the simple hearbeat, we can use that instead of the charge LED + // If we want a LED to be dedicated to the simple hearbeat, we can use that instead of the charge LED + const bool notCharging = power_state != meshtastic::PowerState::Charging && power_state != meshtastic::PowerState::Charged; #if defined(LED_HEARTBEAT) - if (power_state != charging && power_state != charged && !doing_fast_blink && !config.device.led_heartbeat_disabled) { + if (notCharging && !doing_fast_blink && !config.device.led_heartbeat_disabled) { if (HEARTBEAT_LED_state == LED_STATE_ON) { HEARTBEAT_LED_state = LED_STATE_OFF; my_interval = 999; @@ -154,7 +144,7 @@ int32_t StatusLEDModule::runOnce() digitalWrite(LED_HEARTBEAT, HEARTBEAT_LED_state); } #else - if (power_state != charging && power_state != charged && !doing_fast_blink && !config.device.led_heartbeat_disabled) { + if (notCharging && !doing_fast_blink && !config.device.led_heartbeat_disabled) { if (CHARGE_LED_state == LED_STATE_ON) { CHARGE_LED_state = LED_STATE_OFF; my_interval = 999; @@ -219,7 +209,7 @@ int32_t StatusLEDModule::runOnce() #ifdef LED_POWER #ifdef LED_POWER_CRITICAL if (LED_POWER != LED_POWER_CRITICAL) { - if (power_state == critical) { + if (power_state == meshtastic::PowerState::Critical) { digitalWrite(LED_POWER, 0); digitalWrite(LED_POWER_CRITICAL, CHARGE_LED_state); } else { diff --git a/src/modules/StatusLEDModule.h b/src/modules/StatusLEDModule.h index e9e148eaf0d..7d27d2c74f4 100644 --- a/src/modules/StatusLEDModule.h +++ b/src/modules/StatusLEDModule.h @@ -90,9 +90,7 @@ class StatusLEDModule : private concurrency::OSThread uint32_t LORA_LED_starttime = 0; #endif - enum PowerState { discharging, charging, charged, critical }; - - PowerState power_state = discharging; + meshtastic::PowerState power_state = meshtastic::PowerState::Unknown; enum BLEState { unpaired, pairing, connected }; From a3b7266ce69e7eafd4af8f7eef1f8856e4937774 Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Wed, 8 Jul 2026 23:59:35 +0800 Subject: [PATCH 2/2] fix(telemetry): report real battery % while charging Client-visible behavior change: nodes that are actively charging (plugged in, battery below 100%) now report their real battery percentage in mesh telemetry (device_metrics.battery_level) instead of the fixed sentinel 101. Apps/dashboards that show a node's live battery % while it charges will start seeing it move again instead of being pinned at 101/powered for the whole charge cycle. Root cause: battery_level was forced to MAGIC_USB_BATTERY_LEVEL (101) whenever isCharging() was true, at any percentage, masking the real number for the whole 0-99% charge ramp - on any board where charge detection works, regardless of which mechanism drives isCharging() (EXT_CHRG_DETECT/ EXT_PWR_DETECT pins, a PMU/fuel-gauge's own charge-status bit, or plain USB power detection). This is a pre-existing, general bug independent of #10906. I noticed it after #10906 merged, on my own device (a TP4057 charger wired to STBY->EXT_PWR_DETECT and CHRG->EXT_CHRG_DETECT), which reported 101 for its entire charge cycle instead of the real percentage. telemetry.proto documents battery_level as "0-100 (>100 means powered)" - 101 means "no reading available," not "currently charging." Fix: key the sentinel off !hasBattery alone. hasBattery is false whenever there's genuinely no battery ADC/fuel-gauge/PMU on the board, or battery hardware present with no cell inserted; every other board (the common case, including fuel-gauge/PMU boards) now reports its real percentage while charging, 0 through 100 - including a real 100% while still powered, previously reported as 101. Overriding that real 100% reading with 101 would violate the >100-means-powered contract (protobufs commit b546551 replaced an earlier "0 means powered" convention specifically to get an unambiguous "no real reading exists" sentinel, not a modifier meant to override a valid reading), and conflicts with iOS's BatteryLevel.swift, which already special-cases battery_level == 100 for its "topped-up-and-charging" bolt icon. The cost - losing "powered at exactly 100%" as a mesh-visible signal - is the same cost every other percentage already pays under this fix; a real is_charging field is the right way to recover it, not an overloaded battery_level. Deliberately not touched: boards defining NO_BATTERY_LEVEL_ON_CHARGE (today: only pico2_w5500_e22), whose getBatteryPercent() returns a placeholder 0 while charging because their voltage reading is untrusted then. Flipping hasBattery false for that board's whole charge cycle would break isCharging()'s own fallback formula, the JSON HTTP status API's is_charging field, and the on-device debug overlay. Showing 0% while charging there matches its own local display and is out of scope here. Client impact: checked Android, iOS and device-ui source directly. None treat 1-99 as a charging indicator, so no client-visible signal is removed there - the 0-99% ramp goes from hidden/frozen to live. At exactly 100%, iOS's bolt icon and device-ui's charging icon (both keyed off battery_level/percentage == 100) now trigger correctly instead of being defeated by the old 101 override; Android's ">100 means powered" text narrows from "Powered" to "100%" at that one value. This repo's own on-device Base UI (UIRenderer.cpp's remote-node status line) had the identical bug: it shows "Plugged In" in place of any percentage whenever battery_level > 100, so a remote node's entry froze at "Plugged In" for its whole charge cycle too - this fix restores the live percentage there as well. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong --- src/modules/Telemetry/DeviceTelemetry.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/modules/Telemetry/DeviceTelemetry.cpp b/src/modules/Telemetry/DeviceTelemetry.cpp index 06e22029652..12f864d167b 100644 --- a/src/modules/Telemetry/DeviceTelemetry.cpp +++ b/src/modules/Telemetry/DeviceTelemetry.cpp @@ -102,9 +102,8 @@ meshtastic_Telemetry DeviceTelemetryModule::getDeviceTelemetry() t.variant.device_metrics.has_channel_utilization = true; t.variant.device_metrics.has_uptime_seconds = true; t.variant.device_metrics.air_util_tx = airTime->utilizationTXPercent(); - t.variant.device_metrics.battery_level = (!powerStatus->getHasBattery() || powerStatus->getIsCharging()) - ? MAGIC_USB_BATTERY_LEVEL - : powerStatus->getBatteryChargePercent(); + t.variant.device_metrics.battery_level = + !powerStatus->getHasBattery() ? MAGIC_USB_BATTERY_LEVEL : powerStatus->getBatteryChargePercent(); t.variant.device_metrics.channel_utilization = airTime->channelUtilizationPercent(); // Only populate voltage when we actually have a battery reading. Previously this assigned // -0.001 (from -1 mV / 1000) whenever the ADC returned -1, leaking a sentinel onto the wire