Various fixes and improvements (#182)

- Fix for `logMutex` bug where the `Mutex` constructor is not called when doing early boot logging (with `DEBUG` level logging) . The only way to make it work is to explicitly call the constructor in the logging wrapper function.
- Fix for unPhone power states

Boards/UnPhone/Source/PowerOn.cpp

@@ -8,35 +8,72 @@ extern UnPhoneFeatures unPhoneFeatures;
 
 static std::unique_ptr<tt::Thread> powerThread;
 
+enum class PowerState {
+    Initial,
+    On,
+    Off
+};
+
+#define DEBUG_POWER_STATES false
+
+/** Helper method to use the buzzer to signal the different power stages */
+static void powerInfoBuzz(uint8_t count) {
+    if (DEBUG_POWER_STATES) {
+        uint8_t index = 0;
+        while (index < count) {
+            unPhoneFeatures.setVibePower(true);
+            tt::kernel::delayMillis(50);
+            unPhoneFeatures.setVibePower(false);
+
+            index++;
+
+            if (index < count) {
+                tt::kernel::delayMillis(100);
+            }
+        }
+    }
+}
+
 static void updatePowerSwitch() {
-    static bool last_on_state = true;
+    static PowerState last_state = PowerState::Initial;
 
     if (!unPhoneFeatures.isPowerSwitchOn()) {
-        if (last_on_state) {
+        if (last_state != PowerState::Off) {
+            last_state = PowerState::Off;
             TT_LOG_W(TAG, "Power off");
         }
 
-        unPhoneFeatures.turnPeripheralsOff();
-
         if (!unPhoneFeatures.isUsbPowerConnected()) { // and usb unplugged we go into shipping mode
-            if (last_on_state) {
+            TT_LOG_W(TAG, "Shipping mode until USB connects");
-                TT_LOG_W(TAG, "Shipping mode until USB connects");
-                unPhoneFeatures.setShipping(true); // tell BM to stop supplying power until USB connects
-            }
-        } else { // power switch off and usb plugged in we sleep
-            unPhoneFeatures.wakeOnPowerSwitch();
-            // Using UINT64_MAX leads to boot loops because of a bug in esp_sleep_start() converting it to int64_t before sleeping
-            esp_sleep_enable_timer_wakeup(UINT64_MAX / 2); // ea min: USB? else->shipping
-            esp_deep_sleep_start(); // deep sleep, wait for wakeup on GPIO
-        }
 
-        last_on_state = false;
+            unPhoneFeatures.setExpanderPower(true);
-    } else {
+            powerInfoBuzz(3);
-        if (!last_on_state) {
+            unPhoneFeatures.setExpanderPower(false);
-            TT_LOG_W(TAG, "Power on");
+
-            unPhoneFeatures.setShipping(false);
+            unPhoneFeatures.turnPeripheralsOff();
+
+            unPhoneFeatures.setShipping(true); // tell BM to stop supplying power until USB connects
+        } else { // When power switch is off, but USB is plugged in, we wait (deep sleep) until USB is unplugged.
+            TT_LOG_W(TAG, "Waiting for USB disconnect to power off");
+
+            powerInfoBuzz(2);
+            unPhoneFeatures.turnPeripheralsOff();
+
+            // Deep sleep for 1 minute, then awaken to check power state again
+            // GPIO trigger from power switch also awakens the device
+            unPhoneFeatures.wakeOnPowerSwitch();
+            esp_sleep_enable_timer_wakeup(60000000);
+            esp_deep_sleep_start();
+        }
+    } else {
+        if (last_state != PowerState::On) {
+            last_state = PowerState::On;
+            TT_LOG_W(TAG, "Power on");
+
+            unPhoneFeatures.setShipping(false);
+            unPhoneFeatures.setExpanderPower(true);
+            powerInfoBuzz(1);
         }
-        last_on_state = true;
     }
 }
 
@@ -65,22 +102,16 @@ static bool unPhonePowerOn() {
 
     unPhoneFeatures.printInfo();
 
-    // Vibrate once
+    unPhoneFeatures.setBacklightPower(false);
-    // Note: Do this before power switching logic, to detect silent boot loops
-    unPhoneFeatures.setVibePower(true);
-    tt::kernel::delayMillis(150);
     unPhoneFeatures.setVibePower(false);
+    unPhoneFeatures.setIrPower(false);
+    unPhoneFeatures.setExpanderPower(false);
 
     // Turn off the device if power switch is on off state,
     // instead of waiting for the Thread to start and continue booting
     updatePowerSwitch();
     startPowerSwitchThread();
 
-    unPhoneFeatures.setBacklightPower(false);
-    unPhoneFeatures.setVibePower(false);
-    unPhoneFeatures.setIrPower(false);
-    unPhoneFeatures.setExpanderPower(false);
-
     return true;
 }
 

Boards/UnPhone/Source/UnPhoneFeatures.cpp

@@ -168,12 +168,28 @@ bool UnPhoneFeatures::initGpioExpander() {
     assert(ioExpander != nullptr);
 
     // Output pins
+
+    /**
+     * Important:
+     * If you clear the pins too late, the display or vibration motor might briefly turn on.
+     */
+
     esp_io_expander_set_dir(ioExpander, expanderpin::BACKLIGHT, IO_EXPANDER_OUTPUT);
+    esp_io_expander_set_level(ioExpander, expanderpin::BACKLIGHT, 0);
+
     esp_io_expander_set_dir(ioExpander, expanderpin::EXPANDER_POWER, IO_EXPANDER_OUTPUT);
+
     esp_io_expander_set_dir(ioExpander, expanderpin::LED_GREEN, IO_EXPANDER_OUTPUT);
+    esp_io_expander_set_level(ioExpander, expanderpin::LED_GREEN, 0);
+
     esp_io_expander_set_dir(ioExpander, expanderpin::LED_BLUE, IO_EXPANDER_OUTPUT);
+    esp_io_expander_set_level(ioExpander, expanderpin::LED_BLUE, 0);
+
     esp_io_expander_set_dir(ioExpander, expanderpin::VIBE, IO_EXPANDER_OUTPUT);
+    esp_io_expander_set_level(ioExpander, expanderpin::VIBE, 0);
+
     // Input pins
+
     esp_io_expander_set_dir(ioExpander, expanderpin::USB_VSENSE, IO_EXPANDER_INPUT);
 
     return true;
@@ -266,16 +282,12 @@ void UnPhoneFeatures::turnPeripheralsOff() const {
 bool UnPhoneFeatures::setShipping(bool on) const {
     if (on) {
         TT_LOG_W(TAG, "setShipping: on");
-        uint8_t mask = (1 << 4) | (1 << 5);
+        batteryManagement.setWatchDogTimer(Bq24295::WatchDogTimer::Disabled);
-        // REG05[5:4] = 00
-        batteryManagement.setWatchDogBitOff(mask);
         // Set bit 5 to disable
         batteryManagement.setOperationControlBitOn(1 << 5);
     } else {
         TT_LOG_W(TAG, "setShipping: off");
-        // REG05[5:4] = 01
+        batteryManagement.setWatchDogTimer(Bq24295::WatchDogTimer::Enabled40s);
-        batteryManagement.setWatchDogBitOff(1 << 5);
-        batteryManagement.setWatchDogBitOn(1 << 4);
         // Clear bit 5 to enable
         batteryManagement.setOperationControlBitOff(1 << 5);
     }
@@ -287,10 +299,5 @@ void UnPhoneFeatures::wakeOnPowerSwitch() const {
 }
 
 bool UnPhoneFeatures::isUsbPowerConnected() const {
-    uint8_t status;
+    return batteryManagement.isUsbPowerConnected();
-    if (batteryManagement.getStatus(status)) {
-        return (status & 4U) != 0U;
-    } else {
-        return false;
-    }
 }

Boards/UnPhone/Source/bq24295/Bq24295.cpp

@@ -8,24 +8,55 @@
  * https://gitlab.com/hamishcunningham/unphonelibrary/-/blob/main/unPhone.h?ref_type=heads
  */
 namespace registers {
-    static const uint8_t WATCHDOG = 0x05U; // Datasheet page 35: Charge end/timer cntrl
+    static const uint8_t CHARGE_TERMINATION = 0x05U; // Datasheet page 35: Charge end/timer cntrl
     static const uint8_t OPERATION_CONTROL = 0x07U; // Datasheet page 37: Misc operation control
     static const uint8_t STATUS = 0x08U; // Datasheet page 38: System status
     static const uint8_t VERSION = 0x0AU; // Datasheet page 38: Vendor/part/revision status
 } // namespace registers
 
+bool Bq24295::readChargeTermination(uint8_t& out) const {
+    return readRegister8(registers::CHARGE_TERMINATION, out);
+}
+
 // region Watchdog
-bool Bq24295::getWatchDog(uint8_t value) const {
+bool Bq24295::getWatchDogTimer(WatchDogTimer& out) const {
-    return readRegister8(registers::WATCHDOG, value);
+    uint8_t value;
+    if (readChargeTermination(value)) {
+        uint8_t relevant_bits = value & (BIT(4) | BIT(5));
+        switch (relevant_bits) {
+            case 0b000000:
+                out = WatchDogTimer::Disabled;
+                return true;
+            case 0b010000:
+                out = WatchDogTimer::Enabled40s;
+                return true;
+            case 0b100000:
+                out = WatchDogTimer::Enabled80s;
+                return true;
+            case 0b110000:
+                out = WatchDogTimer::Enabled160s;
+                return true;
+            default:
+                return false;
+        }
+    }
+
+    return false;
 }
 
-bool Bq24295::setWatchDogBitOn(uint8_t mask) const {
+bool Bq24295::setWatchDogTimer(WatchDogTimer in) const {
-    return bitOn(registers::WATCHDOG, mask);
+    uint8_t value;
+    if (readChargeTermination(value)) {
+        uint8_t bits_to_set = 0b00110000 & static_cast<uint8_t>(in);
+        uint8_t value_cleared = value & 0b11001111;
+        uint8_t to_set = bits_to_set & value_cleared;
+        TT_LOG_I(TAG, "WatchDogTimer: %02x -> %02x", value, to_set);
+        return writeRegister8(registers::CHARGE_TERMINATION, to_set);
+    }
+
+    return false;
 }
 
-bool Bq24295::setWatchDogBitOff(uint8_t mask) const {
-    return bitOff(registers::WATCHDOG, mask);
-}
 
 // endregoin
 
@@ -51,14 +82,23 @@ bool Bq24295::getStatus(uint8_t& value) const {
     return readRegister8(registers::STATUS, value);
 }
 
+bool Bq24295::isUsbPowerConnected() const {
+    uint8_t status;
+    if (getStatus(status)) {
+        return (status & BIT(2)) != 0U;
+    } else {
+        return false;
+    }
+}
+
 bool Bq24295::getVersion(uint8_t& value) const {
     return readRegister8(registers::VERSION, value);
 }
 
 void Bq24295::printInfo() const {
-    uint8_t version, status;
+    uint8_t version, status, charge_termination;
-    if (getStatus(status) && getVersion(version)) {
+    if (getStatus(status) && getVersion(version) && readChargeTermination(charge_termination)) {
-        TT_LOG_I(TAG, "Version %d, status %02x", version, status);
+        TT_LOG_I(TAG, "Version %d, status %02x, charge termination %02x", version, status, charge_termination);
     } else {
         TT_LOG_E(TAG, "Failed to retrieve version and/or status");
     }

Boards/UnPhone/Source/bq24295/Bq24295.h

@@ -6,13 +6,25 @@
 
 class Bq24295 : I2cDevice {
 
+private:
+
+    bool readChargeTermination(uint8_t& out) const;
+
 public:
 
+    enum class WatchDogTimer {
+        Disabled = 0b000000,
+        Enabled40s = 0b010000,
+        Enabled80s = 0b100000,
+        Enabled160s = 0b110000
+    };
+
     explicit Bq24295(i2c_port_t port) : I2cDevice(port, BQ24295_ADDRESS) {}
 
-    bool getWatchDog(uint8_t value) const;
+    bool getWatchDogTimer(WatchDogTimer& out) const;
-    bool setWatchDogBitOn(uint8_t mask) const;
+    bool setWatchDogTimer(WatchDogTimer in) const;
-    bool setWatchDogBitOff(uint8_t mask) const;
+
+    bool isUsbPowerConnected() const;
 
     bool getOperationControl(uint8_t value) const;
     bool setOperationControlBitOn(uint8_t mask) const;

TactilityCore/Source/Log.cpp

@@ -6,7 +6,19 @@ namespace tt {
 
 static LogEntry* logEntries = nullptr;
 static unsigned int nextLogEntryIndex;
-static Mutex logMutex;
+
+/**
+ * This used to be a simple static value, but that crashes on device boot where early logging happens.
+ * For some unknown reason, the static Mutex instance wouldn't have their constructor called before
+ * the mutex is used.
+ */
+Mutex& getLogMutex() {
+    static Mutex* logMutex = nullptr;
+    if (logMutex == nullptr) {
+        logMutex = new Mutex();
+    }
+    return *logMutex;
+}
 
 static void ensureLogEntriesExist() {
     if (logEntries == nullptr) {
@@ -17,7 +29,7 @@ static void ensureLogEntriesExist() {
 }
 
 static void storeLog(LogLevel level, const char* format, va_list args) {
-    if (logMutex.lock(5 / portTICK_PERIOD_MS)) {
+    if (getLogMutex().lock(5 / portTICK_PERIOD_MS)) {
         ensureLogEntriesExist();
 
         logEntries[nextLogEntryIndex].level = level;
@@ -28,19 +40,19 @@ static void storeLog(LogLevel level, const char* format, va_list args) {
             nextLogEntryIndex = 0;
         }
 
-        logMutex.unlock();
+        getLogMutex().unlock();
     }
 }
 
 LogEntry* copyLogEntries(unsigned int& outIndex) {
-    if (logMutex.lock(5 / portTICK_PERIOD_MS)) {
+    if (getLogMutex().lock(5 / portTICK_PERIOD_MS)) {
         auto* newEntries = new LogEntry[TT_LOG_ENTRY_COUNT];
         assert(newEntries != nullptr);
         for (int i = 0; i < TT_LOG_ENTRY_COUNT; ++i) {
             memcpy(&newEntries[i], &logEntries[i], sizeof(LogEntry));
         }
         outIndex = nextLogEntryIndex;
-        logMutex.unlock();
+        getLogMutex().unlock();
         return newEntries;
     } else {
         return nullptr;

TactilityCore/Source/Mutex.cpp

@@ -46,7 +46,7 @@ Mutex::~Mutex() {
 
 TtStatus Mutex::acquire(TickType_t timeout) const {
     tt_assert(!TT_IS_IRQ_MODE());
-    tt_assert(semaphore);
+    tt_assert(semaphore != nullptr);
 
     tt_mutex_info(mutex, "acquire");