fix(led): handle expected KTD2026 reset NACK

src/drivers/LED_Controller/KTD2026.cpp

@@ -1,5 +1,7 @@
 #include "KTD2026.h"
 
+#include <string.h>
+
 #include <zephyr/pm/device.h>
 #include <zephyr/pm/device_runtime.h>
 
@@ -8,24 +10,84 @@
 #include <zephyr/logging/log.h>
 LOG_MODULE_REGISTER(LED, CONFIG_MAIN_LOG_LEVEL);
 
-bool KTD2026::readReg(uint8_t reg, uint8_t * buffer, uint16_t len) {
-        _i2c->aquire();
+namespace {
 
-        int ret = i2c_burst_read(_i2c->master, address, reg, buffer, len);
-        if (ret) LOG_WRN("I2C read failed: %d\n", ret);
+constexpr uint8_t KTD2026_CTRL_RESET_REGISTERS = 0x05;
+constexpr uint8_t KTD2026_CTRL_RESET_COMPLETE_CHIP = 0x07;
+constexpr uint8_t KTD2026_CTRL_SHUTDOWN = 0x08;
+constexpr uint8_t KTD2026_REGISTER_COUNT = KTD2026::I_B + 1;
 
-        _i2c->release();
+constexpr uint8_t KTD2026_RESET_DEFAULTS[] = {
+        0x00, 0x00, 0x01, 0x01, 0x00, 0x00, 0x4F, 0x4F, 0x4F
+};
+static_assert(sizeof(KTD2026_RESET_DEFAULTS) == KTD2026_REGISTER_COUNT,
+              "KTD2026 reset defaults must cover every cached register");
+
+bool isResetCommand(uint8_t reg, const uint8_t *buffer, uint16_t len) {
+        if ((reg != KTD2026::CTRL) || (buffer == NULL) || (len != 1)) {
+                return false;
+        }
+
+        return (buffer[0] == KTD2026_CTRL_RESET_REGISTERS) ||
+               (buffer[0] == KTD2026_CTRL_RESET_COMPLETE_CHIP);
+}
+
+bool isValidRegisterRange(uint8_t reg, uint16_t len) {
+        return (len > 0) && (reg < KTD2026_REGISTER_COUNT) &&
+               (len <= (KTD2026_REGISTER_COUNT - reg));
+}
+
+void clearColor(RGBColor color) {
+        memset(color, 0, sizeof(RGBColor));
+}
 
-        return (ret == 0);
 }
 
-void KTD2026::writeReg(uint8_t reg, uint8_t *buffer, uint16_t len) {
+bool KTD2026::readReg(uint8_t reg, uint8_t * buffer, uint16_t len) {
+        if ((buffer == NULL) || !isValidRegisterRange(reg, len)) {
+                LOG_WRN("Invalid KTD2026 register read: reg=0x%02x len=%u",
+                        (unsigned int)reg, (unsigned int)len);
+                return false;
+        }
+
+        memcpy(buffer, &_register_cache[reg], len);
+
+        return true;
+}
+
+bool KTD2026::writeReg(uint8_t reg, const uint8_t *buffer, uint16_t len, bool ignore_reset_nack) {
+        bool reset_command = isResetCommand(reg, buffer, len);
+
+        if ((buffer == NULL) || !isValidRegisterRange(reg, len)) {
+                LOG_WRN("Invalid KTD2026 register write: reg=0x%02x len=%u",
+                        (unsigned int)reg, (unsigned int)len);
+                return false;
+        }
+
         _i2c->aquire();
 
         int ret = i2c_burst_write(_i2c->master, address, reg, buffer, len);
-        if (ret) LOG_WRN("I2C write failed: %d", ret);
+        if (ret) {
+                if (ignore_reset_nack && reset_command) {
+                        LOG_DBG("Ignoring expected KTD2026 reset NACK: %d", ret);
+                } else {
+                        LOG_WRN("I2C write failed: %d", ret);
+                }
+        }
 
         _i2c->release();
+
+        if (ret && !(ignore_reset_nack && reset_command)) {
+                return false;
+        }
+
+        if (reset_command) {
+                resetRegisterCache();
+        } else {
+                memcpy(&_register_cache[reg], buffer, len);
+        }
+
+        return true;
 }
 
 void KTD2026::begin() {
@@ -44,17 +106,25 @@ void KTD2026::begin() {
 }
 
 void KTD2026::reset() {
-        uint8_t val = 0x7;
-        writeReg(registers::CTRL, &val, sizeof(val));
+        uint8_t val = KTD2026_CTRL_RESET_COMPLETE_CHIP;
+        if (writeReg(registers::CTRL, &val, sizeof(val), true)) {
+                clearColor(current_color);
+        }
         k_usleep(200);
 }
 
 void KTD2026::power_off() {
-        uint8_t val = 0x8;
-        writeReg(registers::CTRL, &val, sizeof(val));
+        uint8_t channel_enable = 0;
+        (void)writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
+
+        uint8_t val = KTD2026_CTRL_SHUTDOWN;
+        (void)writeReg(registers::CTRL, &val, sizeof(val));
+
         int ret = pm_device_runtime_put(ls_1_8);
         ret = pm_device_runtime_put(ls_3_3);
 
+        clearCachedColor();
+
 	_active = false;
 }
 
@@ -70,8 +140,12 @@ void KTD2026::setColor(const RGBColor& color) {
                 }
         }
 
-        writeReg(registers::I_R, _color, sizeof(RGBColor));
-        writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
+        bool color_written = writeReg(registers::I_R, _color, sizeof(RGBColor));
+        bool channel_written = writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
+
+        if (color_written && channel_written) {
+                getColor(&current_color);
+        }
 }
 
 void KTD2026::blink(const RGBColor& color, const int time_on_millis, const int period_millis) {
@@ -94,13 +168,18 @@ void KTD2026::pulse(const RGBColor& color, const int time_on_millis, const int t
                 }
         }
 
-        writeReg(registers::I_R, _color, sizeof(RGBColor));
-        writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
+        bool color_written = writeReg(registers::I_R, _color, sizeof(RGBColor));
+        bool channel_written = writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
+
+        bool flash_written = writeReg(registers::FP, &flash_period, sizeof(flash_period));
+        bool ramp_written = writeReg(registers::RAMP, &t_rise_fall, sizeof(t_rise_fall));
+        bool pwm1_written = writeReg(registers::PWM1, &time_on, sizeof(time_on));
+        bool pwm2_written = writeReg(registers::PWM2, &time_on_2, sizeof(time_on_2));
 
-        writeReg(registers::FP, &flash_period, sizeof(flash_period));
-        writeReg(registers::RAMP, &t_rise_fall, sizeof(t_rise_fall));
-        writeReg(registers::PWM1, &time_on, sizeof(time_on));
-        writeReg(registers::PWM2, &time_on_2, sizeof(time_on_2));
+        if (color_written && channel_written && flash_written && ramp_written && pwm1_written &&
+            pwm2_written) {
+                getColor(&current_color);
+        }
 }
 
 void KTD2026::pulse2(const RGBColor& color, const RGBColor& color2, const int time_on_millis, const int time_rise_millis, const int time_fall_millis, const int period_millis) {
@@ -126,31 +205,49 @@ void KTD2026::pulse2(const RGBColor& color, const RGBColor& color2, const int ti
                 }
         }
 
-        writeReg(registers::I_R, _color, sizeof(RGBColor));
-        writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
+        bool color_written = writeReg(registers::I_R, _color, sizeof(RGBColor));
+        bool channel_written = writeReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
 
         uint8_t val = 0x2; // slot 3
-        writeReg(registers::CTRL, &val, sizeof(val));
+        bool ctrl_written = writeReg(registers::CTRL, &val, sizeof(val));
 
-        writeReg(registers::FP, &flash_period, sizeof(flash_period));
-        writeReg(registers::RAMP, &t_rise_fall, sizeof(t_rise_fall));
+        bool flash_written = writeReg(registers::FP, &flash_period, sizeof(flash_period));
+        bool ramp_written = writeReg(registers::RAMP, &t_rise_fall, sizeof(t_rise_fall));
 
-        writeReg(registers::PWM2, &time_on, sizeof(time_on));
+        bool pwm_written = writeReg(registers::PWM2, &time_on, sizeof(time_on));
+
+        if (color_written && channel_written && ctrl_written && flash_written && ramp_written &&
+            pwm_written) {
+                getColor(&current_color);
+        }
 }
 
-/* Not working */
 void KTD2026::getColor(RGBColor * color) {
         uint8_t channel_enable = 0;
 
+        if (color == NULL) {
+                LOG_WRN("KTD2026 getColor called without output buffer");
+                return;
+        }
+
         readReg(registers::EN_CH, &channel_enable, sizeof(channel_enable));
         readReg(registers::I_R, *color, sizeof(RGBColor));
 
         for (int i = 0; i < 3; i++) {
                 (*color)[i] ++;
-                if ((channel_enable & BIT(2 * i)) == 0) {
+                if ((channel_enable & (0x3 << (2 * i))) == 0) {
                         (*color)[i] = 0;
                 }
         }
 }
 
-KTD2026 led_controller;
+void KTD2026::resetRegisterCache() {
+        memcpy(_register_cache, KTD2026_RESET_DEFAULTS, sizeof(_register_cache));
+}
+
+void KTD2026::clearCachedColor() {
+        _register_cache[registers::EN_CH] = 0;
+        clearColor(current_color);
+}
+
+KTD2026 led_controller;

src/drivers/LED_Controller/KTD2026.h

@@ -22,30 +22,52 @@ class KTD2026 {
         I_B = 0x08,
     };
 
-    RGBColor current_color;
+    RGBColor current_color = {0, 0, 0};
 
+    /** @brief Enable the LED driver power rails and reset the KTD2026. */
     void begin();
+
+    /** @brief Reset the KTD2026 register bank and clear the cached LED color. */
     void reset();
+
+    /** @brief Put the LED driver into shutdown and release its power rails. */
     void power_off();
+
+    /** @brief Set a steady RGB color. */
     void setColor(const RGBColor& color);
+
+    /** @brief Return the cached RGB current values for enabled channels. */
     void getColor(RGBColor * color);
 
+    /** @brief Blink an RGB color with a fixed on-time and period. */
     void blink(const RGBColor& color, const int time_on_millis, const int period_millis);
+
+    /** @brief Pulse an RGB color using the KTD2026 timer and ramp registers. */
     void pulse(const RGBColor& color, const int time_on_millis, const int time_rise_millis, const int time_fall_millis, const int period_millis);
+
+    /** @brief Pulse two RGB colors using the KTD2026 timer slots. */
     void pulse2(const RGBColor& color, const RGBColor& color2, const int time_on_millis, const int time_rise_millis, const int time_fall_millis, const int period_millis);
     //void setPower();
 private:
     bool readReg(uint8_t reg, uint8_t *buffer, uint16_t len);
-    void writeReg(uint8_t reg, uint8_t *buffer, uint16_t len);
+    bool writeReg(uint8_t reg, const uint8_t *buffer, uint16_t len, bool ignore_reset_nack = false);
+    void resetRegisterCache();
+    void clearCachedColor();
 
     int address = 0x30;
 
     //TwoWire * _pWire = &Wire;
     TWIM * _i2c = &I2C1;
 
+    static constexpr uint8_t REGISTER_COUNT = I_B + 1;
+
+    uint8_t _register_cache[REGISTER_COUNT] = {
+        0x00, 0x00, 0x01, 0x01, 0x00, 0x00, 0x4F, 0x4F, 0x4F
+    };
+
     bool _active = false;
 };
 
 extern KTD2026 led_controller;
 
-#endif
+#endif