fix(m5stack-tab5): only enable charging when a battery is fitted (periodic speaker pop)

components/ina226/include/ina226.hpp

@@ -47,6 +47,25 @@ class Ina226 : public BasePeripheral<uint8_t, true> {
     MS_8_244 = 7,
   };
 
+  /// Number of samples averaged for an Avg setting
+  static constexpr uint32_t averaging_count(Avg avg) {
+    constexpr uint32_t counts[] = {1, 4, 16, 64, 128, 256, 512, 1024};
+    return counts[static_cast<uint16_t>(avg) & 0x7];
+  }
+
+  /// Duration of one conversion for a ConvTime setting, in microseconds
+  static constexpr uint32_t conversion_time_us(ConvTime ct) {
+    constexpr uint32_t us[] = {140, 204, 332, 588, 1100, 2116, 4156, 8244};
+    return us[static_cast<uint16_t>(ct) & 0x7];
+  }
+
+  /// Time for one averaged bus + shunt conversion pair (one update of the
+  /// data registers in continuous mode), in microseconds: averaging x (bus +
+  /// shunt conversion time). E.g. AVG_16 with MS_1_1 for both is 35,200 us.
+  static constexpr uint32_t conversion_time_us(Avg avg, ConvTime bus, ConvTime shunt) {
+    return averaging_count(avg) * (conversion_time_us(bus) + conversion_time_us(shunt));
+  }
+
   /// Operating mode (MODE bits 2..0)
   enum class Mode : uint16_t {
     POWER_DOWN = 0,
@@ -123,6 +142,20 @@ class Ina226 : public BasePeripheral<uint8_t, true> {
     return read_u16_from_register((uint8_t)Reg::DIE_ID, ec);
   }
 
+  /// Whether a conversion has completed since the Mask/Enable register was
+  /// last read (CVRF). After initialize() the data registers hold zeros until
+  /// the first averaged conversion finishes (averaging x (bus + shunt
+  /// conversion time), see conversion_time_us()), so poll this before trusting
+  /// the first reading.
+  /// @param ec Error code to capture any read errors
+  /// @return true once a conversion is complete; false before that or on error
+  /// @note Reading the flag clears it (datasheet), so it reports "at least one
+  ///       conversion since the last call", not the state of the converter.
+  bool conversion_ready(std::error_code &ec) const {
+    uint16_t mask = read_u16_from_register((uint8_t)Reg::MASK_ENABLE, ec);
+    return !ec && (mask & MASK_ENABLE_CVRF) != 0;
+  }
+
   // Engineering-unit helpers
 
   /// Read shunt voltage in volts
@@ -259,6 +292,8 @@ class Ina226 : public BasePeripheral<uint8_t, true> {
     MANUFACTURER_ID = 0xFE, // 0x5449
     DIE_ID = 0xFF,          // 0x2260
   };
+  // Mask/Enable register: conversion ready flag (bit 3)
+  static constexpr uint16_t MASK_ENABLE_CVRF = 1 << 3;
 
   bool init(const Config &c, std::error_code &ec) {
     std::lock_guard<std::recursive_mutex> lock(base_mutex_);

components/m5stack-tab5/README.md

@@ -36,7 +36,12 @@ The `espp::M5StackTab5` component provides a singleton hardware abstraction for
 ### Power Management
 - Removable NP-F550 Li-ion battery
 - MP4560 buck-boost converter
-- IP2326 charge management
+- IP2326 charge management (`set_charging_enabled()`, off by default; only
+  enable it when `read_battery_status().is_present` is true, since the charger
+  retrying to charge an empty battery bay pops the speaker periodically; the
+  enabled charger also pops on every speaker transient, so the BSP pauses
+  charging during playback and resumes it 3 s after the last sound, see
+  `set_charging_pause_on_playback_enabled()`)
 - INA226 real-time power monitoring
 - Multiple power modes for efficiency
 

components/m5stack-tab5/example/README.md

@@ -43,7 +43,15 @@ This example demonstrates the comprehensive functionality of the M5Stack Tab5 de
       dropped. It is logged from an ISR (`ESP_EARLY_LOGE`), which ignores the
       runtime log-level controls, so it cannot be silenced the way the CCM spam
       is without also hiding real errors - it is left as-is.
-- **Battery Management**: INA226 power monitoring with charging control
+- **Battery Management**: INA226 power monitoring with charging control.
+  Charging is enabled only when a battery is detected in the bay
+  (`BatteryStatus::is_present`): with the bay empty, the IP2326 charger keeps
+  retrying to charge nothing, and each retry is an audible pop on the speaker
+  (every few seconds to a couple of minutes, even when the board is idle).
+  With a battery fitted, the BSP pauses charging while the speaker plays and
+  resumes it 3 s after the last sound (the enabled charger pops on every
+  audio transient, whatever the charge current); the example's battery line
+  shows the charger switching off during a click or playback and back on.
 
 ### Communication Interfaces
 - **microSD Card**: File system support with SDIO interface

components/m5stack-tab5/example/main/m5stack_tab5_example.cpp

@@ -205,8 +205,19 @@ extern "C" void app_main(void) {
     return;
   }
 
-  // enable charging
-  tab5.set_charging_enabled(true);
+  // Enable charging only when a battery is actually in the bay. The NP-F550 is
+  // optional (kit-dependent); with the bay empty, the IP2326 charger keeps
+  // retrying to charge nothing and every retry puts an audible pop on the
+  // speaker (every 5 s to 2 min, idle or not), while charging an inserted pack
+  // is silent.
+  if (auto battery = tab5.read_battery_status(); battery.is_present) {
+    logger.info("Battery detected ({:.2f} V), enabling charging", battery.voltage_v);
+    tab5.set_charging_enabled(true);
+  } else {
+    logger.warn("No battery detected ({:.2f} V on the pack line), leaving charging disabled",
+                battery.voltage_v);
+    tab5.set_charging_enabled(false);
+  }
 
   logger.info("Initializing sound...");
   // initialize the sound

components/m5stack-tab5/include/m5stack-tab5.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <chrono>
 #include <cmath>
 #include <functional>
 #include <memory>
@@ -168,7 +169,10 @@ class M5StackTab5 : public BaseComponent {
     float power_mw;       ///< Battery power in milliwatts
     float charge_percent; ///< Estimated charge percentage (0-100)
     bool is_charging;     ///< True if battery is charging
-    bool is_present;      ///< True if battery is present
+    bool is_present;      ///< True if a battery is in the bay (pack voltage reads at least 4 V;
+                          ///< an empty bay reads the charger's ~2 V open-circuit output).
+                          ///< Leave charging disabled when false: the IP2326 retrying to
+                          ///< charge an empty bay puts a periodic pop on the speaker.
   };
 
   /// Expansion port configuration
@@ -599,8 +603,27 @@ class M5StackTab5 : public BaseComponent {
   /// Enable/disable battery charging (IP2326 CHG_EN on 0x44 P7)
   /// \param enable True to enable charging, false to disable
   /// \return true on success
+  /// \note This is the requested state. While speaker playback pauses charging
+  ///       (see set_charging_pause_on_playback_enabled()), a request to enable
+  ///       is remembered and applied when playback has stopped.
   bool set_charging_enabled(bool enable);
 
+  /// Whether speaker playback pauses battery charging (default: enabled).
+  /// With the IP2326 charger enabled, every audio transient on the speaker
+  /// comes with a pop, whatever the charge current (measured on the Tab5:
+  /// clean with CHG_EN low, popping with it high even when the pack is full
+  /// and the charger idles). So while audio is being played the charger is
+  /// switched off, and switched back on CHARGING_RESUME_DELAY_MS after the
+  /// last audio block; charging a pack between sounds is not affected.
+  /// Disabling this leaves the charger alone while audio plays.
+  /// \param enable True to pause charging during playback
+  void set_charging_pause_on_playback_enabled(bool enable);
+
+  /// \return Whether speaker playback pauses battery charging
+  bool charging_pause_on_playback_enabled() const {
+    return charging_pause_on_playback_enabled_.load(std::memory_order_relaxed);
+  }
+
   /// Switch the 5 V supply of the USB-A host jack (USB_5V_EN on the 0x44 IO
   /// expander). On by default after initialize_io_expanders(); turn it off and
   /// back on to power-cycle an attached device (e.g. so a device attached at
@@ -976,6 +999,20 @@ class M5StackTab5 : public BaseComponent {
   std::atomic<bool> battery_monitoring_initialized_{false};
   BatteryStatus battery_status_;
   std::mutex battery_mutex_;
+  // Charger vs. speaker (see set_charging_pause_on_playback_enabled()). The
+  // audio playback task reports whether each block carried audio; the charger
+  // is switched off on the first audio block and back on this long after the
+  // last one, so consecutive sounds do not toggle it.
+  static constexpr uint32_t CHARGING_RESUME_DELAY_MS = 3000;
+  /// Called by the playback task for every block; pauses / resumes charging.
+  void update_charging_for_audio(bool audio_active);
+  /// Resume charging now if playback paused it (teardown, feature disabled).
+  void resume_charging_after_audio();
+  std::mutex charging_mutex_;
+  bool charging_requested_{false};        // last set_charging_enabled() request
+  bool charging_paused_for_audio_{false}; // CHG_EN held low by playback
+  std::atomic<bool> charging_pause_on_playback_enabled_{true};
+  std::chrono::steady_clock::time_point audio_last_active_{}; // playback task only
   std::shared_ptr<I2c::Device<uint8_t>> battery_monitor_i2c_device_;
   std::shared_ptr<BatteryMonitor> battery_monitor_;
   // IO expanders on the internal I2C (addresses 0x43 and 0x44 per Tab5 design)

components/m5stack-tab5/src/audio.cpp

@@ -265,6 +265,8 @@ bool M5StackTab5::initialize_audio(uint32_t sample_rate,
     // play_audio()'s notifications: forget it before the task goes away
     audio_tx_feed_.detach_task();
     audio_task_->stop();
+    // the task may have paused charging on its first blocks
+    resume_charging_after_audio();
     return false;
   }
 
@@ -376,7 +378,7 @@ bool M5StackTab5::audio_task_callback(std::mutex &m, std::condition_variable &cv
   const size_t block = audio_tx_buffer.size();
   uint8_t *tx_buf = audio_tx_buffer.data();
   bool misaligned = false;
-  audio_tx_feed_.fill_block(tx_buf, block, misaligned);
+  const size_t audio_bytes = audio_tx_feed_.fill_block(tx_buf, block, misaligned);
   if (misaligned) {
     // Producers queue whole frames only (play_audio), so this is a bug
     // somewhere; the feeder reset the stream rather than leave a partial frame
@@ -386,6 +388,9 @@ bool M5StackTab5::audio_task_callback(std::mutex &m, std::condition_variable &cv
         audio_tx_feed_.misaligned_bytes,
         audio_tx_feed_.misaligned_bytes % audio_tx_feed_.frame_bytes, audio_tx_feed_.frame_bytes);
   }
+  // the charger pops the speaker while it is enabled: hold it off during
+  // playback (see set_charging_pause_on_playback_enabled)
+  update_charging_for_audio(audio_bytes > 0);
   i2s_channel_write(audio_tx_handle, tx_buf, block, NULL, portMAX_DELAY);
   return false;
 }

components/m5stack-tab5/src/m5stack-tab5.cpp

@@ -78,6 +78,12 @@ bool M5StackTab5::set_speaker_enabled(bool enable) {
 }
 
 bool M5StackTab5::set_charging_enabled(bool enable) {
+  std::lock_guard<std::mutex> lock(charging_mutex_);
+  charging_requested_ = enable;
+  if (enable && charging_paused_for_audio_) {
+    // playback holds the charger off; the request is applied when it resumes
+    return true;
+  }
   return set_io_expander_output(0x44, IO44_BIT_CHG_EN, enable);
 }
 

components/m5stack-tab5/src/power.cpp

@@ -1,3 +1,6 @@
+#include <chrono>
+#include <thread>
+
 #include "m5stack-tab5.hpp"
 
 namespace espp {
@@ -48,6 +51,38 @@ bool M5StackTab5::initialize_battery_monitoring() {
     return false;
   }
 
+  // The INA226's data registers read as zeros until its first averaged
+  // conversion completes (averaging x (bus + shunt conversion time): 35 ms with
+  // the settings above), and a 0 V reading looks exactly like an empty battery
+  // bay to read_battery_status(). Wait for the first conversion so the first
+  // read after this returns is real. The timeout is a generous multiple of the
+  // expected time so a slow I2C bus or a busy scheduler does not trip it.
+  {
+    static constexpr uint32_t conversion_poll_ms = 5;
+    static constexpr uint32_t conversion_timeout_factor = 4;
+    const uint32_t expected_ms =
+        BatteryMonitor::conversion_time_us(cfg.averaging, cfg.bus_conv_time, cfg.shunt_conv_time) /
+        1000;
+    const uint32_t timeout_ms =
+        std::max<uint32_t>(conversion_timeout_factor * expected_ms, 10 * conversion_poll_ms);
+    const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeout_ms);
+    bool ready = false;
+    while (!ready && std::chrono::steady_clock::now() < deadline) {
+      ready = battery_monitor_->conversion_ready(ec);
+      if (ec) {
+        break;
+      }
+      if (!ready) {
+        std::this_thread::sleep_for(std::chrono::milliseconds(conversion_poll_ms));
+      }
+    }
+    if (!ready) {
+      logger_.warn("Battery monitor (INA226) first conversion not ready after {} ms (expected "
+                   "{} ms){}{}",
+                   timeout_ms, expected_ms, ec ? ": " : "", ec ? ec.message() : "");
+    }
+  }
+
   battery_monitoring_initialized_ = true;
 
   // Initialize battery status with default values
@@ -82,6 +117,11 @@ bool M5StackTab5::update_battery_status() {
   static constexpr float num_cells = 2.0f;
   static constexpr float pack_voltage_min = cell_voltage_min * num_cells;
   static constexpr float pack_voltage_max = cell_voltage_max * num_cells;
+  // The INA226 sits on the pack line. With no NP-F550 in the bay it reads the
+  // charger's open-circuit output (about 2 V, measured); a real 2S pack never
+  // reads below its protection cut-off (about 5 V), so anything under this is
+  // an empty bay, not a flat battery.
+  static constexpr float pack_present_min_volts = 4.0f;
 
   std::error_code ec;
   std::lock_guard<std::mutex> lock(battery_mutex_);
@@ -94,14 +134,19 @@ bool M5StackTab5::update_battery_status() {
     battery_status_.voltage_v = vbus;
     battery_status_.current_ma = current_a * 1000.0f;
     battery_status_.power_mw = power_w * 1000.0f;
-    // Basic SoC estimate from voltage (very rough placeholder)
-    float v = vbus;
-    float soc = (v - pack_voltage_min) / (pack_voltage_max - pack_voltage_min);
-    soc = std::clamp(soc, 0.0f, 1.0f);
-    battery_status_.charge_percent = soc * 100.0f;
-    // only charging if the bit says we are and  the current is < -1.0 mA
-    battery_status_.is_charging = get_charging_status() && battery_status_.current_ma < -1.0f;
-    battery_status_.is_present = true;
+    battery_status_.is_present = vbus >= pack_present_min_volts;
+    if (battery_status_.is_present) {
+      // Basic SoC estimate from voltage (very rough placeholder)
+      float v = vbus;
+      float soc = (v - pack_voltage_min) / (pack_voltage_max - pack_voltage_min);
+      soc = std::clamp(soc, 0.0f, 1.0f);
+      battery_status_.charge_percent = soc * 100.0f;
+      // only charging if the bit says we are and the current is < -1.0 mA
+      battery_status_.is_charging = get_charging_status() && battery_status_.current_ma < -1.0f;
+    } else {
+      battery_status_.charge_percent = 0.0f;
+      battery_status_.is_charging = false;
+    }
   } else {
     logger_.warn("Battery status not updated: {}", ec.message());
     battery_status_.voltage_v = 0.0f;
@@ -122,6 +167,47 @@ M5StackTab5::BatteryStatus M5StackTab5::read_battery_status() {
   return get_battery_status();
 }
 
+void M5StackTab5::set_charging_pause_on_playback_enabled(bool enable) {
+  charging_pause_on_playback_enabled_.store(enable, std::memory_order_relaxed);
+  if (!enable) {
+    resume_charging_after_audio();
+  }
+}
+
+void M5StackTab5::update_charging_for_audio(bool audio_active) {
+  // Runs on the audio playback task once per block (about 60 Hz). The IO
+  // expander write happens only on the two transitions; the common case is a
+  // clock read and a compare.
+  if (!charging_pause_on_playback_enabled_.load(std::memory_order_relaxed)) {
+    return;
+  }
+  const auto now = std::chrono::steady_clock::now();
+  if (audio_active) {
+    audio_last_active_ = now;
+    std::lock_guard<std::mutex> lock(charging_mutex_);
+    if (!charging_paused_for_audio_) {
+      charging_paused_for_audio_ = true;
+      if (charging_requested_) {
+        set_io_expander_output(0x44, IO44_BIT_CHG_EN, false);
+      }
+    }
+    return;
+  }
+  if (now - audio_last_active_ >= std::chrono::milliseconds(CHARGING_RESUME_DELAY_MS)) {
+    resume_charging_after_audio();
+  }
+}
+
+void M5StackTab5::resume_charging_after_audio() {
+  std::lock_guard<std::mutex> lock(charging_mutex_);
+  if (charging_paused_for_audio_) {
+    charging_paused_for_audio_ = false;
+    if (charging_requested_) {
+      set_io_expander_output(0x44, IO44_BIT_CHG_EN, true);
+    }
+  }
+}
+
 void M5StackTab5::enable_battery_charging(bool enable) {
   // Control charging via IP2326 charge management IC This is done through the
   // PI4IOE5V6408 GPIO expander