I2C improvements and fixes (#201)

- Show I2C device name in I2C Scanner app
- Register various I2C devices from board implementations
- Fix M5Stack Core2 power status
- Fix pre-allocation issue in `hal::Device`

Boards/M5stackCore2/Source/InitBoot.cpp

@@ -18,11 +18,19 @@
 axp192_t axpDevice;
 
 static int32_t axpI2cRead(TT_UNUSED void* handle, uint8_t address, uint8_t reg, uint8_t* buffer, uint16_t size) {
-    return tt::hal::i2c::masterReadRegister(I2C_NUM_0, address, reg, buffer, size, 50 / portTICK_PERIOD_MS);
+    if (tt::hal::i2c::masterReadRegister(I2C_NUM_0, address, reg, buffer, size, 50 / portTICK_PERIOD_MS)) {
+        return AXP192_OK;
+    } else {
+        return 1;
+    }
 }
 
 static int32_t axpI2cWrite(TT_UNUSED void* handle, uint8_t address, uint8_t reg, const uint8_t* buffer, uint16_t size) {
-    return tt::hal::i2c::masterWriteRegister(I2C_NUM_0, address, reg, buffer, size, 50 / portTICK_PERIOD_MS);
+    if (tt::hal::i2c::masterWriteRegister(I2C_NUM_0, address, reg, buffer, size, 50 / portTICK_PERIOD_MS)) {
+        return AXP192_OK;
+    } else {
+        return 1;
+    }
 }
 
 static bool initSpi2() {

Boards/M5stackCoreS3/Source/InitBoot.cpp

@@ -14,6 +14,9 @@
 #define CORES3_SPI2_PIN_MOSI GPIO_NUM_37
 #define CORES3_SPI2_PIN_MISO GPIO_NUM_35
 
+std::shared_ptr<Axp2101> axp2101;
+std::shared_ptr<Aw9523> aw9523;
+
 /**
  * For details see https://github.com/espressif/esp-bsp/blob/master/bsp/m5stack_core_s3/m5stack_core_s3.c
  */
@@ -92,22 +95,19 @@ bool initGpioExpander() {
     // Boost enable
     p1_state |= (1U << 7U);
 
-    Aw9523 aw(I2C_NUM_0);
+    if (!aw9523->writeP0(p0_state)) {
-
-    if (!aw.writeP0(p0_state)) {
         TT_LOG_E(TAG, "AW9523: Failed to set P0");
         return false;
     }
 
-    if (!aw.writeP1(p1_state)) {
+    if (!aw9523->writeP1(p1_state)) {
         TT_LOG_E(TAG, "AW9523: Failed to set P1");
         return false;
     }
 
-    Axp2101 axp(I2C_NUM_0);
+    if (axp2101->isVBus()) {
-    if (axp.isVBus()) {
         float voltage = 0.0f;
-        axp.getVBusVoltage(voltage);
+        axp2101->getVBusVoltage(voltage);
         TT_LOG_I(TAG, "AXP2101: VBus at %.2f", voltage);
     } else {
         TT_LOG_W(TAG, "AXP2101: VBus disabled");
@@ -119,9 +119,8 @@ bool initGpioExpander() {
 bool initPowerControl() {
     TT_LOG_I(TAG, "Init power control (AXP2101)");
 
-    Aw9523 aw(I2C_NUM_0);
     // Source: https://github.com/m5stack/M5Unified/blob/b8cfec7fed046242da7f7b8024a4e92004a51ff7/src/utility/Power_Class.cpp#L61
-    aw.bitOnP1(0b10000000); // SY7088 boost enable
+    aw9523->bitOnP1(0b10000000); // SY7088 boost enable
 
     /** AXP2101 usage
     Source: https://github.com/m5stack/M5Unified/blob/b8cfec7fed046242da7f7b8024a4e92004a51ff7/README.md?plain=1#L223
@@ -168,8 +167,7 @@ bool initPowerControl() {
         0x30, 0x0F // ADC enabled (for voltage measurement)
     };
 
-    Axp2101 axp(I2C_NUM_0);
+    if (axp2101->setRegisters((uint8_t*)reg_data_array, sizeof(reg_data_array))) {
-    if (axp.setRegisters((uint8_t*)reg_data_array, sizeof(reg_data_array))) {
         TT_LOG_I(TAG, "AXP2101 initialized with %d registers", sizeof(reg_data_array) / 2);
         return true;
     } else {
@@ -180,6 +178,12 @@ bool initPowerControl() {
 
 bool initBoot() {
     TT_LOG_I(TAG, "initBoot()");
+
+    axp2101 = std::make_shared<Axp2101>(I2C_NUM_0);
+    tt::hal::registerDevice(axp2101);
+    aw9523 = std::make_shared<Aw9523>(I2C_NUM_0);
+    tt::hal::registerDevice(aw9523);
+
     return initPowerControl() &&
         initGpioExpander() &&
         initSpi3();

Boards/M5stackCoreS3/Source/hal/CoreS3Power.cpp

@@ -21,7 +21,7 @@ bool CoreS3Power::getMetric(Power::MetricType type, Power::MetricData& data) {
         using enum MetricType;
         case BatteryVoltage: {
             float milliVolt;
-            if (axpDevice.getBatteryVoltage(milliVolt)) {
+            if (axpDevice->getBatteryVoltage(milliVolt)) {
                 data.valueAsUint32 = (uint32_t)milliVolt;
                 return true;
             } else {
@@ -30,7 +30,7 @@ bool CoreS3Power::getMetric(Power::MetricType type, Power::MetricData& data) {
         }
         case ChargeLevel: {
             float vbatMillis;
-            if (axpDevice.getBatteryVoltage(vbatMillis)) {
+            if (axpDevice->getBatteryVoltage(vbatMillis)) {
                 float vbat = vbatMillis / 1000.f;
                 float max_voltage = 4.20f;
                 float min_voltage = 2.69f; // From M5Unified
@@ -47,7 +47,7 @@ bool CoreS3Power::getMetric(Power::MetricType type, Power::MetricData& data) {
         }
         case IsCharging: {
             Axp2101::ChargeStatus status;
-            if (axpDevice.getChargeStatus(status)) {
+            if (axpDevice->getChargeStatus(status)) {
                 data.valueAsBool = (status == Axp2101::CHARGE_STATUS_CHARGING);
                 return true;
             } else {
@@ -61,7 +61,7 @@ bool CoreS3Power::getMetric(Power::MetricType type, Power::MetricData& data) {
 
 bool CoreS3Power::isAllowedToCharge() const {
     bool enabled;
-    if (axpDevice.isChargingEnabled(enabled)) {
+    if (axpDevice->isChargingEnabled(enabled)) {
         return enabled;
     } else {
         return false;
@@ -69,14 +69,16 @@ bool CoreS3Power::isAllowedToCharge() const {
 }
 
 void CoreS3Power::setAllowedToCharge(bool canCharge) {
-    axpDevice.setChargingEnabled(canCharge);
+    axpDevice->setChargingEnabled(canCharge);
 }
 
 static std::shared_ptr<Power> power;
+extern std::shared_ptr<Axp2101> axp2101;
 
 std::shared_ptr<Power> createPower() {
     if (power == nullptr) {
-        power = std::make_shared<CoreS3Power>();
+        power = std::make_shared<CoreS3Power>(axp2101);
     }
+
     return power;
 }

Boards/M5stackCoreS3/Source/hal/CoreS3Power.h

@@ -4,16 +4,17 @@
 
 #include <Tactility/hal/Power.h>
 #include <memory>
+#include <utility>
 
 using namespace tt::hal;
 
-class CoreS3Power : public Power {
+class CoreS3Power final : public Power {
 
-    Axp2101 axpDevice = Axp2101(I2C_NUM_0);
+    std::shared_ptr<Axp2101> axpDevice;
 
 public:
 
-    CoreS3Power() = default;
+    explicit CoreS3Power(std::shared_ptr<Axp2101> axp) : axpDevice(std::move(axp)) {}
     ~CoreS3Power() override = default;
 
     std::string getName() const final { return "AXP2101 Power"; }

Boards/UnPhone/Source/PowerOn.cpp

@@ -5,8 +5,7 @@
 
 #define TAG "unphone"
 
-extern UnPhoneFeatures unPhoneFeatures;
+std::shared_ptr<UnPhoneFeatures> unPhoneFeatures;
-
 static std::unique_ptr<tt::Thread> powerThread;
 
 static const char* bootCountKey = "boot_count";
@@ -89,13 +88,13 @@ static void powerInfoBuzz(uint8_t count) {
 static void updatePowerSwitch() {
     static PowerState last_state = PowerState::Initial;
 
-    if (!unPhoneFeatures.isPowerSwitchOn()) {
+    if (!unPhoneFeatures->isPowerSwitchOn()) {
         if (last_state != PowerState::Off) {
             last_state = PowerState::Off;
             TT_LOG_W(TAG, "Power off");
         }
 
-        if (!unPhoneFeatures.isUsbPowerConnected()) { // and usb unplugged we go into shipping mode
+        if (!unPhoneFeatures->isUsbPowerConnected()) { // and usb unplugged we go into shipping mode
             TT_LOG_W(TAG, "Shipping mode until USB connects");
 
 #if DEBUG_POWER_STATES
@@ -104,11 +103,11 @@ static void updatePowerSwitch() {
             unPhoneFeatures.setExpanderPower(false);
 #endif
 
-            unPhoneFeatures.turnPeripheralsOff();
+            unPhoneFeatures->turnPeripheralsOff();
 
             bootStats.notifyPowerOff();
 
-            unPhoneFeatures.setShipping(true); // tell BM to stop supplying power until USB connects
+            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");
 
@@ -116,13 +115,13 @@ static void updatePowerSwitch() {
             powerInfoBuzz(2);
 #endif
 
-            unPhoneFeatures.turnPeripheralsOff();
+            unPhoneFeatures->turnPeripheralsOff();
 
             bootStats.notifyPowerSleep();
 
             // Deep sleep for 1 minute, then awaken to check power state again
             // GPIO trigger from power switch also awakens the device
-            unPhoneFeatures.wakeOnPowerSwitch();
+            unPhoneFeatures->wakeOnPowerSwitch();
             esp_sleep_enable_timer_wakeup(60000000);
             esp_deep_sleep_start();
         }
@@ -155,23 +154,29 @@ static void startPowerSwitchThread() {
     powerThread->start();
 }
 
+std::shared_ptr<Bq24295> bq24295;
+
 static bool unPhonePowerOn() {
     // Print early, in case of early crash (info will be from previous boot)
     bootStats.printInfo();
-
     bootStats.notifyBootStart();
 
-    if (!unPhoneFeatures.init()) {
+    bq24295 = std::make_shared<Bq24295>(I2C_NUM_0);
+    tt::hal::registerDevice(bq24295);
+
+    unPhoneFeatures = std::make_shared<UnPhoneFeatures>(bq24295);
+
+    if (!unPhoneFeatures->init()) {
         TT_LOG_E(TAG, "UnPhoneFeatures init failed");
         return false;
     }
 
-    unPhoneFeatures.printInfo();
+    unPhoneFeatures->printInfo();
 
-    unPhoneFeatures.setBacklightPower(false);
+    unPhoneFeatures->setBacklightPower(false);
-    unPhoneFeatures.setVibePower(false);
+    unPhoneFeatures->setVibePower(false);
-    unPhoneFeatures.setIrPower(false);
+    unPhoneFeatures->setIrPower(false);
-    unPhoneFeatures.setExpanderPower(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

Boards/UnPhone/Source/UnPhone.cpp

@@ -8,9 +8,6 @@ bool unPhoneInitPower();
 bool unPhoneInitHardware();
 bool unPhoneInitLvgl();
 
-// Shared object, used in PowerOn and UnPhoneDisplay
-UnPhoneFeatures unPhoneFeatures;
-
 extern const tt::hal::Configuration unPhone = {
     .initBoot = unPhoneInitPower,
     .initHardware = unPhoneInitHardware,

Boards/UnPhone/Source/UnPhoneFeatures.cpp

@@ -223,7 +223,7 @@ bool UnPhoneFeatures::init() {
 
 void UnPhoneFeatures::printInfo() const {
     esp_io_expander_print_state(ioExpander);
-    batteryManagement.printInfo();
+    batteryManagement->printInfo();
     bool backlight_power;
     const char* backlight_power_state = getBacklightPower(backlight_power) && backlight_power ? "on" : "off";
     TT_LOG_I(TAG, "Backlight: %s", backlight_power_state);
@@ -282,12 +282,12 @@ void UnPhoneFeatures::turnPeripheralsOff() const {
 bool UnPhoneFeatures::setShipping(bool on) const {
     if (on) {
         TT_LOG_W(TAG, "setShipping: on");
-        batteryManagement.setWatchDogTimer(Bq24295::WatchDogTimer::Disabled);
+        batteryManagement->setWatchDogTimer(Bq24295::WatchDogTimer::Disabled);
-        batteryManagement.setBatFetOn(false);
+        batteryManagement->setBatFetOn(false);
     } else {
         TT_LOG_W(TAG, "setShipping: off");
-        batteryManagement.setWatchDogTimer(Bq24295::WatchDogTimer::Enabled40s);
+        batteryManagement->setWatchDogTimer(Bq24295::WatchDogTimer::Enabled40s);
-        batteryManagement.setBatFetOn(true);
+        batteryManagement->setBatFetOn(true);
     }
     return true;
 }
@@ -297,5 +297,5 @@ void UnPhoneFeatures::wakeOnPowerSwitch() const {
 }
 
 bool UnPhoneFeatures::isUsbPowerConnected() const {
-    return batteryManagement.isUsbPowerConnected();
+    return batteryManagement->isUsbPowerConnected();
 }

Boards/UnPhone/Source/UnPhoneFeatures.h

@@ -7,12 +7,11 @@
 /**
  * Easy access to GPIO pins
  */
-class UnPhoneFeatures {
+class UnPhoneFeatures final {
 
 private:
 
     esp_io_expander_handle_t ioExpander = nullptr;
-    Bq24295 batteryManagement = Bq24295(I2C_NUM_0);
     tt::Thread buttonHandlingThread;
     bool buttonHandlingThreadInterruptRequest = false;
 
@@ -21,9 +20,14 @@ private:
     static bool initPowerSwitch();
     bool initGpioExpander();
 
+    std::shared_ptr<Bq24295> batteryManagement;
+
 public:
 
-    UnPhoneFeatures() = default;
+    explicit UnPhoneFeatures(std::shared_ptr<Bq24295> bq24295) : batteryManagement(std::move(bq24295)) {
+        assert(batteryManagement != nullptr);
+    }
+
     ~UnPhoneFeatures();
 
     bool init();

Boards/UnPhone/Source/hal/UnPhoneDisplay.cpp

@@ -13,7 +13,7 @@
 #define TAG "unphone_display"
 #define BUFFER_SIZE (UNPHONE_LCD_HORIZONTAL_RESOLUTION * UNPHONE_LCD_DRAW_BUFFER_HEIGHT * LV_COLOR_DEPTH / 8)
 
-extern UnPhoneFeatures unPhoneFeatures;
+extern std::shared_ptr<UnPhoneFeatures> unPhoneFeatures;
 
 bool UnPhoneDisplay::start() {
     TT_LOG_I(TAG, "Starting");
@@ -47,7 +47,7 @@ bool UnPhoneDisplay::start() {
 
     if (displayHandle != nullptr) {
         TT_LOG_I(TAG, "Finished");
-        unPhoneFeatures.setBacklightPower(true);
+        unPhoneFeatures->setBacklightPower(true);
         return true;
     } else {
         TT_LOG_I(TAG, "Failed");

Tactility/Source/app/i2cscanner/I2cScanner.cpp

@@ -4,6 +4,7 @@
 
 #include "Tactility/Preferences.h"
 #include "Tactility/app/AppContext.h"
+#include "Tactility/hal/i2c/I2cDevice.h"
 #include "Tactility/lvgl/LvglSync.h"
 #include "Tactility/lvgl/Toolbar.h"
 #include "Tactility/service/loader/Loader.h"
@@ -12,6 +13,9 @@
 #include <Tactility/Tactility.h>
 #include <Tactility/Timer.h>
 
+#include <format>
+#include <ranges>
+
 #define START_SCAN_TEXT "Scan"
 #define STOP_SCAN_TEXT "Stop scan"
 
@@ -328,6 +332,16 @@ void I2cScannerApp::onPressScan(TT_UNUSED lv_event_t* event) {
     updateViews();
 }
 
+static bool findDeviceName(const std::vector<std::shared_ptr<hal::i2c::I2cDevice>>& devices, i2c_port_t port, uint8_t address, std::string& outName) {
+    for (auto& device : devices) {
+        if (device->getPort() == port && device->getAddress() == address) {
+            outName = device->getName();
+            return true;
+        }
+    }
+    return false;
+}
+
 void I2cScannerApp::updateViews() {
     if (mutex.lock(100 / portTICK_PERIOD_MS)) {
         if (scanState == ScanStateScanning) {
@@ -341,10 +355,19 @@ void I2cScannerApp::updateViews() {
         lv_obj_clean(scanListWidget);
         if (scanState == ScanStateStopped) {
             lv_obj_remove_flag(scanListWidget, LV_OBJ_FLAG_HIDDEN);
+
+            auto devices = hal::findDevices<hal::i2c::I2cDevice>(hal::Device::Type::I2c);
+
             if (!scannedAddresses.empty()) {
                 for (auto address: scannedAddresses) {
                     std::string address_text = getAddressText(address);
-                    lv_list_add_text(scanListWidget, address_text.c_str());
+                    std::string device_name;
+                    if (findDeviceName(devices, port, address, device_name)) {
+                        auto text = std::format("{} - {}", address_text, device_name);
+                        lv_list_add_text(scanListWidget, text.c_str());
+                    } else {
+                        lv_list_add_text(scanListWidget, address_text.c_str());
+                    }
                 }
             } else {
                 lv_list_add_text(scanListWidget, "No devices found");
@@ -360,7 +383,7 @@ void I2cScannerApp::updateViews() {
 }
 
 void I2cScannerApp::updateViewsSafely() {
-    if (lvgl::lock(100 / portTICK_PERIOD_MS)) {
+    if (lvgl::lock(200 / portTICK_PERIOD_MS)) {
         updateViews();
         lvgl::unlock();
     } else {
@@ -372,9 +395,10 @@ void I2cScannerApp::onScanTimerFinished() {
     if (mutex.lock(100 / portTICK_PERIOD_MS)) {
         if (scanState == ScanStateScanning) {
             scanState = ScanStateStopped;
-            updateViewsSafely();
         }
         mutex.unlock();
+
+        updateViewsSafely();
     } else {
         TT_LOG_W(TAG, LOG_MESSAGE_MUTEX_LOCK_FAILED_FMT, "onScanTimerFinished");
     }

TactilityHeadless/Include/Tactility/hal/Device.h

@@ -1,6 +1,8 @@
 #pragma once
 
+#include <functional>
 #include <memory>
+#include <ranges>
 #include <string>
 #include <vector>
 
@@ -47,7 +49,6 @@ public:
     virtual std::string getDescription() const = 0;
 };
 
-
 /**
  * Adds a device to the registry.
  * @warning This will leak memory if you want to destroy a device and don't call deregisterDevice()!
@@ -57,6 +58,12 @@ void registerDevice(const std::shared_ptr<Device>& device);
 /** Remove a device from the registry. */
 void deregisterDevice(const std::shared_ptr<Device>& device);
 
+/** Find a single device with a custom filter */
+std::shared_ptr<Device> _Nullable findDevice(const std::function<bool(const std::shared_ptr<Device>&)>& filterFunction);
+
+/** Find devices with a custom filter */
+std::vector<std::shared_ptr<Device>> findDevices(const std::function<bool(const std::shared_ptr<Device>&)>& filterFunction);
+
 /** Find a device in the registry by its name. */
 std::shared_ptr<Device> _Nullable findDevice(std::string name);
 
@@ -69,6 +76,25 @@ std::vector<std::shared_ptr<Device>> findDevices(Device::Type type);
 /** Get a copy of the entire device registry in its current state. */
 std::vector<std::shared_ptr<Device>> getDevices();
 
+/** Find devices of a certain type and cast them to the specified class */
+template<class DeviceType>
+std::vector<std::shared_ptr<DeviceType>> findDevices(Device::Type type) {
+    auto devices = findDevices(type);
+    if (devices.empty()) {
+        return {};
+    } else {
+        std::vector<std::shared_ptr<DeviceType>> result;
+        result.reserve(devices.size());
+        for (auto& device : devices) {
+            auto target_device = std::static_pointer_cast<DeviceType>(device);
+            assert(target_device != nullptr);
+            result.push_back(target_device);
+        }
+        return std::move(result);
+    }
+}
+
+/** Find the first device of the specified type and cast it to the specified class */
 template<class DeviceType>
 std::shared_ptr<DeviceType> findFirstDevice(Device::Type type) {
     auto devices = findDevices(type);

TactilityHeadless/Include/Tactility/hal/i2c/I2cDevice.h

@@ -20,8 +20,6 @@ protected:
 
     static constexpr TickType_t DEFAULT_TIMEOUT = 1000 / portTICK_PERIOD_MS;
 
-    Type getType() const override { return Type::I2c; }
-
     bool readRegister8(uint8_t reg, uint8_t& result) const;
     bool writeRegister8(uint8_t reg, uint8_t value) const;
     bool readRegister12(uint8_t reg, float& out) const;
@@ -35,6 +33,12 @@ protected:
 public:
 
     explicit I2cDevice(i2c_port_t port, uint32_t address) : port(port), address(address) {}
+
+    Type getType() const override { return Type::I2c; }
+
+    i2c_port_t getPort() const { return port; }
+
+    uint8_t getAddress() const { return address; }
 };
 
 }

TactilityHeadless/Source/hal/Device.cpp

@@ -1,6 +1,5 @@
 #include "Tactility/hal/Device.h"
 
-#include <ranges>
 #include <Tactility/Mutex.h>
 
 namespace tt::hal {
@@ -58,7 +57,7 @@ std::vector<std::shared_ptr<Device>> findDevices(const std::function<bool(const
     return {};
 }
 
-static std::shared_ptr<Device> _Nullable findDevice(const std::function<bool(const std::shared_ptr<Device>&)>& filterFunction) {
+std::shared_ptr<Device> _Nullable findDevice(const std::function<bool(const std::shared_ptr<Device>&)>& filterFunction) {
     auto scoped_mutex = mutex.scoped();
     if (scoped_mutex->lock()) {
         auto result_set = devices | std::views::filter([&filterFunction](auto& device) {