Battery updates

lib/pbio/drv/charger/charger_mp2639a.c

@@ -22,12 +22,9 @@
 #include <pbdrv/adc.h>
 #include <pbdrv/charger.h>
 #include <pbdrv/gpio.h>
-#if PBDRV_CONFIG_CHARGER_MP2639A_MODE_PWM | PBDRV_CONFIG_CHARGER_MP2639A_ISET_PWM
 #include <pbdrv/pwm.h>
-#endif
-#if PBDRV_CONFIG_CHARGER_MP2639A_CHG_RESISTOR_LADDER
 #include <pbdrv/resistor_ladder.h>
-#endif
+#include <pbdrv/usb.h>
 #include <pbio/error.h>
 #include <pbio/util.h>
 
@@ -70,7 +67,13 @@ pbdrv_charger_status_t pbdrv_charger_get_status(void) {
     return pbdrv_charger_status;
 }
 
-void pbdrv_charger_enable(bool enable, pbdrv_charger_limit_t limit) {
+/**
+ * Enables or disables the charger.
+ *
+ * @param enable    True to enable the charger, false to disable.
+ * @param limit     The current limit to set. Only used on some platforms.
+ */
+static void pbdrv_charger_enable(bool enable, pbdrv_charger_limit_t limit) {
     #if PBDRV_CONFIG_CHARGER_MP2639A_ISET_PWM
 
     // Set the current limit (ISET) based on the type of charger attached.
@@ -116,6 +119,32 @@ void pbdrv_charger_enable(bool enable, pbdrv_charger_limit_t limit) {
     mode_pin_is_low = enable;
 }
 
+/**
+ * Enables the charger if USB is connected, otherwise disables charger.
+ */
+static void pbdrv_charger_enable_if_usb_connected(void) {
+    pbdrv_usb_bcd_t bcd = pbdrv_usb_get_bcd();
+    bool enable = bcd != PBDRV_USB_BCD_NONE;
+    pbdrv_charger_limit_t limit;
+
+    // This battery charger chip will automatically monitor VBUS and
+    // limit the current if the VBUS voltage starts to drop, so these limits
+    // are a bit looser than they could be.
+    switch (bcd) {
+        case PBDRV_USB_BCD_NONE:
+            limit = PBDRV_CHARGER_LIMIT_NONE;
+            break;
+        case PBDRV_USB_BCD_STANDARD_DOWNSTREAM:
+            limit = PBDRV_CHARGER_LIMIT_STD_MAX;
+            break;
+        default:
+            limit = PBDRV_CHARGER_LIMIT_CHARGING;
+            break;
+    }
+
+    pbdrv_charger_enable(enable, limit);
+}
+
 /**
  * Gets the current CHG signal status (inverted compared to /CHG pin state).
  */
@@ -135,6 +164,24 @@ static bool read_chg(void) {
     #endif
 }
 
+// Sample CHG signal at 4Hz to capture transitions to detect fault condition.
+#define PBDRV_CHARGER_MP2639A_STATUS_SAMPLE_TIME (250)
+
+// After charging for a long time, we disable charging for some time. This
+// matches observed behavior with the LEGO Education SPIKE V3.x firmware.
+//
+// Why? Due to the way the hardware works, the hub cannot be truly turned off
+// while USB is plugged in. As a result, the charger is always on. For some
+// battery-charger pairs, this causes the battery to stop charging normally in
+// hardware when full as intended, automatically starting a new cycle after
+// some time. But in some battery-charger pairs, the charger will reach a
+// timeout state and not restart charging. When leaving such a combination
+// plugged in overnight, it will time out and not be charged in the morning,
+// which is not desirable. For this reason, we pause and restart charging
+// manually if it has been plugged in for a long time.
+#define PBDRV_CHARGER_MP2639A_CHARGE_TIMEOUT_MS (60 * 60 * 1000)
+#define PBDRV_CHARGER_MP2639A_CHARGE_PAUSE_MS (30 * 1000)
+
 PROCESS_THREAD(pbdrv_charger_mp2639a_process, ev, data) {
     PROCESS_BEGIN();
 
@@ -166,17 +213,24 @@ PROCESS_THREAD(pbdrv_charger_mp2639a_process, ev, data) {
     static bool chg_samples[7];
     static uint8_t chg_index = 0;
     static struct etimer timer;
-
-    // sample at 4Hz
-    etimer_set(&timer, 250);
+    static uint32_t charge_count = 0;
 
     for (;;) {
+        etimer_set(&timer, PBDRV_CHARGER_MP2639A_STATUS_SAMPLE_TIME);
         PROCESS_WAIT_EVENT_UNTIL(ev == PROCESS_EVENT_TIMER && etimer_expired(&timer));
-        etimer_restart(&timer);
+
+        // Enable charger chip based on USB state. We don't need to disable it
+        // on charger fault since the chip will automatically disable itself.
+        // If we disable it here we can't detect the fault condition.
+        pbdrv_charger_enable_if_usb_connected();
 
         chg_samples[chg_index] = read_chg();
 
         if (mode_pin_is_low) {
+
+            // Keep track of how long we have been charging.
+            charge_count++;
+
             // Count number of transitions seen during sampling window.
             int transitions = chg_samples[0] != chg_samples[PBIO_ARRAY_SIZE(chg_samples) - 1];
             for (size_t i = 1; i < PBIO_ARRAY_SIZE(chg_samples); i++) {
@@ -205,12 +259,22 @@ PROCESS_THREAD(pbdrv_charger_mp2639a_process, ev, data) {
             // devices) requires a momentary pulse on the /PB pin, which is
             // not wired up.
             pbdrv_charger_status = PBDRV_CHARGER_STATUS_DISCHARGE;
+            charge_count = 0;
         }
 
         // Increment sampling index with wrap around.
         if (++chg_index >= PBIO_ARRAY_SIZE(chg_samples)) {
             chg_index = 0;
         }
+
+        // If we have been charging for a long time, pause charging for a while.
+        if (charge_count > (PBDRV_CHARGER_MP2639A_CHARGE_TIMEOUT_MS / PBDRV_CHARGER_MP2639A_STATUS_SAMPLE_TIME)) {
+            pbdrv_charger_status = PBDRV_CHARGER_STATUS_DISCHARGE;
+            pbdrv_charger_enable(false, PBDRV_CHARGER_LIMIT_NONE);
+            etimer_set(&timer, PBDRV_CHARGER_MP2639A_CHARGE_PAUSE_MS);
+            PROCESS_WAIT_EVENT_UNTIL(etimer_expired(&timer));
+            charge_count = 0;
+        }
     }
 
     PROCESS_END();

lib/pbio/include/pbdrv/charger.h

@@ -72,13 +72,6 @@ pbio_error_t pbdrv_charger_get_current_now(uint16_t *current);
  */
 pbdrv_charger_status_t pbdrv_charger_get_status(void);
 
-/**
- * Enables or disables charging.
- * @param [in]  enable  True to enable charging or false for discharging.
- * @param [in]  limit   The current limit for the charging rate.
- */
-void pbdrv_charger_enable(bool enable, pbdrv_charger_limit_t limit);
-
 #else
 
 static inline pbio_error_t pbdrv_charger_get_current_now(uint16_t *current) {
@@ -90,9 +83,6 @@ static inline pbdrv_charger_status_t pbdrv_charger_get_status(void) {
     return PBDRV_CHARGER_STATUS_FAULT;
 }
 
-static inline void pbdrv_charger_enable(bool enable, pbdrv_charger_limit_t limit) {
-}
-
 #endif
 
 #endif // _PBDRV_CHARGER_H_

lib/pbio/platform/prime_hub/platform.c

@@ -139,7 +139,7 @@ const pbdrv_bluetooth_btstack_platform_data_t pbdrv_bluetooth_btstack_platform_d
 
 const pbdrv_charger_mp2639a_platform_data_t pbdrv_charger_mp2639a_platform_data = {
     .mode_pwm_id = PWM_DEV_5_TLC5955,
-    .mode_pwm_ch = 15,
+    .mode_pwm_ch = 14,
     .chg_resistor_ladder_id = RESISTOR_LADDER_DEV_0,
     .chg_resistor_ladder_ch = PBDRV_RESISTOR_LADDER_CH_2,
     .ib_adc_ch = 3,

lib/pbio/src/battery.c

@@ -14,6 +14,10 @@
 #include <pbio/battery.h>
 #include <pbio/int_math.h>
 
+#if !PBIO_CONFIG_MOTOR_PROCESS
+#error "PBIO_CONFIG_MOTOR_PROCESS must be enabled to continously update battery voltage."
+#endif
+
 // Slow moving average battery voltage.
 static int32_t battery_voltage_avg_scaled;
 

lib/pbio/sys/battery.c

@@ -7,15 +7,13 @@
 // TODO: need to handle battery pack switch and Li-ion batteries for Technic Hub and NXT
 
 #include <pbdrv/battery.h>
+#include <pbio/battery.h>
 #include <pbdrv/charger.h>
 #include <pbdrv/config.h>
 #include <pbdrv/clock.h>
 #include <pbdrv/usb.h>
 #include <pbsys/status.h>
 
-// period over which the battery voltage is averaged (in milliseconds)
-#define BATTERY_PERIOD_MS       2500
-
 // These values are for Alkaline (AA/AAA) batteries
 #define BATTERY_OK_MV           6000    // 1.0V per cell
 #define BATTERY_LOW_MV          5400    // 0.9V per cell
@@ -27,46 +25,7 @@
 #define LIION_LOW_MV            6800    // 3.4V per cell
 #define LIION_CRITICAL_MV       6000    // 3.0V per cell
 
-static uint32_t prev_poll_time;
-static uint16_t avg_battery_voltage;
-
-#if PBDRV_CONFIG_BATTERY_ADC_TYPE == 1
-// special case to reduce code size on Move hub
-#define battery_critical_mv BATTERY_CRITICAL_MV
-#define battery_low_mv BATTERY_LOW_MV
-#define battery_ok_mv BATTERY_OK_MV
-#else
-static uint16_t battery_critical_mv;
-static uint16_t battery_low_mv;
-static uint16_t battery_ok_mv;
-#endif
-
-/**
- * Initializes the system battery monitor.
- */
 void pbsys_battery_init(void) {
-    #if PBDRV_CONFIG_BATTERY_ADC_TYPE != 1
-    pbdrv_battery_type_t type;
-    if (pbdrv_battery_get_type(&type) == PBIO_SUCCESS && type == PBDRV_BATTERY_TYPE_LIION) {
-        battery_critical_mv = LIION_CRITICAL_MV;
-        battery_low_mv = LIION_LOW_MV;
-        battery_ok_mv = LIION_OK_MV;
-    } else {
-        battery_critical_mv = BATTERY_CRITICAL_MV;
-        battery_low_mv = BATTERY_LOW_MV;
-        battery_ok_mv = BATTERY_OK_MV;
-    }
-    #endif
-
-    pbdrv_battery_get_voltage_now(&avg_battery_voltage);
-    // This is mainly for the Technic Hub. It seems that the first battery voltage
-    // read is always low and causes the hub to shut down because of low battery
-    // voltage even though the battery isn't that low.
-    if (avg_battery_voltage < battery_critical_mv) {
-        avg_battery_voltage = battery_ok_mv;
-    }
-
-    prev_poll_time = pbdrv_clock_get_ms();
 }
 
 /**
@@ -75,18 +34,15 @@ void pbsys_battery_init(void) {
  * This is called periodically to update the current battery state.
  */
 void pbsys_battery_poll(void) {
-    uint32_t now;
-    uint32_t poll_interval;
-    uint16_t battery_voltage;
 
-    now = pbdrv_clock_get_ms();
-    poll_interval = now - prev_poll_time;
-    prev_poll_time = now;
+    pbdrv_battery_type_t type;
+    bool is_liion = pbdrv_battery_get_type(&type) == PBIO_SUCCESS && type == PBDRV_BATTERY_TYPE_LIION;
 
-    pbdrv_battery_get_voltage_now(&battery_voltage);
+    uint32_t battery_critical_mv = is_liion ? LIION_CRITICAL_MV : BATTERY_CRITICAL_MV;
+    uint32_t battery_low_mv = is_liion ? LIION_LOW_MV : BATTERY_LOW_MV;
+    uint32_t battery_ok_mv = is_liion ? LIION_OK_MV : BATTERY_OK_MV;
 
-    avg_battery_voltage = (avg_battery_voltage * (BATTERY_PERIOD_MS - poll_interval)
-        + battery_voltage * poll_interval) / BATTERY_PERIOD_MS;
+    uint32_t avg_battery_voltage = pbio_battery_get_average_voltage();
 
     if (avg_battery_voltage <= battery_critical_mv) {
         pbsys_status_set(PBIO_PYBRICKS_STATUS_BATTERY_LOW_VOLTAGE_SHUTDOWN);
@@ -99,32 +55,6 @@ void pbsys_battery_poll(void) {
     } else if (avg_battery_voltage >= battery_ok_mv) {
         pbsys_status_clear(PBIO_PYBRICKS_STATUS_BATTERY_LOW_VOLTAGE_WARNING);
     }
-
-    // REVISIT: we should be able to make this event driven rather than polled
-    #if PBDRV_CONFIG_CHARGER
-
-    pbdrv_usb_bcd_t bcd = pbdrv_usb_get_bcd();
-    bool enable = bcd != PBDRV_USB_BCD_NONE;
-    pbdrv_charger_limit_t limit;
-
-    // REVISIT: The only current battery charger chip will automatically monitor
-    // VBUS and limit the current if the VBUS voltage starts to drop, so these
-    // limits are a bit looser than they could be.
-    switch (bcd) {
-        case PBDRV_USB_BCD_NONE:
-            limit = PBDRV_CHARGER_LIMIT_NONE;
-            break;
-        case PBDRV_USB_BCD_STANDARD_DOWNSTREAM:
-            limit = PBDRV_CHARGER_LIMIT_STD_MAX;
-            break;
-        default:
-            limit = PBDRV_CHARGER_LIMIT_CHARGING;
-            break;
-    }
-
-    pbdrv_charger_enable(enable, limit);
-
-    #endif // PBDRV_CONFIG_CHARGER
 }
 
 /**
@@ -133,5 +63,5 @@ void pbsys_battery_poll(void) {
  * This is only valid on hubs with a built-in battery charger.
  */
 bool pbsys_battery_is_full(void) {
-    return avg_battery_voltage >= LIION_FULL_MV;
+    return pbio_battery_get_average_voltage() >= LIION_FULL_MV;
 }