diff --git a/.eil.yml b/.eil.yml index f634593..953b818 100644 --- a/.eil.yml +++ b/.eil.yml @@ -1,6 +1,6 @@ name: i2cdev description: ESP-IDF I2C master thread-safe utilities -version: 2.1.3 +version: 2.2.0 groups: - common code_owners: diff --git a/CMakeLists.txt b/CMakeLists.txt index 6cf8ddb..ec73d83 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,9 +5,16 @@ eil_check_idf_has_esp_drivers() # List of common ESP-IDF components required by this component set(REQUIRED_COMPONENTS log freertos esp_idf_lib_helpers) -# Add version-specific required componenets +# Add version-specific required components. +# +# NOTE: "driver" must be listed unconditionally on ESP-IDF versions that also +# provide the split esp_driver_* components. i2cdev_legacy.c includes +# (provided by "driver") whenever CONFIG_I2CDEV_USE_LEGACY_DRIVER +# is enabled. That Kconfig option is not yet visible during the first CMake +# configure pass of a clean project, so deriving the requirement list from it +# would leave the legacy build without the "driver" include path. if(EIL_IDF_HAS_ESP_DRIVERS) - list(APPEND REQUIRED_COMPONENTS esp_driver_gpio esp_driver_i2c) + list(APPEND REQUIRED_COMPONENTS driver esp_driver_gpio esp_driver_i2c) else() list(APPEND REQUIRED_COMPONENTS driver) endif() diff --git a/Kconfig b/Kconfig index 953a7d1..83a374d 100644 --- a/Kconfig +++ b/Kconfig @@ -42,10 +42,12 @@ config I2CDEV_DEFAULT_SCL_PIN Default SCL pin for I2C devices. config I2CDEV_MAX_DEVICES_PER_PORT - int "Maximum number of devices per I2C port" + int "Maximum number of devices per I2C port (legacy driver only)" default 8 help - Maximum number of devices that can be registered on a single I2C port. + Maximum number of devices that can be tracked on a single I2C port. + Used by the legacy driver only: the new i2c_master based driver keeps a + dynamic list and accepts any number of devices per port. config I2CDEV_TIMEOUT int "I2C transaction timeout, milliseconds" @@ -53,12 +55,19 @@ config I2CDEV_TIMEOUT range 10 5000 config I2CDEV_NOLOCK - bool "Disable the use of mutexes" + bool "Disable per-device mutexes" default n help - Attention! After enabling this option, all I2C device - drivers will become non-thread safe. - Use this option if you need to access your I2C devices - from interrupt handlers. + Disables the per-device mutex (dev->mutex) and makes + i2c_dev_take_mutex()/i2c_dev_give_mutex(), as well as the + I2C_DEV_TAKE_MUTEX/I2C_DEV_GIVE_MUTEX macros, no-ops. + + Bus-level management is not affected: the port mutex, device + handles and bus teardown in i2cdev_done() still run, so devices + are removed from the bus and the bus is released as usual. + + Use this option only when all access to each device is already + serialized by the caller, or when devices must be accessed from + interrupt context where taking a mutex is not allowed. endmenu \ No newline at end of file diff --git a/i2cdev.c b/i2cdev.c index 5c3ddee..63edfd3 100644 --- a/i2cdev.c +++ b/i2cdev.c @@ -38,6 +38,7 @@ #include #include #include +#include #include static const char *TAG = "i2cdev"; @@ -52,18 +53,25 @@ static const char *TAG = "i2cdev"; #define I2C_RETRY_BASE_DELAY_MS 20 #define I2CDEV_MAX_STACK_ALLOC_SIZE 32 // Stack allocation threshold to avoid heap fragmentation for small buffers +// Node of the per-port list of devices whose handle must be removed on cleanup. +// The list has no fixed size: any number of devices can share a port. +typedef struct i2c_dev_node +{ + struct i2c_dev_node *next; + i2c_dev_t *dev; +} i2c_dev_node_t; + typedef struct { SemaphoreHandle_t lock; // Mutex for exclusive access to this port's state i2c_master_bus_handle_t bus_handle; // Handle to the initialized I2C master bus bool installed; // Flag indicating if the bus for this port has been installed - uint32_t ref_count; // Number of devices currently active on this bus port int sda_pin_current; // Actual SDA pin the bus was initialized with int scl_pin_current; // Actual SCL pin the bus was initialized with + i2c_dev_node_t *devices; // Devices added to this bus, newest first } i2c_port_state_t; static i2c_port_state_t i2c_ports[I2C_NUM_MAX] = { 0 }; -static i2c_dev_t *active_devices[I2C_NUM_MAX][CONFIG_I2CDEV_MAX_DEVICES_PER_PORT] = { { NULL } }; // Helper to register a device static esp_err_t register_device(i2c_dev_t *dev) @@ -71,36 +79,64 @@ static esp_err_t register_device(i2c_dev_t *dev) if (!dev) return ESP_ERR_INVALID_ARG; int port = dev->port; - if (port >= I2C_NUM_MAX) + if (port < 0 || port >= I2C_NUM_MAX) return ESP_ERR_INVALID_ARG; // Note: Port mutex should be held by caller - for (int i = 0; i < CONFIG_I2CDEV_MAX_DEVICES_PER_PORT; i++) + i2c_dev_node_t *node = calloc(1, sizeof(i2c_dev_node_t)); + if (!node) { - if (active_devices[port][i] == NULL) + ESP_LOGE(TAG, "[0x%02x at %d] Could not allocate device tracking node", dev->addr, port); + return ESP_ERR_NO_MEM; + } + + node->dev = dev; + node->next = i2c_ports[port].devices; + i2c_ports[port].devices = node; + ESP_LOGV(TAG, "[0x%02x at %d] Registered device for cleanup", dev->addr, port); + return ESP_OK; +} + +// Helper to drop a device from the per-port list. The node is allocated by +// register_device(), so it must be freed here. +static void deregister_device(i2c_dev_t *dev) +{ + i2c_dev_node_t **link = &i2c_ports[dev->port].devices; + while (*link) + { + if ((*link)->dev == dev) { - active_devices[port][i] = dev; - ESP_LOGV(TAG, "[0x%02x at %d] Registered device in slot %d", dev->addr, port, i); - return ESP_OK; + i2c_dev_node_t *node = *link; + *link = node->next; + free(node); + ESP_LOGV(TAG, "[0x%02x at %d] Deregistered device", dev->addr, dev->port); + return; } + link = &(*link)->next; } - ESP_LOGE(TAG, "[0x%02x at %d] No free slots to register device - limit reached", dev->addr, port); - return ESP_ERR_NO_MEM; } -esp_err_t i2cdev_init(void) +// Helper to release the whole per-port device list +static void free_device_list(i2c_port_state_t *port_state) { - ESP_LOGV(TAG, "Initializing I2C subsystem..."); - - // Guard against re-initialization to prevent resource leaks - static bool initialized = false; - if (initialized) + i2c_dev_node_t *node = port_state->devices; + while (node) { - ESP_LOGW(TAG, "I2C subsystem already initialized, skipping re-initialization"); - return ESP_OK; + i2c_dev_node_t *next = node->next; + free(node); + node = next; } + port_state->devices = NULL; +} - memset(active_devices, 0, sizeof(active_devices)); +esp_err_t i2cdev_init(void) +{ + ESP_LOGV(TAG, "Initializing I2C subsystem..."); + + // There is no separate "initialized" flag: the state of the subsystem is + // derived from i2c_ports[]. A port that owns a mutex is usable, so calling + // i2cdev_init() again only creates the missing mutexes. This makes + // init -> done -> init work, and a partially failed init can be retried. for (int i = 0; i < I2C_NUM_MAX; i++) { if (!i2c_ports[i].lock) @@ -113,24 +149,30 @@ esp_err_t i2cdev_init(void) } ESP_LOGV(TAG, "Created port mutex %d", i); } - i2c_ports[i].installed = false; - i2c_ports[i].ref_count = 0; - i2c_ports[i].bus_handle = NULL; - i2c_ports[i].sda_pin_current = -1; - i2c_ports[i].scl_pin_current = -1; + + // Reset a port only while no bus is installed. A bus handle that is + // still alive (i2cdev_done() failed to delete it) must be kept, + // otherwise the platform I2C port stays occupied while i2cdev loses + // its only reference to it. + if (!i2c_ports[i].installed) + { + i2c_ports[i].bus_handle = NULL; + i2c_ports[i].sda_pin_current = -1; + i2c_ports[i].scl_pin_current = -1; + free_device_list(&i2c_ports[i]); + } } - initialized = true; // Mark as initialized ESP_LOGV(TAG, "I2C subsystem initialized."); return ESP_OK; } esp_err_t i2c_dev_create_mutex(i2c_dev_t *dev) { -#if !CONFIG_I2CDEV_NOLOCK if (!dev) return ESP_ERR_INVALID_ARG; +#if !CONFIG_I2CDEV_NOLOCK ESP_LOGV(TAG, "[0x%02x at %d] Creating device mutex...", dev->addr, dev->port); if (dev->mutex) { @@ -146,57 +188,56 @@ esp_err_t i2c_dev_create_mutex(i2c_dev_t *dev) } ESP_LOGV(TAG, "[0x%02x at %d] Device mutex created (Handle: %p)", dev->addr, dev->port, dev->mutex); +#else + ESP_LOGV(TAG, "[0x%02x at %d] Mutex creation skipped (CONFIG_I2CDEV_NOLOCK=1)", dev->addr, dev->port); +#endif - // Register the device for cleanup tracking (under port mutex for consistency) - if (dev->port < I2C_NUM_MAX && i2c_ports[dev->port].lock) + // Address bit length is not a locking concern: normalise it in both modes. + // The device itself is registered on the bus (and therefore tracked for + // cleanup) in i2c_setup_device(), where the handle is actually created. + if (dev->addr_bit_len != I2C_ADDR_BIT_LEN_7 +#if SOC_I2C_SUPPORT_10BIT_ADDR + && dev->addr_bit_len != I2C_ADDR_BIT_LEN_10 +#endif + ) { - if (xSemaphoreTake(i2c_ports[dev->port].lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) == pdTRUE) - { - esp_err_t reg_res = register_device(dev); - if (reg_res != ESP_OK) - { - ESP_LOGW(TAG, "[0x%02x at %d] Failed to register device: %s - device will work but cleanup tracking disabled", dev->addr, dev->port, esp_err_to_name(reg_res)); - // Continue - device can still function without registration tracking - } - else - { - ESP_LOGV(TAG, "[0x%02x at %d] Device registered successfully for cleanup tracking", dev->addr, dev->port); - } - xSemaphoreGive(i2c_ports[dev->port].lock); - } - else - { - ESP_LOGW(TAG, "[0x%02x at %d] Could not take port mutex for device registration", dev->addr, dev->port); - // Continue - device can still function without registration tracking - } + ESP_LOGV(TAG, "[0x%02x at %d] Setting default 7-bit address format", dev->addr, dev->port); + dev->addr_bit_len = I2C_ADDR_BIT_LEN_7; } - // Set default address bit length if not explicitly set - if (dev->addr_bit_len != I2C_ADDR_BIT_LEN_7 && dev->addr_bit_len != I2C_ADDR_BIT_LEN_10) + return ESP_OK; +} + +// Drops the per-device mutex. Bus-level cleanup does not depend on it, and +// with CONFIG_I2CDEV_NOLOCK=1 no per-device mutex is ever created. +static void delete_device_mutex(i2c_dev_t *dev) +{ +#if !CONFIG_I2CDEV_NOLOCK + if (dev->mutex) { - ESP_LOGV(TAG, "[0x%02x at %d] Setting default 7-bit address format", dev->addr, dev->port); - dev->addr_bit_len = I2C_ADDR_BIT_LEN_7; + vSemaphoreDelete(dev->mutex); + dev->mutex = NULL; } #else - ESP_LOGV(TAG, "[0x%02x at %d] Mutex creation skipped (CONFIG_I2CDEV_NOLOCK=1)", dev->addr, dev->port); + (void)dev; #endif - return ESP_OK; } esp_err_t i2c_dev_delete_mutex(i2c_dev_t *dev) { -#if !CONFIG_I2CDEV_NOLOCK + esp_err_t result = ESP_OK; + if (!dev) return ESP_ERR_INVALID_ARG; ESP_LOGV(TAG, "[0x%02x at %d] Deleting device mutex and cleaning up resources", dev->addr, dev->port); - // If we cannot safely touch the port state (bad port index, or the - // subsystem was never initialized), there is no bus handle to release - // and no ref_count to decrement. Just drop the device mutex. - if (dev->port >= I2C_NUM_MAX || !i2c_ports[dev->port].lock) + // The bus lifecycle (device handle, the per-port device list and the port's + // master bus) is handled in both locking modes: disabling device mutexes + // must not disable cleanup. + if (dev->port < 0 || dev->port >= I2C_NUM_MAX || !i2c_ports[dev->port].lock) { - if (dev->port >= I2C_NUM_MAX) + if (dev->port < 0 || dev->port >= I2C_NUM_MAX) { ESP_LOGE(TAG, "[0x%02x at %d] Invalid port %d, skipping port-scoped cleanup", dev->addr, dev->port, dev->port); } @@ -204,11 +245,7 @@ esp_err_t i2c_dev_delete_mutex(i2c_dev_t *dev) { ESP_LOGW(TAG, "[0x%02x at %d] I2C subsystem not initialized, skipping port-scoped cleanup", dev->addr, dev->port); } - if (dev->mutex) - { - vSemaphoreDelete(dev->mutex); - dev->mutex = NULL; - } + delete_device_mutex(dev); return ESP_OK; } @@ -223,9 +260,12 @@ esp_err_t i2c_dev_delete_mutex(i2c_dev_t *dev) // Capture whether this device was actually added to the bus before // we clear the field. - bool was_on_bus = (dev->dev_handle != NULL); + bool was_on_bus = dev->dev_handle; - // 1) Remove the device from the bus, if it was added + // 1) Remove the device from the bus, if it was added. + // The handle is released only when the driver reports success: on failure + // the device stays attached to the bus, and keeping the pointer allows a + // repeated call to retry the removal instead of silently losing the handle. if (was_on_bus) { ESP_LOGV(TAG, "[0x%02x at %d] Removing device handle %p from bus", dev->addr, dev->port, dev->dev_handle); @@ -233,72 +273,37 @@ esp_err_t i2c_dev_delete_mutex(i2c_dev_t *dev) 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; } - dev->dev_handle = NULL; - } - - // 2) Deregister from active_devices[] under the same port mutex - for (int i = 0; i < CONFIG_I2CDEV_MAX_DEVICES_PER_PORT; i++) - { - if (active_devices[dev->port][i] == dev) + else { - active_devices[dev->port][i] = NULL; - ESP_LOGV(TAG, "[0x%02x at %d] Deregistered device from slot %d", dev->addr, dev->port, i); - break; + dev->dev_handle = NULL; } } - // 3) Decrement ref_count; delete the bus if this was the last device - if (was_on_bus && port_state->installed && port_state->ref_count > 0) - { - port_state->ref_count--; - ESP_LOGV(TAG, "[Port %d] Decremented ref_count to %" PRIu32, dev->port, port_state->ref_count); + // 2) Deregister the device from the per-port list under the same port mutex + deregister_device(dev); - if (port_state->ref_count == 0) - { - ESP_LOGI(TAG, "[Port %d] Last device removed, cleaning up port's bus", dev->port); - if (port_state->bus_handle) - { - ESP_LOGI(TAG, "[Port %d] Deleting bus handle %p", dev->port, port_state->bus_handle); - esp_err_t del_bus_res = i2c_del_master_bus(port_state->bus_handle); - if (del_bus_res != ESP_OK) - { - ESP_LOGE(TAG, "[Port %d] Failed to delete master bus: %s", dev->port, esp_err_to_name(del_bus_res)); - } - port_state->bus_handle = NULL; - } - port_state->installed = false; - port_state->sda_pin_current = -1; - port_state->scl_pin_current = -1; - } - } - else if (!was_on_bus) - { - ESP_LOGV(TAG, "[0x%02x at %d] Device was never added to bus, skipping ref_count decrement", dev->addr, dev->port); - } + // The bus itself is not deleted here: it lives from i2cdev_init() (first + // operation) until i2cdev_done(). Removing the last known device must not + // tear the bus down, because other consumers may hold the shared bus + // handle (i2cdev_get_shared_handle()) or keep devices that this library + // does not track. xSemaphoreGive(port_state->lock); // 4) Finally, drop the per-device mutex - if (dev->mutex) - { - vSemaphoreDelete(dev->mutex); - dev->mutex = NULL; - } - else - { - ESP_LOGV(TAG, "[0x%02x at %d] Device mutex was NULL, nothing to delete", dev->addr, dev->port); - } -#endif - return ESP_OK; + delete_device_mutex(dev); + + return result; } esp_err_t i2c_dev_take_mutex(i2c_dev_t *dev) { -#if !CONFIG_I2CDEV_NOLOCK if (!dev) return ESP_ERR_INVALID_ARG; +#if !CONFIG_I2CDEV_NOLOCK ESP_LOGV(TAG, "[0x%02x at %d] Attempting to take device mutex (Handle: %p)...", dev->addr, dev->port, dev->mutex); if (!dev->mutex) { @@ -322,10 +327,10 @@ esp_err_t i2c_dev_take_mutex(i2c_dev_t *dev) esp_err_t i2c_dev_give_mutex(i2c_dev_t *dev) { -#if !CONFIG_I2CDEV_NOLOCK if (!dev) return ESP_ERR_INVALID_ARG; +#if !CONFIG_I2CDEV_NOLOCK ESP_LOGV(TAG, "[0x%02x at %d] Giving device mutex (Handle: %p)...", dev->addr, dev->port, dev->mutex); if (!dev->mutex) { @@ -353,7 +358,7 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de { if (!dev) return ESP_ERR_INVALID_ARG; - if (dev->port >= I2C_NUM_MAX) + if (dev->port < 0 || dev->port >= I2C_NUM_MAX) { ESP_LOGE(TAG, "Invalid I2C port number: %d", dev->port); return ESP_ERR_INVALID_ARG; @@ -390,6 +395,15 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de return ESP_ERR_INVALID_ARG; } + // SDA and SCL must be different pins. Zero-initialized i2c_dev_t leaves + // both at GPIO0, which would configure one pin as SDA and SCL at once. + if (sda_pin == scl_pin) + { + ESP_LOGE(TAG, "[Port %d] SDA and SCL are the same pin (GPIO%d). Set dev->cfg.sda_io_num/dev->cfg.scl_io_num or Kconfig defaults.", dev->port, (int)sda_pin); + xSemaphoreGive(port_state->lock); + return ESP_ERR_INVALID_ARG; + } + /* * OPTIONAL I2C PULLUP AUTO-CONFIGURATION * @@ -401,6 +415,10 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de * Manual pullup configuration: * - Set sda_pullup_en=true, scl_pullup_en=true for internal pullups * - Set sda_pullup_en=false, scl_pullup_en=false for external pullups + * + * The IDF bus config has a single `enable_internal_pullup` flag for both + * lines, so a mixed request (only SDA or only SCL) is applied per line with + * gpio_set_pull_mode() right after the bus is created. */ // Read user's pullup configuration (default false if not set) @@ -428,7 +446,7 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de .sda_io_num = sda_pin, .scl_io_num = scl_pin, .glitch_ignore_cnt = 7, - .flags.enable_internal_pullup = (sda_pullup || scl_pullup), + .flags.enable_internal_pullup = (sda_pullup && scl_pullup), // Bus speed is not set here. It's per-device or a global target for the bus can be set // if desired, but i2c_master supports per-device speeds. }; @@ -447,8 +465,19 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de res = i2c_new_master_bus(&bus_config, &port_state->bus_handle); if (res == ESP_OK) { + // The IDF bus config enables internal pullups on both lines at once, + // so a mixed request (only SDA or only SCL) is applied per line here. + if (sda_pullup != scl_pullup) + { + gpio_num_t pullup_pin = sda_pullup ? sda_pin : scl_pin; + esp_err_t pull_res = gpio_set_pull_mode(pullup_pin, GPIO_PULLUP_ONLY); + if (pull_res == ESP_OK) + ESP_LOGI(TAG, "[Port %d] Internal pullup enabled on GPIO%d only (SDA:%d, SCL:%d)", dev->port, (int)pullup_pin, sda_pullup, scl_pullup); + else + ESP_LOGW(TAG, "[Port %d] Could not enable internal pullup on GPIO%d: %s", dev->port, (int)pullup_pin, esp_err_to_name(pull_res)); + } + port_state->installed = true; - port_state->ref_count = 0; // Will be incremented when a device is successfully added port_state->sda_pin_current = sda_pin; port_state->scl_pin_current = scl_pin; dev->sda_pin = sda_pin; // Update dev struct with actual pins used @@ -484,7 +513,7 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de dev->sda_pin = port_state->sda_pin_current; // Update dev struct with actual pins used dev->scl_pin = port_state->scl_pin_current; } - // ref_count is managed by i2c_setup_device when adding/removing device handles + // The bus outlives individual devices: it is deleted in i2cdev_done() } xSemaphoreGive(port_state->lock); @@ -493,7 +522,7 @@ static esp_err_t i2c_setup_port(i2c_dev_t *dev) // dev is non-const to update de } // i2c_setup_device: Ensures port is set up and adds the device to the bus if not already added. -// It also registers the device in active_devices for cleanup purposes. +// It also registers the device in the port's device list for cleanup purposes. static esp_err_t i2c_setup_device(i2c_dev_t *dev) // dev is non-const - modifies dev->dev_handle, dev->addr_bit_len { if (!dev) @@ -538,23 +567,27 @@ static esp_err_t i2c_setup_device(i2c_dev_t *dev) // dev is non-const - modifies } #endif - if (dev->dev_handle == NULL) - { - i2c_port_state_t *port_state = &i2c_ports[dev->port]; + i2c_port_state_t *port_state = &i2c_ports[dev->port]; - // Check if i2cdev subsystem is initialized - if (!port_state->lock) - { - ESP_LOGE(TAG, "[0x%02x at %d] I2C subsystem not initialized - call i2cdev_init() first", dev->addr, dev->port); - return ESP_ERR_INVALID_STATE; - } + // Check if i2cdev subsystem is initialized + if (!port_state->lock) + { + ESP_LOGE(TAG, "[0x%02x at %d] I2C subsystem not initialized - call i2cdev_init() first", dev->addr, dev->port); + return ESP_ERR_INVALID_STATE; + } - 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 device add", dev->addr, dev->port); - return ESP_ERR_TIMEOUT; - } + 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 device add", dev->addr, dev->port); + return ESP_ERR_TIMEOUT; + } + // Decide under the port mutex whether the device still has to be added: + // i2c_master_bus_add_device() does not detect duplicates, so checking the + // handle before taking the mutex would let two tasks add the same device + // twice and leaking the first handle. + if (!dev->dev_handle) + { if (!port_state->installed || !port_state->bus_handle) { ESP_LOGE(TAG, "[0x%02x at %d] Cannot add device, bus for port %d not ready!", dev->addr, dev->port, dev->port); @@ -586,18 +619,31 @@ static esp_err_t i2c_setup_device(i2c_dev_t *dev) // dev is non-const - modifies res = i2c_master_bus_add_device(port_state->bus_handle, &dev_config, (i2c_master_dev_handle_t *)&dev->dev_handle); if (res == ESP_OK) { - 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); - - // 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); + // Track the device for cleanup while the port mutex is held: the + // registration must cover exactly the devices that have a handle. + // A device that cannot be tracked would survive i2cdev_done() and + // block bus deletion forever, so this failure is fatal. + res = register_device(dev); + if (res != ESP_OK) + { + ESP_LOGE(TAG, "[0x%02x at %d] Failed to register device: %s - removing it from the bus", dev->addr, dev->port, esp_err_to_name(res)); + esp_err_t rm_res = i2c_master_bus_rm_device((i2c_master_dev_handle_t)dev->dev_handle); + if (rm_res != ESP_OK) + { + ESP_LOGE(TAG, "[0x%02x at %d] Failed to remove untracked device handle: %s", dev->addr, dev->port, esp_err_to_name(rm_res)); + } + dev->dev_handle = NULL; + } + else + { + 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); + } } else { ESP_LOGE(TAG, "[0x%02x at %d] Failed to add device to bus: %d (%s)", dev->addr, dev->port, res, esp_err_to_name(res)); dev->dev_handle = NULL; } - xSemaphoreGive(port_state->lock); } else { @@ -605,6 +651,8 @@ static esp_err_t i2c_setup_device(i2c_dev_t *dev) // dev is non-const - modifies res = ESP_OK; } + xSemaphoreGive(port_state->lock); + ESP_LOGV(TAG, "[0x%02x at %d] Device context setup finished with res %d.", dev->addr, dev->port, res); return res; } @@ -623,8 +671,9 @@ static esp_err_t i2c_do_operation_with_retry(i2c_dev_t *dev, esp_err_t (*i2c_fun while (retry <= I2C_MAX_RETRIES) { - // Ensure device is set up before each attempt, in case handle became stale or bus was reset - // This is more robust if issues like bus errors or device resets occur. + // Ensure the device is set up before each attempt: the bus may have + // been installed after a previous failure, or the port may have been + // reconfigured. An existing handle is reused as is. res = i2c_setup_device(dev); if (res != ESP_OK) { @@ -659,41 +708,12 @@ static esp_err_t i2c_do_operation_with_retry(i2c_dev_t *dev, esp_err_t (*i2c_fun ESP_LOGW(TAG, "[0x%02x at %d] I2C op failed (Try %d, Handle %p): %d (%s).", dev->addr, dev->port, retry, dev->dev_handle, res, esp_err_to_name(res)); - // Only remove handle on errors that indicate handle corruption or permanent invalidity - // Don't remove on temporary errors like ESP_ERR_TIMEOUT, ESP_FAIL (NACK), etc. - bool should_remove_handle = false; - switch (res) - { - case ESP_ERR_INVALID_ARG: - // Handle was likely removed by another task or is corrupted - should_remove_handle = true; - ESP_LOGW(TAG, "[0x%02x at %d] Invalid argument error - handle may be corrupted", dev->addr, dev->port); - break; - case ESP_ERR_INVALID_STATE: - // I2C driver is in invalid state, handle likely needs recreation - should_remove_handle = true; - ESP_LOGW(TAG, "[0x%02x at %d] Invalid state error - handle may need recreation", dev->addr, dev->port); - break; - default: - // For other errors (timeout, NACK, bus busy, etc.), keep the handle - // These are usually temporary and don't require handle recreation - should_remove_handle = false; - ESP_LOGV(TAG, "[0x%02x at %d] Temporary error - keeping handle for retry", dev->addr, dev->port); - break; - } - - if (should_remove_handle && dev->dev_handle) - { - ESP_LOGW(TAG, "[0x%02x at %d] Removing potentially corrupted device handle %p after permanent error", dev->addr, dev->port, dev->dev_handle); - // Try to remove the handle from the bus before nullifying - esp_err_t rm_res = i2c_master_bus_rm_device(dev->dev_handle); - if (rm_res != ESP_OK) - { - ESP_LOGW(TAG, "[0x%02x at %d] Failed to remove corrupted handle (expected): %s", dev->addr, dev->port, esp_err_to_name(rm_res)); - // This is expected if the handle was already invalid - continue cleanup - } - dev->dev_handle = NULL; - } + // The device handle is deliberately left untouched here. It is created + // in i2c_setup_device() and destroyed in i2c_dev_delete_mutex(), both + // of which run under the port mutex and keep the per-port device list + // consistent. Releasing it from a transfer error path would (a) race + // with tasks still using the handle and (b) leave a stale entry in the + // per-port device list. Errors are reported to the caller instead. retry++; if (retry <= I2C_MAX_RETRIES) @@ -742,7 +762,7 @@ esp_err_t i2c_dev_write(const i2c_dev_t *dev, const void *out_reg, size_t out_re { if (!dev) return ESP_ERR_INVALID_ARG; - if ((!out_reg || !out_reg_size) && (!out_data || !out_size)) + if (!(out_reg && out_reg_size) && !(out_data && out_size)) return ESP_ERR_INVALID_ARG; ESP_LOGV(TAG, "[0x%02x at %d] i2c_dev_write called (reg_size: %u, data_size: %u)", dev->addr, dev->port, out_reg_size, out_size); @@ -801,7 +821,9 @@ esp_err_t i2c_dev_write(const i2c_dev_t *dev, const void *out_reg, size_t out_re esp_err_t i2c_dev_read_reg(const i2c_dev_t *dev, uint8_t reg, void *data, size_t size) { - if (!dev) + // Same contract as the legacy driver: a read without a destination buffer + // or without a size is a caller error, not a silent no-op. + if (!dev || !data || !size) return ESP_ERR_INVALID_ARG; ESP_LOGV(TAG, "[0x%02x at %d] i2c_dev_read_reg called (reg: 0x%02x, size: %u)", dev->addr, dev->port, reg, size); @@ -810,7 +832,10 @@ esp_err_t i2c_dev_read_reg(const i2c_dev_t *dev, uint8_t reg, void *data, size_t esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, size_t size) { - if (!dev) + // Same contract as the legacy driver: writing a register without data is a + // caller error. Use i2c_dev_write(dev, ®, 1, NULL, 0) to write a bare + // register byte. + if (!dev || !data || !size) return ESP_ERR_INVALID_ARG; ESP_LOGV(TAG, "[0x%02x at %d] i2c_dev_write_reg called (reg: 0x%02x, size: %u)", dev->addr, dev->port, reg, size); @@ -836,7 +861,7 @@ esp_err_t i2c_dev_check_present(const i2c_dev_t *dev_const) } // Now probe using the initialized bus - if (dev_const->port < I2C_NUM_MAX && i2c_ports[dev_const->port].lock) + if (dev_const->port >= 0 && dev_const->port < I2C_NUM_MAX && i2c_ports[dev_const->port].lock) { if (xSemaphoreTake(i2c_ports[dev_const->port].lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) == pdTRUE) { @@ -872,8 +897,10 @@ esp_err_t i2c_dev_check_present(const i2c_dev_t *dev_const) } else { - ESP_LOGE(TAG, "[0x%02x at %d] Invalid port or port not initialized", dev_const->addr, dev_const->port); - return ESP_ERR_INVALID_ARG; + // The port range is already validated in i2c_setup_port(), so the only + // way here is a port that was never initialized by i2cdev_init(). + ESP_LOGE(TAG, "[0x%02x at %d] Port is not initialized, call i2cdev_init() first", dev_const->addr, dev_const->port); + return ESP_ERR_INVALID_STATE; } } @@ -896,61 +923,86 @@ esp_err_t i2cdev_done(void) ESP_LOGV(TAG, "Cleaning up I2C subsystem (i2c_master)..."); for (int i = 0; i < I2C_NUM_MAX; i++) { - if (i2c_ports[i].lock) + if (!i2c_ports[i].lock) + continue; + + ESP_LOGV(TAG, "[Port %d] Cleaning up port...", i); + if (xSemaphoreTake(i2c_ports[i].lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) != pdTRUE) { - ESP_LOGV(TAG, "[Port %d] Cleaning up port...", i); - if (xSemaphoreTake(i2c_ports[i].lock, pdMS_TO_TICKS(CONFIG_I2CDEV_TIMEOUT)) != pdTRUE) - { - ESP_LOGE(TAG, "[Port %d] Could not take port mutex for cleanup", i); - result = ESP_FAIL; - } - else + // Another task owns the port. Neither its bus nor its mutex can be + // touched safely; leave the port intact so that a later call can + // finish the cleanup. + ESP_LOGE(TAG, "[Port %d] Could not take port mutex for cleanup", i); + if (result == ESP_OK) + result = ESP_ERR_TIMEOUT; + continue; + } + + bool released = true; + if (i2c_ports[i].installed) + { + ESP_LOGV(TAG, "[Port %d] Removing active devices before deleting bus...", i); + // Remove all registered devices for this port from the bus. A + // handle is dropped only after a successful removal: on failure + // the device is still attached, and keeping the pointer allows a + // retry instead of blocking bus deletion forever. + for (i2c_dev_node_t *node = i2c_ports[i].devices; node; node = node->next) { - if (i2c_ports[i].installed) + i2c_dev_t *dev_ptr = node->dev; + if (dev_ptr && dev_ptr->dev_handle) { - ESP_LOGV(TAG, "[Port %d] Removing active devices before deleting bus...", i); - // Remove all registered devices for this port from the bus - for (int j = 0; j < CONFIG_I2CDEV_MAX_DEVICES_PER_PORT; j++) + ESP_LOGV(TAG, "[Port %d] Removing device 0x%02x (Handle %p)", i, dev_ptr->addr, dev_ptr->dev_handle); + esp_err_t rm_res = i2c_master_bus_rm_device(dev_ptr->dev_handle); + if (rm_res != ESP_OK) { - i2c_dev_t *dev_ptr = active_devices[i][j]; - if (dev_ptr != NULL && dev_ptr->dev_handle != NULL) - { - ESP_LOGV(TAG, "[Port %d] Removing device 0x%02x (Handle %p)", i, dev_ptr->addr, dev_ptr->dev_handle); - esp_err_t rm_res = i2c_master_bus_rm_device(dev_ptr->dev_handle); - if (rm_res != ESP_OK) - { - ESP_LOGE(TAG, "[Port %d] Failed to remove device 0x%02x handle: %d", i, dev_ptr->addr, rm_res); - // Continue cleanup despite error - if (result == ESP_OK) - result = rm_res; // Report first error - } - dev_ptr->dev_handle = NULL; - } + ESP_LOGE(TAG, "[Port %d] Failed to remove device 0x%02x handle: %d", i, dev_ptr->addr, rm_res); + if (result == ESP_OK) + result = rm_res; // Report first error + released = false; } - - ESP_LOGV(TAG, "[Port %d] Deleting master bus handle %p...", i, i2c_ports[i].bus_handle); - esp_err_t del_res = i2c_del_master_bus(i2c_ports[i].bus_handle); - if (del_res != ESP_OK) + else { - ESP_LOGE(TAG, "[Port %d] Failed to delete I2C bus during cleanup: %d", i, del_res); - if (result == ESP_OK) - result = del_res; + dev_ptr->dev_handle = NULL; } + } + } + + if (released) + { + ESP_LOGV(TAG, "[Port %d] Deleting master bus handle %p...", i, i2c_ports[i].bus_handle); + esp_err_t del_res = i2c_del_master_bus(i2c_ports[i].bus_handle); + if (del_res != ESP_OK) + { + // The bus is still installed in the driver. Keep the + // handle: dropping it here would leak the port until + // reboot, because i2cdev would no longer know about it. + ESP_LOGE(TAG, "[Port %d] Failed to delete I2C bus during cleanup: %d", i, del_res); + if (result == ESP_OK) + result = del_res; + released = false; + } + else + { i2c_ports[i].installed = false; i2c_ports[i].bus_handle = NULL; - i2c_ports[i].ref_count = 0; } - xSemaphoreGive(i2c_ports[i].lock); - } // End else (mutex taken) + } + } - ESP_LOGV(TAG, "[Port %d] Deleting port mutex...", i); - vSemaphoreDelete(i2c_ports[i].lock); - i2c_ports[i].lock = NULL; - // Clear the active device list for this port - memset(active_devices[i], 0, sizeof(active_devices[i])); - ESP_LOGV(TAG, "[Port %d] Cleanup complete.", i); + xSemaphoreGive(i2c_ports[i].lock); - } // end if lock exists + if (!released) + { + ESP_LOGW(TAG, "[Port %d] Port was not fully released, keeping it for a later cleanup attempt", i); + continue; + } + + ESP_LOGV(TAG, "[Port %d] Deleting port mutex...", i); + vSemaphoreDelete(i2c_ports[i].lock); + i2c_ports[i].lock = NULL; + // Clear the device list for this port + free_device_list(&i2c_ports[i]); + ESP_LOGV(TAG, "[Port %d] Cleanup complete.", i); } // end for loop ESP_LOGV(TAG, "I2C subsystem cleanup finished with result: %d", result); return result; @@ -958,7 +1010,7 @@ esp_err_t i2cdev_done(void) esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle) { - if (port >= I2C_NUM_MAX || bus_handle == NULL) + if (port < 0 || port >= I2C_NUM_MAX || !bus_handle) { ESP_LOGE(TAG, "Invalid arguments: port=%d, bus_handle=%p", port, bus_handle); return ESP_ERR_INVALID_ARG; @@ -967,7 +1019,7 @@ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle) i2c_port_state_t *port_state = &i2c_ports[port]; // Check if i2cdev subsystem is initialized - if (port_state->lock == NULL) + if (!port_state->lock) { ESP_LOGE(TAG, "[Port %d] I2C subsystem not initialized - call i2cdev_init() first", port); return ESP_ERR_INVALID_STATE; @@ -981,7 +1033,7 @@ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle) } // Check if bus is initialized - if (!port_state->installed || port_state->bus_handle == NULL) + if (!port_state->installed || !port_state->bus_handle) { xSemaphoreGive(port_state->lock); ESP_LOGE(TAG, "[Port %d] Bus not initialized - create an i2c_dev_t device first to initialize the bus", port); @@ -991,9 +1043,9 @@ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle) // Return the bus handle *bus_handle = (void *)port_state->bus_handle; - ESP_LOGI(TAG, "[Port %d] Shared bus handle retrieved: %p (SDA=%d, SCL=%d, ref_count=%" PRIu32 ")", + ESP_LOGI(TAG, "[Port %d] Shared bus handle retrieved: %p (SDA=%d, SCL=%d)", port, port_state->bus_handle, port_state->sda_pin_current, - port_state->scl_pin_current, port_state->ref_count); + port_state->scl_pin_current); xSemaphoreGive(port_state->lock); return ESP_OK; diff --git a/i2cdev.h b/i2cdev.h index 5a56bba..ba4ef10 100644 --- a/i2cdev.h +++ b/i2cdev.h @@ -3,10 +3,17 @@ * @defgroup i2cdev i2cdev * @{ * - * ESP-IDF I2C master thread-safe functions for communication with I2C slave + * ESP-IDF I2C master functions for communication with I2C slave * - * This implementation uses the newer ESP-IDF I2C master driver (v5.0+). - * For ESP-IDF versions using the legacy I2C driver, use i2cdev_legacy.c instead. + * Thread safety is per device descriptor: every access to a device must be + * performed under I2C_DEV_TAKE_MUTEX()/I2C_DEV_GIVE_MUTEX(). The bus itself is + * serialised by the ESP-IDF I2C driver. With CONFIG_I2CDEV_NOLOCK enabled no + * per-device mutex exists and serialisation becomes the caller's responsibility. + * + * This header is served by two implementations, selected by the build system: + * i2cdev.c (new i2c_master driver, ESP-IDF v5.3 and later, unless legacy is + * requested explicitly) and i2cdev_legacy.c (legacy driver/i2c.h, used on + * older ESP-IDF or with CONFIG_I2CDEV_USE_LEGACY_DRIVER=y). * * Copyright (C) 2018 Ruslan V. Uss * Updated 2025 by quinkq to use newer ESP-IDF I2C master driver API @@ -23,7 +30,7 @@ * * Optional auto-pullup (CONFIG_I2CDEV_AUTO_ENABLE_PULLUPS=y): * - If both pullup flags are false (not set/default state), automatically enables internal pullups - * - Only available on ESP32 family (modern driver) + * - Implemented by the i2c_master based driver only (i2cdev.c), on any target * - Legacy driver always uses explicit configuration * * @@ -68,7 +75,8 @@ typedef enum } i2c_addr_bit_len_t; #endif -// Definition for I2CDEV_MAX_STRETCH_TIME +// Clock stretching timeout for the legacy driver only: i2cdev_legacy.c uses it +// when dev->timeout_ticks is 0. The i2c_master based driver has no such knob. #if HELPER_TARGET_IS_ESP8266 #define I2CDEV_MAX_STRETCH_TIME 0xffffffff #else @@ -91,7 +99,7 @@ typedef enum #endif #ifndef CONFIG_I2CDEV_MAX_DEVICES_PER_PORT -#define CONFIG_I2CDEV_MAX_DEVICES_PER_PORT 8 // Maximum devices per I2C port +#define CONFIG_I2CDEV_MAX_DEVICES_PER_PORT 8 // Legacy driver only: devices per I2C port #endif #ifndef CONFIG_I2CDEV_DEFAULT_SDA_PIN @@ -102,14 +110,6 @@ typedef enum #define CONFIG_I2CDEV_DEFAULT_SCL_PIN 22 // Default SCL pin #endif -#ifndef CONFIG_FREERTOS_HZ -#define CONFIG_FREERTOS_HZ 100 // Default value in most ESP-IDF configs -#endif - -#ifndef CONFIG_LOG_MAXIMUM_LEVEL -#define CONFIG_LOG_MAXIMUM_LEVEL 3 // INFO level as default -#endif - #ifdef __cplusplus extern "C" { #endif @@ -128,29 +128,46 @@ typedef enum * * This structure supports both legacy ESP-IDF I2C driver and modern i2c_master driver. * - * @note INITIALIZATION CHECKLIST - Set these fields before calling i2c_dev_create_mutex(): - * - * ┌─── REQUIRED (Set by user) ───────────────────────────────────────────┐ - * │ - dev->port - I2C port number (e.g., I2C_NUM_0) │ - * │ - dev->addr - Device I2C address (e.g., 0x48) │ - * │ - dev->cfg.sda_io_num - SDA pin (-1 = use Kconfig default) │ - * │ - dev->cfg.scl_io_num - SCL pin (-1 = use Kconfig default) │ - * │ - dev->cfg.master.clk_speed - Clock speed in Hz (e.g., 400000) │ - * └──────────────────────────────────────────────────────────────────────┘ - * - * ┌─── OPTIONAL (Set by user if needed) ─────────────────────────────────┐ - * │ - dev->addr_bit_len - Address format (defaults to 7-bit) - NEW │ - * │ - 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) │ - * └──────────────────────────────────────────────────────────────────────┘ - * - * ┌─── AUTO-POPULATED (library fills these) ─────────────────────────────┐ - * │ - dev->mutex - Device mutex handle │ - * │ - dev->dev_handle - I2C device handle (modern driver) - NEW │ - * │ - dev->sda_pin - Actual SDA pin used by bus │ - * │ - dev->scl_pin - Actual SCL pin used by bus │ - * └──────────────────────────────────────────────────────────────────────┘ + * @note INITIALIZATION CHECKLIST - fill the descriptor fields before the + * first I/O operation on it, not before i2c_dev_create_mutex(). + * + * REQUIRED (set by user): + * - dev->port: port number, 0 .. I2C_NUM_MAX-1 + * - dev->addr: device I2C address (for example 0x48) + * - dev->cfg.sda_io_num: SDA pin, -1 means "use the Kconfig default" + * - dev->cfg.scl_io_num: SCL pin, -1 means "use the Kconfig default" + * + * OPTIONAL (set by user if needed): + * - dev->addr_bit_len: address format, defaults to 7-bit + * - dev->cfg.master.clk_speed: clock speed in Hz, 0 means 400000 with a warning + * - dev->cfg.sda_pullup_en: enable internal SDA pullup + * - dev->cfg.scl_pullup_en: enable internal SCL pullup + * - dev->timeout_ticks: clock stretching timeout, legacy driver only + * + * AUTO-POPULATED (library fills these): + * - dev->dev_handle: device handle, created lazily, modern driver only + * - dev->sda_pin: actual SDA pin used by the bus + * - dev->scl_pin: actual SCL pin used by the bus + * + * @note dev->mutex is NOT populated automatically: it is created by + * i2c_dev_create_mutex() and does not exist at all with + * CONFIG_I2CDEV_NOLOCK enabled. + * + * @note dev->cfg.clk_flags exists only to keep the i2c_config_t compatible + * layout; both implementations ignore it. + * + * @note One port is bound to one pin pair: the first operation fixes SDA/SCL + * for the port, and a device that asks for other pins on the same port + * fails with ESP_ERR_INVALID_STATE. + * + * @note 10-bit addressing requires SOC_I2C_SUPPORT_10BIT_ADDR; elsewhere + * addr_bit_len is forced to 7-bit (the driver logs a warning). + * + * @note LIFETIME: the library keeps a pointer to the descriptor once the + * device has been added to the bus. The descriptor must stay valid + * until i2c_dev_delete_mutex() was called for it (or i2cdev_done() + * released the port): do not copy, move or free it while registered, + * i2cdev_done() walks the registered descriptors. * * @note BACKWARD COMPATIBILITY DESIGN: * The custom 'cfg' structure mimics ESP-IDF's deprecated i2c_config_t layout @@ -162,12 +179,12 @@ typedef enum typedef struct { // ═══ Core Device Identity (REQUIRED) ═══ - i2c_port_t port; //!< I2C port number (e.g., I2C_NUM_0) + i2c_port_t port; //!< I2C port number: 0 .. I2C_NUM_MAX-1 uint16_t addr; //!< Device I2C address (e.g., 0x48 for 7-bit) - i2c_addr_bit_len_t addr_bit_len; //!< Address format: I2C_ADDR_BIT_LEN_7 (default) or I2C_ADDR_BIT_LEN_10 + i2c_addr_bit_len_t addr_bit_len; //!< Address format: I2C_ADDR_BIT_LEN_7 (default) or I2C_ADDR_BIT_LEN_10 where SOC_I2C_SUPPORT_10BIT_ADDR - // ═══ Library Internal State (AUTO-POPULATED) ═══ - SemaphoreHandle_t mutex; //!< Device mutex - Created by i2c_dev_create_mutex() + // ═══ Library Internal State ═══ + SemaphoreHandle_t mutex; //!< Device mutex - Created by i2c_dev_create_mutex(), unused with CONFIG_I2CDEV_NOLOCK void *dev_handle; //!< Device handle - Modern driver only, created lazily (when actual I2C operation is performed) int sda_pin; //!< Actual SDA pin used - Populated after port setup int scl_pin; //!< Actual SCL pin used - Populated after port setup @@ -175,7 +192,7 @@ typedef struct // ═══ Legacy Driver Compatibility ═══ uint32_t timeout_ticks; //!< Clock stretching timeout - Legacy driver only - // ═══ User Configuration (REQUIRED) ═══ + // ═══ User Configuration ═══ // Configuration structure with i2c_config_t compatible field layout. struct { @@ -183,10 +200,10 @@ typedef struct gpio_num_t scl_io_num; //!< Desired SCL pin (-1 = use Kconfig default) uint8_t sda_pullup_en; //!< Enable internal SDA pullup (optional) uint8_t scl_pullup_en; //!< Enable internal SCL pullup (optional) - uint32_t clk_flags; //!< Bitwise of ``I2C_SCLK_SRC_FLAG_**FOR_DFS**`` for clk source choice + uint32_t clk_flags; //!< Reserved for i2c_config_t layout compatibility: ignored by both drivers struct { - uint32_t clk_speed; //!< Clock speed in Hz + uint32_t clk_speed; //!< Clock speed in Hz, 0 = 400000 with a warning in the log } master; //!< Master-specific config (mimics old i2c_config_t.master) } cfg; //!< Configuration set by device drivers (i2c_config_t compatible layout) } i2c_dev_t; @@ -197,7 +214,13 @@ typedef struct * @note This should be called once at the beginning of your application * before any I2C devices are initialized. * - * @return ESP_OK on success + * @note Calling it more than once is allowed and costs nothing beyond creating + * the port mutexes that are missing, so it is safe to call again after + * i2cdev_done(). A port whose bus is still installed (because its cleanup + * failed) keeps that bus instead of being reset. + * + * @return `ESP_OK` on success, `ESP_ERR_NO_MEM` if a port mutex could not be + * created */ esp_err_t i2cdev_init(void); @@ -207,20 +230,40 @@ esp_err_t i2cdev_init(void); * @note Call this when no more I2C operations will be performed * to clean up resources. * - * @return ESP_OK on success + * @note Cleanup is best effort and may be partial. If a bus or one of its + * devices cannot be released (for example, the bus was handed out via + * i2cdev_get_shared_handle() and is still used by another component), the + * port state is kept intact and its mutex is not deleted, so the call can + * be repeated once the obstacle is gone. Ports whose mutex is held by + * another task are skipped for the same reason. + * + * @return ESP_OK if every port was released; on a partial failure the first + * error is returned (ESP_ERR_TIMEOUT if a port mutex could not be + * taken, otherwise the error reported by the I2C driver). A non-ESP_OK + * result means the subsystem was not fully released and i2cdev_done() + * may be called again. */ esp_err_t i2cdev_done(void); /** - * @brief Create mutex for device descriptor and register device + * @brief Create mutex for device descriptor + * + * @note The descriptor fields (port, address, pins, speed) are read by the data + * transfer functions, so they must be filled before the first I/O + * operation on the device, not before this call. * - * @note IMPORTANT: Before calling this function, you must properly initialize the i2c_dev_t - * structure with device address, port, and pin settings. For ESP-IDF legacy driver, - * set pins in dev->cfg.sda_io_num and dev->cfg.scl_io_num. For newer ESP-IDF version, - * either method is compatible. See the structure documentation for details. + * @note This call only creates the per-device mutex and normalises + * dev->addr_bit_len: it does not touch the bus. The port mutexes are + * created by i2cdev_init(), the bus is created lazily by the first + * operation on the port (i2c_dev_check_present() and i2c_dev_probe() + * create the bus as well), and the device handle is added to the bus on + * the first real transfer. Probe calls do not attach the device. * * @param dev Pointer to device descriptor - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev is NULL, + * `ESP_ERR_NO_MEM` if the device mutex could not be created. With + * @c CONFIG_I2CDEV_NOLOCK no mutex is created, so this call cannot fail + * with `ESP_ERR_NO_MEM` in that configuration. */ esp_err_t i2c_dev_create_mutex(i2c_dev_t *dev); @@ -230,48 +273,92 @@ esp_err_t i2c_dev_create_mutex(i2c_dev_t *dev); * @note This function performs cleanup tasks including removing the device from the * I2C bus, deregistering it, and deleting its mutex. * + * @note The bus itself is NOT deleted here: it lives from i2cdev_init() (or the + * first I2C operation) until i2cdev_done(). Removing the last tracked device + * therefore leaves the shared bus available for any other consumer of + * i2cdev_get_shared_handle(). A device handle whose removal failed is + * preserved instead of being dropped: the failure is reported and the + * operation can be retried. + * + * @note With @c CONFIG_I2CDEV_NOLOCK enabled no per-device mutex exists, but the + * cleanup described above is still performed in that configuration. + * + * @note If the descriptor's port is out of range, or i2cdev_init() was never + * called, nothing port-scoped can be cleaned up: the call logs a warning, + * deletes only the per-device mutex and returns `ESP_OK`. + * * @param dev Pointer to device descriptor - * @return `ESP_OK` on success + * @return `ESP_OK` on success or when the port was never initialised (see the + * note above), `ESP_ERR_INVALID_ARG` if @p dev is NULL, + * `ESP_ERR_TIMEOUT` if the port mutex could not be taken, otherwise the + * error reported by the I2C driver while removing the device */ esp_err_t i2c_dev_delete_mutex(i2c_dev_t *dev); /** * @brief Take device mutex * + * @note With @c CONFIG_I2CDEV_NOLOCK enabled this is a no-op that returns + * `ESP_OK`. + * * @param dev Pointer to device descriptor - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev is NULL, + * `ESP_ERR_INVALID_STATE` if the device mutex was not created, + * `ESP_ERR_TIMEOUT` if the mutex was not released within + * @c CONFIG_I2CDEV_TIMEOUT milliseconds */ esp_err_t i2c_dev_take_mutex(i2c_dev_t *dev); /** * @brief Give device mutex * + * @note With @c CONFIG_I2CDEV_NOLOCK enabled this is a no-op that returns + * `ESP_OK`. + * * @param dev Pointer to device descriptor - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev is NULL, + * `ESP_ERR_INVALID_STATE` if the device mutex was not created, + * `ESP_FAIL` if the mutex was not taken before */ esp_err_t i2c_dev_give_mutex(i2c_dev_t *dev); /** - * @brief Check the availability of a device on the I2C bus (New Driver) - legacy's i2c_dev_probe function equivalent. + * @brief Check the availability of a device on the I2C bus * - * This function attempts to communicate with the I2C device to see if it ACKs. - * It is non-intrusive; if the device is found, any temporary setup for - * the check is torn down. Uses the new I2C driver logic. + * Probes the bus with a bare START/STOP pair and reports whether the device + * ACKs. No device handle is added to the bus and the per-device mutex is not + * used, so the device list stays untouched. Two side effects remain: the port + * is set up if this is the first operation on it (the bus is created and the + * internal pullups are configured from the descriptor), and the probe always + * runs at 100 kHz, independent of dev->cfg.master.clk_speed. dev->addr_bit_len + * is ignored: the address is sent as a 7-bit address. * * @param dev Pointer to the device descriptor. Pins and address must be configured. * @return `ESP_OK` if the device ACKs (is present), an error code otherwise. + * `ESP_ERR_INVALID_ARG` if @p dev is NULL or its port is out of range, + * `ESP_ERR_INVALID_STATE` if the port was never initialized (call + * i2cdev_init() first), `ESP_ERR_TIMEOUT` if the port mutex could not be + * taken. If the bus answered but the device did not ACK, the error comes + * from the I2C driver (typically `ESP_ERR_NOT_FOUND` or `ESP_ERR_TIMEOUT`). */ esp_err_t i2c_dev_check_present(const i2c_dev_t *dev); /** - * @brief Check the availability of a device on the I2C bus (Legacy Driver). + * @brief Check the availability of a device on the I2C bus * - * Issue an operation of `operation_type` to the I2C device then stops. - * Primarily for use with the legacy i2cdev_legacy.c implementation. + * Legacy-compatible entry point. The legacy implementation issues an operation + * of @p operation_type; the i2c_master based implementation ignores that + * argument and forwards the call to i2c_dev_check_present(). * * @param dev Device descriptor. - * @param operation_type Operation type (I2C_DEV_WRITE or I2C_DEV_READ). - * @return `ESP_OK` if device is available for the specified operation type. + * @param operation_type Operation type (I2C_DEV_WRITE or I2C_DEV_READ), ignored + * by the i2c_master based implementation. + * @return Same codes as i2c_dev_check_present(): `ESP_OK` if the device ACKs, + * `ESP_ERR_INVALID_ARG` for a NULL descriptor or a port out of range, + * `ESP_ERR_INVALID_STATE` if the port was never initialised, + * `ESP_ERR_TIMEOUT` if the port mutex could not be taken, otherwise the + * error reported by the I2C driver (`ESP_ERR_NOT_FOUND` when the device + * does not ACK). */ esp_err_t i2c_dev_probe(const i2c_dev_t *dev, i2c_dev_type_t operation_type); @@ -279,11 +366,25 @@ esp_err_t i2c_dev_probe(const i2c_dev_t *dev, i2c_dev_type_t operation_type); * @brief Read from device * * @param dev Pointer to device descriptor - * @param[in] out_data Data to write before reading (can be NULL if out_size is 0) - * @param out_size Size of data to write + * @param[in] out_data Data to write before reading, or NULL for a plain read + * @param out_size Size of data to write before reading * @param[out] in_data Buffer to store data read * @param in_size Number of bytes to read - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev or @p in_data is + * NULL, if @p in_size is 0 or if the descriptor's port is out of range, + * `ESP_ERR_INVALID_STATE` if i2cdev_init() was not called, + * `ESP_ERR_TIMEOUT` if the port mutex could not be taken, otherwise the + * error reported by the I2C driver + * + * @p out_data and @p out_size count only together: if either of them is empty, + * a plain read is performed and nothing is written first. Reading a register + * therefore needs both; i2c_dev_read_reg() is the shorthand for it. + * + * @note The call is retried up to four times (I2C_MAX_RETRIES + 1) with 20, 40 + * and 80 ms of backoff, and the device is set up again before every + * attempt. The retry loop also covers errors that cannot go away, such as + * `ESP_ERR_INVALID_ARG`. In the worst case the call blocks for about four + * times @c CONFIG_I2CDEV_TIMEOUT plus the backoff delays. */ esp_err_t i2c_dev_read(const i2c_dev_t *dev, const void *out_data, size_t out_size, void *in_data, size_t in_size); @@ -291,11 +392,20 @@ esp_err_t i2c_dev_read(const i2c_dev_t *dev, const void *out_data, size_t out_si * @brief Write to device * * @param dev Pointer to device descriptor - * @param[in] out_reg Register address to write to (can be NULL if out_reg_size is 0) + * @param[in] out_reg Register address to write to, or NULL to write data as is * @param out_reg_size Size of register address - * @param[in] out_data Data to write (can be NULL if out_size is 0) + * @param[in] out_data Data to write, or NULL to write only the register * @param out_size Size of data to write - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev is NULL, if both + * pairs are empty or if the descriptor's port is out of range, + * `ESP_ERR_INVALID_STATE` if i2cdev_init() was not called, + * `ESP_ERR_TIMEOUT` if the port mutex could not be taken, otherwise the + * error reported by the I2C driver + * + * @p out_reg and @p out_reg_size count only together, the same for @p out_data + * and @p out_size: an empty half of a pair is ignored. + * + * @note The retry policy is the same as in i2c_dev_read(). */ esp_err_t i2c_dev_write(const i2c_dev_t *dev, const void *out_reg, size_t out_reg_size, const void *out_data, size_t out_size); @@ -306,7 +416,15 @@ esp_err_t i2c_dev_write(const i2c_dev_t *dev, const void *out_reg, size_t out_re * @param reg Command to write before reading * @param[out] data Buffer to store data * @param size Number of bytes to read - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev or @p data is + * NULL, if @p size is 0 or if the descriptor's port is out of range, + * `ESP_ERR_INVALID_STATE` if i2cdev_init() was not called, + * `ESP_ERR_TIMEOUT` if the port mutex could not be taken, otherwise the + * error reported by the I2C driver + * + * @note The register address is a single byte, sent as a write before the + * repeated START of the read. The retry policy is the same as in + * i2c_dev_read(). */ esp_err_t i2c_dev_read_reg(const i2c_dev_t *dev, uint8_t reg, void *data, size_t size); @@ -317,7 +435,15 @@ esp_err_t i2c_dev_read_reg(const i2c_dev_t *dev, uint8_t reg, void *data, size_t * @param reg Command to write before writing data * @param data Buffer with data to write * @param size Number of bytes to write - * @return `ESP_OK` on success + * @return `ESP_OK` on success, `ESP_ERR_INVALID_ARG` if @p dev or @p data is + * NULL, if @p size is 0 or if the descriptor's port is out of range, + * `ESP_ERR_INVALID_STATE` if i2cdev_init() was not called, + * `ESP_ERR_TIMEOUT` if the port mutex could not be taken, otherwise the + * error reported by the I2C driver + * + * @note The register address is a single byte prepended to @p data. To write + * only the register byte use i2c_dev_write(dev, ®, 1, NULL, 0). The + * retry policy is the same as in i2c_dev_read(). */ esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, size_t size); @@ -325,13 +451,16 @@ esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, * @brief Get shared I2C bus handle for external ESP-IDF components * * This function allows external ESP-IDF components (like esp_lcd) to access - * the i2cdev-managed I2C bus handle for shared bus operations. This enables - * thread-safe sharing of the I2C bus between i2cdev-based sensors and native - * ESP-IDF components. + * the i2cdev-managed I2C bus handle for shared bus operations. The bus lock of + * the ESP-IDF driver serialises transactions, so i2cdev-based sensors and + * native ESP-IDF components can share one bus. Only the bus is covered by that: + * devices added by an external component are unknown to i2cdev, so removing + * them stays the caller's job (see the cleanup step in the example). * * @note USAGE PATTERN: * 1. Initialize i2cdev subsystem with i2cdev_init() - * 2. Create at least one i2c_dev_t device on the desired port (this initializes the bus) + * 2. Create at least one i2c_dev_t device on the desired port and perform at least + * one I2C operation on it (the bus is created lazily, on the first operation) * 3. Call this function to retrieve the bus handle * 4. Pass the handle to ESP-IDF components (e.g., esp_lcd_new_panel_io_i2c) * @@ -339,7 +468,7 @@ esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, * // 1. Initialize i2cdev * i2cdev_init(); * - * // 2. Create a sensor device (initializes the bus) + * // 2. Describe a device on the port; the fields are read by the first operation * i2c_dev_t sensor = { * .port = I2C_NUM_0, * .addr = 0x29, // TSL2591 light sensor @@ -350,6 +479,10 @@ esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, * } * }; * i2c_dev_create_mutex(&sensor); + * // The bus does not exist until the first operation on the port: this call + * // creates it. Its result is not checked on purpose - any transfer that sets + * // the port up does the job, even one that fails to find the device. + * (void)i2c_dev_check_present(&sensor); * * // 3. Get the shared bus handle * i2c_master_bus_handle_t bus_handle; @@ -366,7 +499,17 @@ esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, * }; * ESP_ERROR_CHECK(esp_lcd_new_panel_io_i2c(bus_handle, &io_config, &io_handle)); * - * // Both the sensor and display now share the same I2C bus with proper synchronization! + * // 5. Cleanup, in this order + * esp_lcd_panel_io_del(io_handle); // external device leaves the bus + * i2c_dev_delete_mutex(&sensor); // per-device mutex and device handle + * i2cdev_done(); // now the bus can be deleted + * + * // Both the sensor and display use the same I2C bus. Transactions are + * // serialised by the ESP-IDF bus lock; per-device serialisation stays the + * // caller's job: an i2c_dev_t handle must be used under + * // I2C_DEV_TAKE_MUTEX()/I2C_DEV_GIVE_MUTEX(), and the external component + * // serialises its own device handle. Delete the external device before + * // i2cdev_done(): i2cdev does not track it and cannot remove it from the bus. * * @param port I2C port number (e.g., I2C_NUM_0) * @param[out] bus_handle Pointer to store the bus handle @@ -376,12 +519,15 @@ esp_err_t i2c_dev_write_reg(const i2c_dev_t *dev, uint8_t reg, const void *data, * @return ESP_ERR_TIMEOUT if port mutex could not be acquired * * @warning The returned handle is managed by i2cdev. Do NOT call i2c_del_master_bus() - * on it directly - i2cdev will handle cleanup when the last device is removed. + * on it directly: the bus lives until i2cdev_done(), which releases it. */ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle); /** * @brief Take device mutex with error checking + * + * @note With @c CONFIG_I2CDEV_NOLOCK enabled this is a no-op that returns + * @c ESP_OK; only @p dev is validated. */ #define I2C_DEV_TAKE_MUTEX(dev) \ do \ @@ -394,6 +540,9 @@ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle); /** * @brief Give device mutex with error checking + * + * @note With @c CONFIG_I2CDEV_NOLOCK enabled this is a no-op that returns + * @c ESP_OK; only @p dev is validated. */ #define I2C_DEV_GIVE_MUTEX(dev) \ do \ @@ -405,7 +554,11 @@ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle); while (0) /** - * @brief Execute operation, assuming mutex is held. Gives mutex ONLY on error. + * @brief Execute operation, assuming the device mutex is held. Gives the mutex + * ONLY on error. + * + * On success the mutex stays taken, so the caller must release it with + * I2C_DEV_GIVE_MUTEX(). With @c CONFIG_I2CDEV_NOLOCK that release is a no-op. */ #define I2C_DEV_CHECK(dev, X) \ do \ @@ -421,7 +574,12 @@ esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle); while (0) /** - * @brief Execute operation, assuming mutex is held. Gives mutex ONLY on error, logs error. + * @brief Execute operation, assuming the device mutex is held. Gives the mutex + * ONLY on error and logs the failure. + * + * Like I2C_DEV_CHECK(), the mutex stays taken on success, so the caller must + * call I2C_DEV_GIVE_MUTEX(). The failure is logged through @c TAG, which has to + * be defined in the scope where the macro is used. */ #define I2C_DEV_CHECK_LOGE(dev, X, msg, ...) \ do \ diff --git a/i2cdev_legacy.c b/i2cdev_legacy.c index 9cbcb97..4f296ed 100644 --- a/i2cdev_legacy.c +++ b/i2cdev_legacy.c @@ -264,6 +264,11 @@ esp_err_t i2cdev_done() return ESP_OK; } +esp_err_t i2cdev_get_shared_handle(i2c_port_t port, void **bus_handle) +{ + return ESP_ERR_NOT_SUPPORTED; +} + esp_err_t i2c_dev_create_mutex(i2c_dev_t *dev) { #if !CONFIG_I2CDEV_NOLOCK diff --git a/idf_component.yml b/idf_component.yml index 7d2cca9..22846bf 100644 --- a/idf_component.yml +++ b/idf_component.yml @@ -1,5 +1,5 @@ --- -version: 2.1.3 +version: 2.2.0 description: ESP-IDF I2C master thread-safe utilities license: MIT dependencies: