From dc14bb56e952f0e3bef7d3e373b425f4db281314 Mon Sep 17 00:00:00 2001 From: quinkq Date: Sun, 28 Dec 2025 15:56:55 +0100 Subject: [PATCH 1/2] feat: add per-device ACK control with handle invalidation Add support for sensors that don't acknowledge specific I2C commands. This enables support for devices like SCD4x, that don't ACK during wake-up from sleep mode: - Add 'disable_ack_check' field to i2c_dev_t structure - Implement i2c_dev_invalidate_handle() for dynamic reconfiguration - Use device flag in i2c_device_config_t creation - Add warning log when ACK checking is disabled --- .gitmodules | 6 ++--- common | 1 - eil-cmake-utils | 1 + i2cdev.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++- i2cdev.h | 33 ++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 5 deletions(-) delete mode 160000 common create mode 160000 eil-cmake-utils diff --git a/.gitmodules b/.gitmodules index a9554a2..ad39b0a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,3 @@ -[submodule "common"] - path = common - url = https://github.com/esp-idf-lib/common.git +[submodule "eil-cmake-utils"] + path = eil-cmake-utils + url = https://github.com/esp-idf-lib/eil-cmake-utils.git diff --git a/common b/common deleted file mode 160000 index 58f97e3..0000000 --- a/common +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 58f97e3e8de40a3c8e3b0bb14bff176b389827ac diff --git a/eil-cmake-utils b/eil-cmake-utils new file mode 160000 index 0000000..c91055e --- /dev/null +++ b/eil-cmake-utils @@ -0,0 +1 @@ +Subproject commit c91055e05b68b6f0cdbd73969e725d0b91aa25fa diff --git a/i2cdev.c b/i2cdev.c index b690b17..961cc1c 100644 --- a/i2cdev.c +++ b/i2cdev.c @@ -320,6 +320,65 @@ esp_err_t i2c_dev_give_mutex(i2c_dev_t *dev) return ESP_OK; } +esp_err_t i2c_dev_invalidate_handle(i2c_dev_t *dev) +{ + if (!dev) + return ESP_ERR_INVALID_ARG; + + ESP_LOGV(TAG, "[0x%02x at %d] Invalidating device handle for reconfiguration", dev->addr, dev->port); + + // Check if port is valid and initialized + if (dev->port >= I2C_NUM_MAX) + { + ESP_LOGE(TAG, "[0x%02x at %d] Invalid port number", dev->addr, dev->port); + return ESP_ERR_INVALID_ARG; + } + + i2c_port_state_t *port_state = &i2c_ports[dev->port]; + + // Check if port mutex exists + if (!port_state->lock) + { + ESP_LOGE(TAG, "[0x%02x at %d] Port not initialized - call i2cdev_init() first", dev->addr, dev->port); + return ESP_ERR_INVALID_STATE; + } + + // Take port mutex for thread-safe handle removal + if (xSemaphoreTake(port_state->lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) != pdTRUE) + { + ESP_LOGE(TAG, "[0x%02x at %d] Could not take port mutex for handle invalidation", dev->addr, dev->port); + return ESP_ERR_TIMEOUT; + } + + esp_err_t result = ESP_OK; + + // Remove device handle if it exists + if (dev->dev_handle) + { + ESP_LOGV(TAG, "[0x%02x at %d] Removing device handle %p", dev->addr, dev->port, dev->dev_handle); + + esp_err_t rm_res = i2c_master_bus_rm_device((i2c_master_dev_handle_t)dev->dev_handle); + if (rm_res != ESP_OK) + { + ESP_LOGW(TAG, "[0x%02x at %d] Failed to remove device handle: %s", + dev->addr, dev->port, esp_err_to_name(rm_res)); + result = rm_res; + } + + // Always NULL the handle to force recreation on next operation + dev->dev_handle = NULL; + ESP_LOGV(TAG, "[0x%02x at %d] Device handle invalidated, will be recreated on next I2C operation", + dev->addr, dev->port); + } + else + { + ESP_LOGV(TAG, "[0x%02x at %d] No device handle to invalidate", dev->addr, dev->port); + } + + xSemaphoreGive(port_state->lock); + return result; +} + // i2c_setup_port: Initializes the I2C master bus for a given port if not already done. // It uses pin configurations from dev->cfg.sda_io_num and dev->cfg.scl_io_num. // The pins for a port are fixed after the first device initializes it. @@ -529,7 +588,7 @@ static esp_err_t i2c_setup_device(i2c_dev_t *dev) // dev is non-const - modifies .dev_addr_length = dev->addr_bit_len, .device_address = dev->addr, .scl_speed_hz = effective_dev_speed, - .flags.disable_ack_check = false, + .flags.disable_ack_check = dev->disable_ack_check, }; res = i2c_master_bus_add_device(port_state->bus_handle, &dev_config, (i2c_master_dev_handle_t *)&dev->dev_handle); @@ -537,6 +596,13 @@ static esp_err_t i2c_setup_device(i2c_dev_t *dev) // dev is non-const - modifies { ESP_LOGI(TAG, "[0x%02x at %d] Device added successfully (Device Handle: %p, Speed: %" PRIu32 " Hz).", dev->addr, dev->port, dev->dev_handle, effective_dev_speed); + // Log if ACK checking is disabled (non-standard configuration) + if (dev->disable_ack_check) + { + ESP_LOGW(TAG, "[0x%02x at %d] WARNING: ACK checking DISABLED - communication errors won't be detected", + dev->addr, dev->port); + } + // Increment the port reference count for each device successfully added port_state->ref_count++; ESP_LOGV(TAG, "[Port %d] Incremented ref_count to %" PRIu32, dev->port, port_state->ref_count); diff --git a/i2cdev.h b/i2cdev.h index 4dab9ba..ab6791a 100644 --- a/i2cdev.h +++ b/i2cdev.h @@ -143,6 +143,9 @@ typedef enum * │ - dev->cfg.sda_pullup_en - Enable internal SDA pullup │ * │ - dev->cfg.scl_pullup_en - Enable internal SCL pullup │ * │ - dev->timeout_ticks - Legacy driver timeout (legacy only) │ + * │ - dev->disable_ack_check - Disable ACK checking (false=default) │ + * │ WARNING: Changing after first transaction requires handle │ + * │ invalidation via i2c_dev_invalidate_handle() │ * └──────────────────────────────────────────────────────────────────────┘ * * ┌─── AUTO-POPULATED (library fills these) ─────────────────────────────┐ @@ -175,6 +178,11 @@ typedef struct // ═══ Legacy Driver Compatibility ═══ uint32_t timeout_ticks; //!< Clock stretching timeout - Legacy driver only + // ═══ Device Behavior Configuration (OPTIONAL) ═══ + bool disable_ack_check; //!< Disable ACK checking (false=enabled [default], true=disabled) + //!< WARNING: PERMANENT after first I2C transaction unless handle invalidated + //!< Use i2c_dev_invalidate_handle() to force reconfiguration + // ═══ User Configuration (REQUIRED) ═══ // Configuration structure with i2c_config_t compatible field layout. struct @@ -251,6 +259,31 @@ esp_err_t i2c_dev_take_mutex(i2c_dev_t *dev); */ esp_err_t i2c_dev_give_mutex(i2c_dev_t *dev); +/** + * @brief Invalidate device handle to force recreation on next I2C operation + * + * Use this when you need to change device configuration (like disable_ack_check) + * after the device has already been used. The handle will be recreated with the + * new config on the next I2C transaction. + * + * This is needed because ESP-IDF caches device handles for performance, so just + * changing the config field won't do anything unless you invalidate the handle first. + * + * @note Thread-safe (uses port mutex), but don't call during active I2C operations + * + * Example (SCD4x wake-up that doesn't ACK): + * dev->disable_ack_check = true; + * i2c_dev_invalidate_handle(dev); + * i2c_dev_write(...); // No ACK required + * dev->disable_ack_check = false; + * i2c_dev_invalidate_handle(dev); + * + * @param dev Device descriptor + * @return ESP_OK on success, ESP_ERR_INVALID_ARG if dev is NULL, + * ESP_ERR_TIMEOUT if mutex couldn't be acquired + */ +esp_err_t i2c_dev_invalidate_handle(i2c_dev_t *dev); + /** * @brief Check the availability of a device on the I2C bus (New Driver) - legacy's i2c_dev_probe function equivalent. * From 76dd5310333e01ff3878917ff7be7c2b55078e55 Mon Sep 17 00:00:00 2001 From: quinkq Date: Sun, 28 Dec 2025 17:48:41 +0100 Subject: [PATCH 2/2] style: fix astyle formatting for multi-line comments --- i2cdev.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/i2cdev.h b/i2cdev.h index ab6791a..f0fbf0c 100644 --- a/i2cdev.h +++ b/i2cdev.h @@ -180,8 +180,8 @@ typedef struct // ═══ Device Behavior Configuration (OPTIONAL) ═══ bool disable_ack_check; //!< Disable ACK checking (false=enabled [default], true=disabled) - //!< WARNING: PERMANENT after first I2C transaction unless handle invalidated - //!< Use i2c_dev_invalidate_handle() to force reconfiguration + //!< WARNING: PERMANENT after first I2C transaction unless handle invalidated + //!< Use i2c_dev_invalidate_handle() to force reconfiguration // ═══ User Configuration (REQUIRED) ═══ // Configuration structure with i2c_config_t compatible field layout.