Power_Class::isCharging() reports is_discharging for a fully-charged battery on external power (IP5306)

[a4x4kiwi]

Power_Class::isCharging() reports is_discharging for a fully-charged battery on external power (IP5306)

Summary

On boards using the IP5306 PMIC (e.g. M5Stack Basic / Core), M5.Power.isCharging()
returns is_charging_t::is_discharging once the battery reaches full charge, even
though external power (USB) is clearly connected and supplying the board. There is
no way for calling code to distinguish this state from the battery genuinely being
unplugged and running down — is_charging_t::charge_unknown is never returned on
this code path, unlike several other PMIC paths in the same function.

This makes M5.Power.isCharging() unsafe to use for the common pattern of "stay
powered while on a charger, shut down / warn when genuinely on battery," which is
exactly the use case the tri-state enum (is_charging / is_discharging /
charge_unknown) appears designed to support.

Environment

• Library: M5Unified 0.2.21 (installed via PlatformIO, m5stack/M5Unified @ 0.2.21)
• Board: M5Stack Basic (Core, original), board_t::board_M5Stack, PMIC pmic_ip5306
• Framework: Arduino-ESP32, PlatformIO

Root cause

Power_Class::isCharging() in src/utility/Power_Class.cpp (around line 2692):

case pmic_t::pmic_ip5306:
  return Ip5306.isCharging() ? is_charging_t::is_charging : is_charging_t::is_discharging;

This is a plain boolean-to-enum mapping with no charge_unknown path, unlike
other PMIC branches in the same function (e.g. the ESP32C6/pmic_m5pm1 branches
explicitly return charge_unknown when a read fails or battery presence can't be
determined).

IP5306_Class::isCharging() in src/utility/power/IP5306_Class.cpp (lines
106–131) itself already documents the ambiguity it's built on:

bool IP5306_Class::isCharging(void)
{ /// This needs both of the flags the datasheet describes, not one of them:
  /// REG_READ0 bit3 tells charging from discharging, and REG_READ1 bit3
  /// tells whether the cell has already been filled. Only the first was
  /// read. It stays set once charging is enabled and a supply is present -
  /// the completed charge included - so a finished charge was reported as an
  /// ongoing one, as was a board running with no cell installed at all.
  ...
  std::uint8_t val = 0;
  if (!readRegister(REG_READ0, &val, 1)) { return false; }
  if (!(val & 0x08)) { return false; }                // no supply / charging disabled
  if (!readRegister(REG_READ1, &val, 1)) { return false; }
  return !(val & 0x08);                                // false once already full
}

So the function ANDs together two genuinely different conditions:

• REG_READ0 bit3 — a supply is present and charging is enabled
• REG_READ1 bit3 — the battery is not yet full

Once a battery finishes charging while still connected to USB, REG_READ1 bit3 sets,
isCharging() returns false, and Power_Class::isCharging() reports
is_discharging — identical to what a genuinely unplugged, draining battery would
report. The 0.2.11→0.2.21 fix to this function (per its own comment) correctly
stopped a finished charge from being reported as an ongoing one, but didn't add
a way for callers to tell "finished charge, supply present" apart from "no supply at
all." Both still collapse to is_discharging.

Steps to reproduce

1. Flash any sketch calling M5.Power.isCharging() to an M5Stack Basic with a
   battery fitted.
2. Connect USB power and leave the battery to charge fully.
3. Once the battery reaches 100% / charge-complete, call M5.Power.isCharging()
   while still connected to USB.

Actual behaviour

M5.Power.isCharging() returns is_charging_t::is_discharging.

Expected behaviour

Some indication that a supply is present and the "not charging" state is because
the battery is already full — either is_charging_t::charge_unknown (consistent
with how other PMIC branches in this same function already use it for an
ambiguous/undetermined state), or a distinct signal callers can check for supply
presence independent of charge completion.

Hardware evidence

Captured directly from a board sitting on USB power with a fully-charged battery,
by reading the same two registers IP5306_Class::isCharging() reads:

REG_READ0 = 0x19   (bit3 set -> supply present, charging enabled)
REG_READ1 = 0xA8   (bit3 set -> battery already full)
Ip5306.isCharging()        = false
Power_Class::isCharging()  = is_discharging

REG_READ0 alone (supply-present) stays correctly set throughout — it's only the
AND with REG_READ1 that turns a legitimate "on charger, topped up" state into
"discharging."

Impact

Any application using M5.Power.isCharging() to decide "is this unit on a
charger" (battery-protection auto-shutoff, UI charge icons, power-saving logic,
etc.) will incorrectly treat a fully-charged, USB-connected unit as running on
battery. In our case this caused a battery-protection screen to power the unit off
five seconds after every reflash whenever the battery happened to be full,
including screens that were unwritable/unflashable again until the physical power
button was pressed — bad enough with a battery involved that we've stopped relying
on this API and read REG_READ0 bit 3 directly instead.

Suggested fix (either would resolve it)

1. In Power_Class::isCharging()'s pmic_ip5306 branch, return
   charge_unknown when REG_READ0 bit3 is set (supply present) but the battery
   is already full, rather than is_discharging — consistent with how ambiguous
   states are already handled for other PMICs in this same function.
2. Expose a separate method (e.g. IP5306_Class::isSupplyPresent()) that reads
   REG_READ0 bit3 alone, so callers who specifically need "is external power
   present" (independent of charge completion) have a documented, unambiguous way
   to ask that question without relying on isCharging()'s conflated semantics.

Happy to open a PR for either approach if a maintainer has a preference.

[a4x4kiwi]

Suggested patch

Adds IP5306_Class::isSupplyPresent() (reads REG_READ0 bit3 alone — supply
present, independent of charge completion), then uses it to close the gap in
Power_Class::isCharging() so a full battery on a live supply reports
charge_unknown instead of is_discharging, consistent with how other PMIC
branches in the same function already use charge_unknown for an
undetermined state.

src/utility/power/IP5306_Class.hpp

     /// Get whether the battery is currently charging or not.
     bool isCharging(void);
 
+    /// Get whether external power (a charger/USB supply) is present,
+    /// independent of whether the battery has finished charging. Unlike
+    /// isCharging(), this stays true once the battery is full - it only
+    /// reflects REG_READ0 bit3, not the AND with the "already full" flag
+    /// in REG_READ1 that isCharging() also checks.
+    bool isSupplyPresent(void);
+
     /// Set whether or not to continue supplying power even at low loads.
     bool setPowerBoostKeepOn(bool en);

src/utility/power/IP5306_Class.cpp

   bool IP5306_Class::isCharging(void)
   { /// This needs both of the flags the datasheet describes, not one of them:
     ...
     std::uint8_t val = 0;
     if (!readRegister(REG_READ0, &val, 1)) { return false; }
     if (!(val & 0x08)) { return false; }
     if (!readRegister(REG_READ1, &val, 1)) { return false; }
     return !(val & 0x08);
   }
 
+  bool IP5306_Class::isSupplyPresent(void)
+  {
+    std::uint8_t val = 0;
+    if (!readRegister(REG_READ0, &val, 1)) { return false; }
+    return (val & 0x08) != 0;
+  }
+
   bool IP5306_Class::setPowerBoostKeepOn(bool en) {

src/utility/Power_Class.cpp (~line 2692)

     case pmic_t::pmic_ip5306:
-      return Ip5306.isCharging() ? is_charging_t::is_charging : is_charging_t::is_discharging;
+      // A full battery on a live supply must not read the same as no supply at
+      // all - isCharging() alone can't tell those apart (see #349).
+      if (!Ip5306.isSupplyPresent()) { return is_charging_t::is_discharging; }
+      return Ip5306.isCharging() ? is_charging_t::is_charging : is_charging_t::charge_unknown;

This is a behaviour change for existing callers only in the "full battery, still
on supply" case - it moves from is_discharging (arguably always wrong there) to
charge_unknown (matches how the rest of this function already represents "can't
say charging or discharging"). Everything else isCharging()/Power_Class::isCharging()
already handled correctly is untouched.

Happy to send this as a PR against a specific branch if that's easier to review
than a diff in the issue.