From d1579af4b1efbde678903663733e7b375ec72f7c Mon Sep 17 00:00:00 2001 From: harmon25 Date: Wed, 1 Jul 2026 18:28:05 -0400 Subject: [PATCH 1/3] esp32: migrate i2c drivers to the new I2C master driver (i2c_master.h) Migrate both the "i2c" port driver and the i2c NIF-resource driver from the legacy driver/i2c.h API to the new driver/i2c_master.h API. The legacy driver is end-of-life as of IDF v6.0 and scheduled for removal in v7.0. - Add a shared i2c_bus_manager helper used by both implementations for bus open/close and per-address device add/transact/remove, since the new driver requires a persistent per-address device handle while AtomVM's i2c API takes the target address on every call (e.g. the i2c_scanner example probes arbitrary addresses on one open bus). - Replace the legacy command-link queuing (begin_transmission/ write_byte(s)/end_transmission) with a small growable byte buffer that is flushed as a single i2c_master_transmit() call. - Redefine i2c_driver_acquire/i2c_driver_release to hand out the new i2c_master_bus_handle_t instead of i2c_port_t; this API has no in-tree callers and was already documented as subject to change. - Normalize the new driver's NACK error codes (ESP_ERR_INVALID_STATE on IDF <= 5.5, ESP_ERR_INVALID_RESPONSE on IDF >= 6.0) back to ESP_FAIL, preserving the {error, esp_fail} contract existing applications and the i2c test suite rely on. - Link esp_driver_i2c explicitly for IDF >= 6.0, matching the existing esp_driver_dac/tinyusb pattern in this component. Verified by building libavm_builtins.a against real ESP-IDF v5.2.7 and v5.5.4 for both esp32 and esp32c3 targets. Signed-off-by: harmon25 --- CHANGELOG.md | 5 + .../components/avm_builtins/CMakeLists.txt | 8 + .../components/avm_builtins/i2c_bus_manager.c | 188 ++++++++++ .../components/avm_builtins/i2c_driver.c | 336 ++++++------------ .../components/avm_builtins/i2c_resource.c | 265 +++++--------- .../avm_builtins/include/i2c_bus_manager.h | 117 ++++++ .../avm_builtins/include/i2c_driver.h | 19 +- 7 files changed, 515 insertions(+), 423 deletions(-) create mode 100644 src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c create mode 100644 src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c82fb122c..598a52e42f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added USB CDC port drivers for ESP32, RP2, and STM32 platforms ### Changed +- ESP32: migrated both the `i2c` port driver and the `i2c` NIF-resource driver from the legacy, + now end-of-life (and slated for removal in IDF v7.0) `driver/i2c.h` API to the new + `driver/i2c_master.h` API. This is an internal implementation change with no expected impact + on the Erlang-level `i2c` API; `{error, esp_fail}` is still returned for NACK'd transactions + on all supported IDF versions - Updated network type db() to dbm() to reflect the actual representation of the type - Use ES6 modules for emscripten port, using .mjs suffix - `ahttp_client` now returns `{error, {parser, incomplete_response}}` when a socket closes mid-response diff --git a/src/platforms/esp32/components/avm_builtins/CMakeLists.txt b/src/platforms/esp32/components/avm_builtins/CMakeLists.txt index aad06d9767..c79ba0ad9a 100644 --- a/src/platforms/esp32/components/avm_builtins/CMakeLists.txt +++ b/src/platforms/esp32/components/avm_builtins/CMakeLists.txt @@ -21,6 +21,7 @@ set(AVM_BUILTIN_COMPONENT_SRCS "dac_driver.c" "gpio_driver.c" + "i2c_bus_manager.c" "i2c_driver.c" "i2c_resource.c" "ledc_nif.c" @@ -74,6 +75,13 @@ if(CONFIG_AVM_ENABLE_USB_CDC_PORT_DRIVER) endif() endif() +# The (new, non-legacy) I2C master driver (driver/i2c_master.h) is provided by the "driver" +# component up to IDF 5.x, but starting with IDF 6.0 the legacy "driver" component no longer +# has a public dependency on it, so it must be linked explicitly. +if((CONFIG_AVM_ENABLE_I2C_PORT_DRIVER OR CONFIG_AVM_ENABLE_I2C_RESOURCE_NIFS) AND IDF_VERSION_MAJOR GREATER_EQUAL 6) + target_link_libraries(${COMPONENT_LIB} PRIVATE idf::esp_driver_i2c) +endif() + if (IDF_VERSION_MAJOR EQUAL 4) idf_build_set_property( LINK_OPTIONS "-Wl,--whole-archive ${CMAKE_CURRENT_BINARY_DIR}/lib${COMPONENT_NAME}.a -Wl,--no-whole-archive" diff --git a/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c b/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c new file mode 100644 index 0000000000..375a1b06bf --- /dev/null +++ b/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c @@ -0,0 +1,188 @@ +/* + * This file is part of AtomVM. + * + * Copyright 2026 The AtomVM Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later + */ + +#include "include/i2c_bus_manager.h" + +#include +#include + +#include + +#include "utils.h" + +#define TAG "i2c_bus_manager" + +#define I2C_TX_BUFFER_INITIAL_CAP 16 + +esp_err_t i2c_bus_manager_open( + int i2c_port, int scl_io_num, int sda_io_num, i2c_master_bus_handle_t *out_bus_handle) +{ + i2c_master_bus_config_t bus_config; + memset(&bus_config, 0, sizeof(bus_config)); + bus_config.i2c_port = i2c_port; + bus_config.scl_io_num = scl_io_num; + bus_config.sda_io_num = sda_io_num; + bus_config.clk_source = I2C_CLK_SRC_DEFAULT; + // Matches the glitch filter length recommended by Espressif examples and + // used by other in-tree/vendored I2C master bus users. + bus_config.glitch_ignore_cnt = 7; + bus_config.flags.enable_internal_pullup = true; + + return i2c_new_master_bus(&bus_config, out_bus_handle); +} + +esp_err_t i2c_bus_manager_close(i2c_master_bus_handle_t bus_handle) +{ + return i2c_del_master_bus(bus_handle); +} + +static esp_err_t add_device(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, i2c_master_dev_handle_t *out_dev_handle) +{ + i2c_device_config_t dev_config; + memset(&dev_config, 0, sizeof(dev_config)); + dev_config.dev_addr_length = I2C_ADDR_BIT_LEN_7; + dev_config.device_address = address; + dev_config.scl_speed_hz = clock_speed_hz; + + return i2c_master_bus_add_device(bus_handle, &dev_config, out_dev_handle); +} + +// The legacy `driver/i2c.h` API (via `i2c_master_cmd_begin()`) always reported +// a NACK'd transaction as `ESP_FAIL`. The new I2C master driver reports the +// very same condition as `ESP_ERR_INVALID_STATE` on IDF <= 5.5, and as +// `ESP_ERR_INVALID_RESPONSE` starting with IDF 6.0 (see the "I2C Master +// Driver Updates" section of the IDF 6.0 migration guide). To preserve a +// single, version-independent `{error, esp_fail}` contract for existing +// AtomVM applications (and the `i2c` NACK test suite) across all supported +// IDF versions, both codes are normalized to `ESP_FAIL` here. +// +// NOTE: because each transaction owns a freshly-added, then removed, device +// handle (see `add_device`/`remove_device`), a NACK is by far the most likely +// cause of either of these two codes in this driver's usage pattern. +static esp_err_t normalize_transaction_error(esp_err_t err) +{ + if (err == ESP_ERR_INVALID_STATE || err == ESP_ERR_INVALID_RESPONSE) { + return ESP_FAIL; + } + return err; +} + +static esp_err_t remove_device(i2c_master_dev_handle_t dev_handle, esp_err_t transaction_result) +{ + esp_err_t err = i2c_master_bus_rm_device(dev_handle); + if (UNLIKELY(err != ESP_OK)) { + ESP_LOGW(TAG, "Failed to remove transient I2C device handle: err=%i", err); + } + // Prefer surfacing the transaction error (if any) over a remove-device + // error, since the former is more useful/actionable to the caller. + return (transaction_result != ESP_OK) ? transaction_result : err; +} + +esp_err_t i2c_bus_manager_transmit(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, const uint8_t *write_buffer, size_t write_size, int xfer_timeout_ms) +{ + i2c_master_dev_handle_t dev_handle; + esp_err_t err = add_device(bus_handle, address, clock_speed_hz, &dev_handle); + if (UNLIKELY(err != ESP_OK)) { + return err; + } + + err = normalize_transaction_error( + i2c_master_transmit(dev_handle, write_buffer, write_size, xfer_timeout_ms)); + + return remove_device(dev_handle, err); +} + +esp_err_t i2c_bus_manager_receive(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms) +{ + i2c_master_dev_handle_t dev_handle; + esp_err_t err = add_device(bus_handle, address, clock_speed_hz, &dev_handle); + if (UNLIKELY(err != ESP_OK)) { + return err; + } + + err = normalize_transaction_error( + i2c_master_receive(dev_handle, read_buffer, read_size, xfer_timeout_ms)); + + return remove_device(dev_handle, err); +} + +esp_err_t i2c_bus_manager_transmit_receive(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, const uint8_t *write_buffer, size_t write_size, + uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms) +{ + i2c_master_dev_handle_t dev_handle; + esp_err_t err = add_device(bus_handle, address, clock_speed_hz, &dev_handle); + if (UNLIKELY(err != ESP_OK)) { + return err; + } + + err = normalize_transaction_error(i2c_master_transmit_receive( + dev_handle, write_buffer, write_size, read_buffer, read_size, xfer_timeout_ms)); + + return remove_device(dev_handle, err); +} + +void i2c_tx_buffer_init(struct I2CTxBuffer *buf) +{ + buf->data = NULL; + buf->len = 0; + buf->cap = 0; +} + +void i2c_tx_buffer_reset(struct I2CTxBuffer *buf) +{ + free(buf->data); + buf->data = NULL; + buf->len = 0; + buf->cap = 0; +} + +esp_err_t i2c_tx_buffer_append(struct I2CTxBuffer *buf, const uint8_t *data, size_t len) +{ + if (len == 0) { + return ESP_OK; + } + + if (buf->len + len > buf->cap) { + size_t new_cap = buf->cap == 0 ? I2C_TX_BUFFER_INITIAL_CAP : buf->cap * 2; + while (new_cap < buf->len + len) { + new_cap *= 2; + } + uint8_t *new_data = realloc(buf->data, new_cap); + if (IS_NULL_PTR(new_data)) { + return ESP_ERR_NO_MEM; + } + buf->data = new_data; + buf->cap = new_cap; + } + + memcpy(buf->data + buf->len, data, len); + buf->len += len; + + return ESP_OK; +} + +esp_err_t i2c_tx_buffer_append_byte(struct I2CTxBuffer *buf, uint8_t byte) +{ + return i2c_tx_buffer_append(buf, &byte, 1); +} diff --git a/src/platforms/esp32/components/avm_builtins/i2c_driver.c b/src/platforms/esp32/components/avm_builtins/i2c_driver.c index d9e9818033..82c7d8e5d8 100644 --- a/src/platforms/esp32/components/avm_builtins/i2c_driver.c +++ b/src/platforms/esp32/components/avm_builtins/i2c_driver.c @@ -21,9 +21,11 @@ #include #ifdef CONFIG_AVM_ENABLE_I2C_PORT_DRIVER +#include #include -#include +#include +#include #include #include #include @@ -49,10 +51,15 @@ #include "esp32_sys.h" #include "sys.h" +#include "include/i2c_bus_manager.h" #include "include/i2c_driver.h" #define TAG "i2c_driver" +// Matches the previous legacy-driver behavior, which always waited +// indefinitely for a bus transaction to complete. +#define I2C_PORT_DRIVER_TIMEOUT_MS I2C_BUS_MANAGER_TIMEOUT_INFINITE + static void i2c_driver_init(GlobalContext *global); static Context *i2c_driver_create_port(GlobalContext *global, term opts); @@ -88,9 +95,12 @@ static const AtomStringIntPair cmd_table[] = { struct I2CData { - i2c_cmd_handle_t cmd; + i2c_master_bus_handle_t bus_handle; + struct I2CTxBuffer tx_buf; term transmitting_pid; - i2c_port_t i2c_num; + int i2c_port; + uint32_t clock_speed_hz; + uint8_t tx_address; // no need to make it atomic, we use it only when the process table is locked int ref_count; @@ -112,6 +122,7 @@ Context *i2c_driver_create_port(GlobalContext *global, term opts) struct I2CData *i2c_data = calloc(1, sizeof(struct I2CData)); i2c_data->ref_count = 1; i2c_data->transmitting_pid = term_invalid_term(); + i2c_tx_buffer_init(&i2c_data->tx_buf); term scl_io_num_term = interop_kv_get_value(opts, ATOM_STR("\x3", "scl"), global); I2C_VALIDATE_NOT_INVALID(scl_io_num); @@ -122,42 +133,30 @@ Context *i2c_driver_create_port(GlobalContext *global, term opts) term clock_hz_term = interop_kv_get_value(opts, ATOM_STR("\xE", "clock_speed_hz"), global); I2C_VALIDATE_NOT_INVALID(clock_hz); - i2c_data->i2c_num = I2C_NUM_0; + i2c_data->i2c_port = I2C_NUM_0; term i2c_num_term = interop_kv_get_value(opts, ATOM_STR("\xA", "peripheral"), global); if (!term_is_invalid_term(i2c_num_term)) { if (!term_is_integer(i2c_num_term)) { ESP_LOGE(TAG, "Invalid parameter: peripheral is not an integer"); goto free_and_exit; } - i2c_data->i2c_num = term_to_int32(i2c_num_term); - if (i2c_data->i2c_num < 0 || i2c_data->i2c_num > I2C_NUM_MAX - 1) { + i2c_data->i2c_port = term_to_int32(i2c_num_term); + if (i2c_data->i2c_port < 0 || i2c_data->i2c_port > I2C_NUM_MAX - 1) { ESP_LOGE(TAG, "Invalid parameter: i2c_num out of range"); goto free_and_exit; } } - i2c_config_t conf; - memset(&conf, 0, sizeof(i2c_config_t)); - conf.mode = I2C_MODE_MASTER; - conf.scl_io_num = term_to_int32(scl_io_num_term); - conf.sda_io_num = term_to_int32(sda_io_num_term); - conf.sda_pullup_en = GPIO_PULLUP_ENABLE; - conf.scl_pullup_en = GPIO_PULLUP_ENABLE; - conf.master.clk_speed = term_to_int32(clock_hz_term); - esp_err_t err = i2c_param_config(i2c_data->i2c_num, &conf); - - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "Failed to initialize I2C parameters. err=%s", esp_err_to_name(err)); - goto free_and_exit; - } + i2c_data->clock_speed_hz = term_to_int32(clock_hz_term); - err = i2c_driver_install(i2c_data->i2c_num, I2C_MODE_MASTER, 0, 0, 0); + esp_err_t err = i2c_bus_manager_open(i2c_data->i2c_port, term_to_int32(scl_io_num_term), + term_to_int32(sda_io_num_term), &i2c_data->bus_handle); if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "Failed to install I2C driver. err=%s", esp_err_to_name(err)); + ESP_LOGE(TAG, "Failed to create I2C master bus. err=%s", esp_err_to_name(err)); goto free_and_exit; } - ESP_LOGI(TAG, "I2C driver installed using I2C port %i", i2c_data->i2c_num); + ESP_LOGI(TAG, "I2C driver installed using I2C port %i", i2c_data->i2c_port); Context *ctx = context_new(global); ctx->native_handler = i2cdriver_consume_mailbox; @@ -179,9 +178,11 @@ static NativeHandlerResult i2c_driver_unref(Context *ctx) ctx->platform_data = NULL; - esp_err_t err = i2c_driver_delete(i2c_data->i2c_num); + i2c_tx_buffer_reset(&i2c_data->tx_buf); + + esp_err_t err = i2c_bus_manager_close(i2c_data->bus_handle); if (UNLIKELY(err != ESP_OK)) { - ESP_LOGW(TAG, "Failed to delete I2C driver. err=%s", esp_err_to_name(err)); + ESP_LOGW(TAG, "Failed to delete I2C master bus. err=%s", esp_err_to_name(err)); } free(i2c_data); @@ -189,6 +190,29 @@ static NativeHandlerResult i2c_driver_unref(Context *ctx) return NativeTerminate; } +static term make_einprogress_error(Context *ctx, term owner_pid) +{ + if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { + ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); + return OUT_OF_MEMORY_ATOM; + } + + term reason_tuple = term_alloc_tuple(2, &ctx->heap); + term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); + term_put_tuple_element(reason_tuple, 1, owner_pid); + + return port_create_error_tuple(ctx, reason_tuple); +} + +static term make_esp_err_error(Context *ctx, esp_err_t err) +{ + if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { + ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); + return OUT_OF_MEMORY_ATOM; + } + return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, err)); +} + static term i2cdriver_begin_transmission(Context *ctx, term pid, term req) { struct I2CData *i2c_data = ctx->platform_data; @@ -196,27 +220,13 @@ static term i2cdriver_begin_transmission(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != term_invalid_term())) { // another process is already transmitting ESP_LOGE(TAG, "i2cdriver_begin_transmission: Another process is already transmitting"); - - // {error, {einprogress, Pid :: pid()}} - if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - term reason_tuple = term_alloc_tuple(2, &ctx->heap); - term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); - - return port_create_error_tuple(ctx, reason_tuple); + return make_einprogress_error(ctx, i2c_data->transmitting_pid); } term address_term = term_get_tuple_element(req, 1); - uint8_t address = term_to_int32(address_term); - - i2c_data->cmd = i2c_cmd_link_create(); - i2c_master_start(i2c_data->cmd); - i2c_master_write_byte(i2c_data->cmd, (address << 1) | I2C_MASTER_WRITE, true); + i2c_tx_buffer_reset(&i2c_data->tx_buf); + i2c_data->tx_address = term_to_int32(address_term); i2c_data->transmitting_pid = pid; return OK_ATOM; @@ -229,36 +239,19 @@ static term i2cdriver_end_transmission(Context *ctx, term pid) if (UNLIKELY(i2c_data->transmitting_pid != pid)) { // transaction owned from a different pid ESP_LOGE(TAG, "i2cdriver_end_transmission: Another process is already transmitting"); - - // {error, {einprogress, Pid :: pid()}} - if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - term reason_tuple = term_alloc_tuple(2, &ctx->heap); - term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); - - return port_create_error_tuple(ctx, reason_tuple); + return make_einprogress_error(ctx, i2c_data->transmitting_pid); } - i2c_master_stop(i2c_data->cmd); - esp_err_t result = i2c_master_cmd_begin(i2c_data->i2c_num, i2c_data->cmd, portMAX_DELAY); - i2c_cmd_link_delete(i2c_data->cmd); + esp_err_t result = i2c_bus_manager_transmit(i2c_data->bus_handle, i2c_data->tx_address, + i2c_data->clock_speed_hz, i2c_data->tx_buf.data, i2c_data->tx_buf.len, + I2C_PORT_DRIVER_TIMEOUT_MS); + i2c_tx_buffer_reset(&i2c_data->tx_buf); i2c_data->transmitting_pid = term_invalid_term(); if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_end_transmission i2c_master_cmd_begin error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + ESP_LOGE(TAG, "i2cdriver_end_transmission transmit error: result was: %s", esp_err_to_name(result)); + return make_esp_err_error(ctx, result); } return OK_ATOM; @@ -271,34 +264,16 @@ static term i2cdriver_write_byte(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != pid)) { // transaction owned from a different pid ESP_LOGE(TAG, "i2cdriver_write_byte: Another process is already transmitting"); - - // {error, {einprogress, Pid :: pid()}} - if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - term reason_tuple = term_alloc_tuple(2, &ctx->heap); - term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); - - return port_create_error_tuple(ctx, reason_tuple); + return make_einprogress_error(ctx, i2c_data->transmitting_pid); } term data_term = term_get_tuple_element(req, 1); uint8_t data = term_to_int32(data_term); - esp_err_t result = i2c_master_write_byte(i2c_data->cmd, (uint8_t) data, true); - - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_write_byte: i2c_master_write_byte error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + esp_err_t err = i2c_tx_buffer_append_byte(&i2c_data->tx_buf, data); + if (UNLIKELY(err != ESP_OK)) { + ESP_LOGE(TAG, "i2cdriver_write_byte: failed to enqueue byte: err: %i.", err); + return make_esp_err_error(ctx, err); } return OK_ATOM; @@ -311,18 +286,7 @@ static term i2cdriver_qwrite_bytes(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != pid)) { // transaction owned from a different pid ESP_LOGE(TAG, "i2cdriver_qwrite_bytes: Another process is already transmitting"); - - // {error, {einprogress, Pid :: pid()}} - if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - term reason_tuple = term_alloc_tuple(2, &ctx->heap); - term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); - - return port_create_error_tuple(ctx, reason_tuple); + return make_einprogress_error(ctx, i2c_data->transmitting_pid); } term data_term = term_get_tuple_element(req, 1); @@ -333,18 +297,12 @@ static term i2cdriver_qwrite_bytes(Context *ctx, term pid, term req) } return port_create_error_tuple(ctx, BADARG_ATOM); } - esp_err_t result = i2c_master_write(i2c_data->cmd, (const uint8_t *) term_binary_data(data_term), term_binary_size(data_term), true); - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_qwrite_bytes: i2c_master_write_byte error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + esp_err_t err = i2c_tx_buffer_append(&i2c_data->tx_buf, + (const uint8_t *) term_binary_data(data_term), term_binary_size(data_term)); + if (UNLIKELY(err != ESP_OK)) { + ESP_LOGE(TAG, "i2cdriver_qwrite_bytes: failed to enqueue bytes: err: %i.", err); + return make_esp_err_error(ctx, err); } return OK_ATOM; @@ -356,18 +314,7 @@ static term i2cdriver_read_bytes(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != term_invalid_term())) { ESP_LOGE(TAG, "i2cdriver_read_bytes: Another process is already transmitting"); - - // {error, {einprogress, Pid :: pid()}} - if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - term reason_tuple = term_alloc_tuple(2, &ctx->heap); - term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); - - return port_create_error_tuple(ctx, reason_tuple); + return make_einprogress_error(ctx, i2c_data->transmitting_pid); } int arity = term_get_tuple_arity(req); @@ -379,7 +326,7 @@ static term i2cdriver_read_bytes(Context *ctx, term pid, term req) avm_int_t read_count = term_to_int32(read_bytes_term); term register_term = term_invalid_term(); - uint8_t register_address; + uint8_t register_address = 0; if (arity == 4) { register_term = term_get_tuple_element(req, 3); register_address = term_to_int32(register_term); @@ -393,57 +340,19 @@ static term i2cdriver_read_bytes(Context *ctx, term pid, term req) term data_term = term_create_uninitialized_binary(read_count, &ctx->heap, ctx->global); uint8_t *data = (uint8_t *) term_binary_data(data_term); - i2c_data->cmd = i2c_cmd_link_create(); - i2c_master_start(i2c_data->cmd); - + esp_err_t result; if (!term_is_invalid_term(register_term)) { - i2c_master_write_byte(i2c_data->cmd, (address << 1) | I2C_MASTER_WRITE, 0x1); - i2c_master_write_byte(i2c_data->cmd, register_address, 0x1); - i2c_master_start(i2c_data->cmd); - } - esp_err_t result = i2c_master_write_byte(i2c_data->cmd, (address << 1) | I2C_MASTER_READ, true); - - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_read_bytes: i2c_master_write_byte error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); - } - - result = i2c_master_read(i2c_data->cmd, data, read_count, I2C_MASTER_LAST_NACK); - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_read_bytes: i2c_master_read error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + result = i2c_bus_manager_transmit_receive(i2c_data->bus_handle, address, + i2c_data->clock_speed_hz, ®ister_address, 1, data, read_count, + I2C_PORT_DRIVER_TIMEOUT_MS); + } else { + result = i2c_bus_manager_receive(i2c_data->bus_handle, address, i2c_data->clock_speed_hz, + data, read_count, I2C_PORT_DRIVER_TIMEOUT_MS); } - i2c_master_stop(i2c_data->cmd); - result = i2c_master_cmd_begin(i2c_data->i2c_num, i2c_data->cmd, portMAX_DELAY); - i2c_cmd_link_delete(i2c_data->cmd); - - i2c_data->transmitting_pid = term_invalid_term(); - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_read_bytes: i2c_master_cmd_begin error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + ESP_LOGE(TAG, "i2cdriver_read_bytes: transaction error: result was: %s", esp_err_to_name(result)); + return make_esp_err_error(ctx, result); } term ok_tuple = term_alloc_tuple(2, &ctx->heap); @@ -459,18 +368,7 @@ static term i2cdriver_write_bytes(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != term_invalid_term())) { ESP_LOGE(TAG, "i2cdriver_write_bytes: Another process is already transmitting"); - - // {error, {einprogress, Pid :: pid()}} - if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - term reason_tuple = term_alloc_tuple(2, &ctx->heap); - term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); - - return port_create_error_tuple(ctx, reason_tuple); + return make_einprogress_error(ctx, i2c_data->transmitting_pid); } int arity = term_get_tuple_arity(req); @@ -491,62 +389,36 @@ static term i2cdriver_write_bytes(Context *ctx, term pid, term req) data_len = 1; } - term register_term = term_invalid_term(); - uint8_t register_address; + bool has_register = false; + uint8_t register_address = 0; if (arity == 4) { - register_term = term_get_tuple_element(req, 3); + term register_term = term_get_tuple_element(req, 3); register_address = term_to_int32(register_term); + has_register = true; } - i2c_data->cmd = i2c_cmd_link_create(); - i2c_master_start(i2c_data->cmd); - - esp_err_t result = i2c_master_write_byte(i2c_data->cmd, (address << 1) | I2C_MASTER_WRITE, 0x01); - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_write_bytes i2c_master_write_byte error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; + size_t write_size = (has_register ? 1 : 0) + data_len; + uint8_t *write_buf = NULL; + if (write_size > 0) { + write_buf = malloc(write_size); + if (IS_NULL_PTR(write_buf)) { + return make_esp_err_error(ctx, ESP_ERR_NO_MEM); } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); - } - - if (!term_is_invalid_term(register_term)) { - i2c_master_write_byte(i2c_data->cmd, register_address, 0x1); - } - - result = i2c_master_write(i2c_data->cmd, data, data_len, 0x01); - if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_write_bytes i2c_master_write error: result was: %s", esp_err_to_name(result)); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; + size_t offset = 0; + if (has_register) { + write_buf[0] = register_address; + offset = 1; } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + memcpy(write_buf + offset, data, data_len); } - i2c_master_stop(i2c_data->cmd); - result = i2c_master_cmd_begin(i2c_data->i2c_num, i2c_data->cmd, portMAX_DELAY); - i2c_cmd_link_delete(i2c_data->cmd); - - i2c_data->transmitting_pid = term_invalid_term(); + esp_err_t result = i2c_bus_manager_transmit(i2c_data->bus_handle, address, + i2c_data->clock_speed_hz, write_buf, write_size, I2C_PORT_DRIVER_TIMEOUT_MS); + free(write_buf); if (UNLIKELY(result != ESP_OK)) { - ESP_LOGE(TAG, "i2cdriver_write_bytes: i2c_master_cmd_begin error: result was: %i", result); - - // {error, Reason :: atom()} - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - - return port_create_error_tuple(ctx, esp_err_to_term(ctx->global, result)); + ESP_LOGE(TAG, "i2cdriver_write_bytes: transaction error: result was: %s", esp_err_to_name(result)); + return make_esp_err_error(ctx, result); } return OK_ATOM; @@ -636,7 +508,7 @@ static NativeHandlerResult i2cdriver_consume_mailbox(Context *ctx) return handler_result; } -I2CAcquireResult i2c_driver_acquire(term i2c_port, i2c_port_t *i2c_num, GlobalContext *global) +I2CAcquireResult i2c_driver_acquire(term i2c_port, i2c_master_bus_handle_t *bus_handle, GlobalContext *global) { if (UNLIKELY(!term_is_local_port(i2c_port))) { ESP_LOGW(TAG, "acquire: given term is not a port."); @@ -656,7 +528,7 @@ I2CAcquireResult i2c_driver_acquire(term i2c_port, i2c_port_t *i2c_num, GlobalCo struct I2CData *i2c_data = ctx->platform_data; i2c_data->ref_count++; - *i2c_num = i2c_data->i2c_num; + *bus_handle = i2c_data->bus_handle; globalcontext_get_process_unlock(global, ctx); diff --git a/src/platforms/esp32/components/avm_builtins/i2c_resource.c b/src/platforms/esp32/components/avm_builtins/i2c_resource.c index c8f906c85e..a0aaedfd99 100644 --- a/src/platforms/esp32/components/avm_builtins/i2c_resource.c +++ b/src/platforms/esp32/components/avm_builtins/i2c_resource.c @@ -21,11 +21,11 @@ #include +#include #include -// Note: The i2c.h legacy driver will be deprecated in 5.2 -// TODO: Migrate to i2c_master.h, once we drop support for IDF SDK 4.x -#include +#include +#include #include #include @@ -43,6 +43,8 @@ #include #include +#include "include/i2c_bus_manager.h" + #define TAG "i2c_resource" #define CHECK_ERROR(ctx, err, msg) \ @@ -54,8 +56,6 @@ if (UNLIKELY(err != ESP_OK)) { return create_error_tuple(ctx, esp_err_to_term(ctx->global, err)); \ } -#define ACK_ENABLE true -#define MS_TO_TICKS(MS) (MS / portTICK_PERIOD_MS) #define DEFAULT_SEND_TIMEOUT_MS 500 #define EINPROGRESS_ATOMSTR (ATOM_STR("\xB", "einprogress")) @@ -66,11 +66,18 @@ static ErlNifResourceType *i2c_resource_type; struct I2CResource { term transmitting_pid; - i2c_port_t i2c_num; - i2c_cmd_handle_t cmd; - uint32_t send_timeout_ms; + i2c_master_bus_handle_t bus_handle; + struct I2CTxBuffer tx_buf; + int i2c_port; + uint32_t clock_speed_hz; + // -1 (I2C_BUS_MANAGER_TIMEOUT_INFINITE) means wait forever, otherwise + // this is in milliseconds, as expected by the new I2C master driver. + int32_t xfer_timeout_ms; + uint8_t tx_address; }; +#define I2C_RESOURCE_PORT_INVALID (-1) + static term create_pair(Context *ctx, term term1, term term2) { term ret = term_alloc_tuple(2, &ctx->heap); @@ -87,15 +94,12 @@ static term create_error_tuple(Context *ctx, term reason) static bool is_i2c_resource_open(const struct I2CResource *rsrc_obj) { - return rsrc_obj->i2c_num != I2C_NUM_MAX; + return rsrc_obj->i2c_port != I2C_RESOURCE_PORT_INVALID; } static void reset_i2c_transmission_state(struct I2CResource *rsrc_obj) { - if (rsrc_obj->cmd != NULL) { - i2c_cmd_link_delete(rsrc_obj->cmd); - rsrc_obj->cmd = NULL; - } + i2c_tx_buffer_reset(&rsrc_obj->tx_buf); rsrc_obj->transmitting_pid = term_invalid_term(); } @@ -105,9 +109,9 @@ static esp_err_t close_i2c_resource(struct I2CResource *rsrc_obj) return ESP_OK; } - esp_err_t err = i2c_driver_delete(rsrc_obj->i2c_num); + esp_err_t err = i2c_bus_manager_close(rsrc_obj->bus_handle); if (err == ESP_OK) { - rsrc_obj->i2c_num = I2C_NUM_MAX; + rsrc_obj->i2c_port = I2C_RESOURCE_PORT_INVALID; reset_i2c_transmission_state(rsrc_obj); } @@ -170,28 +174,28 @@ static term nif_i2c_open(Context *ctx, int argc, term argv[]) term clock_speed_hz = interop_kv_get_value(opts, ATOM_STR("\xE", "clock_speed_hz"), global); VALIDATE_VALUE(clock_speed_hz, term_is_integer); - i2c_port_t i2c_num = I2C_NUM_0; + int i2c_port = I2C_NUM_0; term peripheral = interop_kv_get_value(opts, ATOM_STR("\xA", "peripheral"), global); if (!term_is_invalid_term(peripheral)) { if (!term_is_integer(peripheral)) { ESP_LOGE(TAG, "Invalid parameter: peripheral is not an integer"); RAISE_ERROR(BADARG_ATOM); } - i2c_num = term_to_int32(peripheral); - if (i2c_num < 0 || i2c_num > I2C_NUM_MAX - 1) { + i2c_port = term_to_int32(peripheral); + if (i2c_port < 0 || i2c_port > I2C_NUM_MAX - 1) { ESP_LOGE(TAG, "Invalid parameter: i2c_num out of range"); RAISE_ERROR(BADARG_ATOM); } } term send_timeout_ms = interop_kv_get_value_default(opts, ATOM_STR("\xF", "send_timeout_ms"), term_from_int(DEFAULT_SEND_TIMEOUT_MS), global); - uint32_t send_timeout_ms_val = portMAX_DELAY; + int32_t xfer_timeout_ms_val = I2C_BUS_MANAGER_TIMEOUT_INFINITE; if (term_is_integer(send_timeout_ms)) { if (term_to_int32(send_timeout_ms) < 0) { ESP_LOGE(TAG, "Invalid parameter: send_timeout_ms < 0"); RAISE_ERROR(BADARG_ATOM); } else { - send_timeout_ms_val = term_to_int32(send_timeout_ms); + xfer_timeout_ms_val = term_to_int32(send_timeout_ms); } } else if (send_timeout_ms != INFINITY_ATOM) { ESP_LOGE(TAG, "Invalid parameter: send_timeout_ms send_timeout_ms must be a non-negative integer or `infinity`"); @@ -199,22 +203,14 @@ static term nif_i2c_open(Context *ctx, int argc, term argv[]) } // - // Initialize config + // Create the I2C master bus // - i2c_config_t conf; - memset(&conf, 0, sizeof(i2c_config_t)); - conf.mode = I2C_MODE_MASTER; - conf.scl_io_num = term_to_int32(scl); - conf.sda_io_num = term_to_int32(sda); - conf.sda_pullup_en = GPIO_PULLUP_ENABLE; - conf.scl_pullup_en = GPIO_PULLUP_ENABLE; - conf.master.clk_speed = term_to_int32(clock_speed_hz); - - esp_err_t err; - err = i2c_param_config(i2c_num, &conf); + i2c_master_bus_handle_t bus_handle; + esp_err_t err = i2c_bus_manager_open( + i2c_port, term_to_int32(scl), term_to_int32(sda), &bus_handle); if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "Failed to initialize I2C parameters. err=%i", err); + ESP_LOGE(TAG, "Failed to create I2C master bus. err=%i", err); if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); return OUT_OF_MEMORY_ATOM; @@ -223,22 +219,7 @@ static term nif_i2c_open(Context *ctx, int argc, term argv[]) return create_error_tuple(ctx, term_from_int(err)); } - // - // Install the I2C driver - // - - err = i2c_driver_install(i2c_num, I2C_MODE_MASTER, 0, 0, 0); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "Failed to install I2C driver. err=%i", err); - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); - return OUT_OF_MEMORY_ATOM; - } - // TODO breaks i2c::open/1 API - return create_error_tuple(ctx, term_from_int(err)); - } - - ESP_LOGI(TAG, "I2C driver installed using I2C port %i", i2c_num); + ESP_LOGI(TAG, "I2C driver installed using I2C port %i", i2c_port); // // allocate and initialize the Nif resource @@ -246,14 +227,16 @@ static term nif_i2c_open(Context *ctx, int argc, term argv[]) struct I2CResource *rsrc_obj = enif_alloc_resource(i2c_resource_type, sizeof(struct I2CResource)); if (IS_NULL_PTR(rsrc_obj)) { - i2c_driver_delete(i2c_num); + i2c_bus_manager_close(bus_handle); ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); RAISE_ERROR(OUT_OF_MEMORY_ATOM); } rsrc_obj->transmitting_pid = term_invalid_term(); - rsrc_obj->i2c_num = i2c_num; - rsrc_obj->cmd = NULL; - rsrc_obj->send_timeout_ms = send_timeout_ms_val; + rsrc_obj->bus_handle = bus_handle; + rsrc_obj->i2c_port = i2c_port; + rsrc_obj->clock_speed_hz = term_to_int32(clock_speed_hz); + rsrc_obj->xfer_timeout_ms = xfer_timeout_ms_val; + i2c_tx_buffer_init(&rsrc_obj->tx_buf); if (UNLIKELY(memory_ensure_free(ctx, TERM_BOXED_RESOURCE_SIZE) != MEMORY_GC_OK)) { close_i2c_resource(rsrc_obj); @@ -397,56 +380,40 @@ static term nif_i2c_write_bytes(Context *ctx, int argc, term argv[]) RAISE_ERROR(BADARG_ATOM); } - term register_ = term_invalid_term(); - uint8_t register_address; + bool has_register = false; + uint8_t register_address = 0; if (arity == 3) { - register_ = term_get_tuple_element(req, 2); + term register_ = term_get_tuple_element(req, 2); register_address = term_to_int32(register_); + has_register = true; } // - // Enqueue and run the I2C commands + // Build the write buffer (optional register prefix + data) and send it // - esp_err_t err; - i2c_cmd_handle_t cmd = i2c_cmd_link_create(); - - err = i2c_master_start(cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_write_bytes; enqueue start bit: err: %i.", err); - goto cleanup_write; - } - - err = i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_WRITE, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_write_bytes; enqueue I2C bus address: err: %i.", err); - goto cleanup_write; - } - - if (!term_is_invalid_term(register_)) { - err = i2c_master_write_byte(cmd, register_address, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_write_bytes; enqueue register address: err: %i.", err); - goto cleanup_write; + size_t write_size = (has_register ? 1 : 0) + data_len; + uint8_t *write_buf = NULL; + if (write_size > 0) { + write_buf = malloc(write_size); + if (IS_NULL_PTR(write_buf)) { + if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { + return OUT_OF_MEMORY_ATOM; + } + return create_error_tuple(ctx, esp_err_to_term(ctx->global, ESP_ERR_NO_MEM)); } + size_t offset = 0; + if (has_register) { + write_buf[0] = register_address; + offset = 1; + } + memcpy(write_buf + offset, buf, data_len); } - err = i2c_master_write(cmd, buf, data_len, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_write_bytes; enqueue write data: err: %i.", err); - goto cleanup_write; - } - - err = i2c_master_stop(cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_write_bytes; enqueue stop bit: err: %i.", err); - goto cleanup_write; - } - - err = i2c_master_cmd_begin(rsrc_obj->i2c_num, cmd, MS_TO_TICKS(rsrc_obj->send_timeout_ms)); + esp_err_t err = i2c_bus_manager_transmit(rsrc_obj->bus_handle, addr, rsrc_obj->clock_speed_hz, + write_buf, write_size, rsrc_obj->xfer_timeout_ms); + free(write_buf); -cleanup_write: - i2c_cmd_link_delete(cmd); if (UNLIKELY(err != ESP_OK)) { if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { return OUT_OF_MEMORY_ATOM; @@ -532,58 +499,18 @@ static term nif_i2c_read_bytes(Context *ctx, int argc, term argv[]) uint8_t *buf = (uint8_t *) term_binary_data(data); // - // Enqueue and run the I2C commands + // Perform the transaction // esp_err_t err; - i2c_cmd_handle_t cmd = i2c_cmd_link_create(); - - err = i2c_master_start(cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue start bit: err: %i.", err); - goto cleanup_read; - } - if (!term_is_invalid_term(register_)) { - err = i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_WRITE, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue write address: err: %i.", err); - goto cleanup_read; - } - err = i2c_master_write_byte(cmd, register_address, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue register address: err: %i.", err); - goto cleanup_read; - } - err = i2c_master_start(cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue repeated start: err: %i.", err); - goto cleanup_read; - } - } - - err = i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_READ, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue I2C bus address: err: %i.", err); - goto cleanup_read; - } - - err = i2c_master_read(cmd, buf, read_count, I2C_MASTER_LAST_NACK); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue read data: err: %i.", err); - goto cleanup_read; - } - - err = i2c_master_stop(cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_read_bytes; enqueue stop bit: err: %i.", err); - goto cleanup_read; + err = i2c_bus_manager_transmit_receive(rsrc_obj->bus_handle, addr, rsrc_obj->clock_speed_hz, + ®ister_address, 1, buf, read_count, rsrc_obj->xfer_timeout_ms); + } else { + err = i2c_bus_manager_receive(rsrc_obj->bus_handle, addr, rsrc_obj->clock_speed_hz, buf, + read_count, rsrc_obj->xfer_timeout_ms); } - err = i2c_master_cmd_begin(rsrc_obj->i2c_num, cmd, MS_TO_TICKS(rsrc_obj->send_timeout_ms)); - -cleanup_read: - i2c_cmd_link_delete(cmd); if (UNLIKELY(err != ESP_OK)) { if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { return OUT_OF_MEMORY_ATOM; @@ -646,35 +573,15 @@ static term nif_i2c_begin_transmission(Context *ctx, int argc, term argv[]) uint8_t addr = term_to_int32(address); // - // Initiate the I2C command + // Start accumulating the transmission; the actual bus transaction is + // deferred until nif_i2c_end_transmission/1 // - esp_err_t err; - i2c_cmd_handle_t cmd = i2c_cmd_link_create(); - - err = i2c_master_start(cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_begin_transmission; enqueue start bit: err: %i.", err); - goto cleanup_begin; - } - - err = i2c_master_write_byte(cmd, (addr << 1) | I2C_MASTER_WRITE, ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_begin_transmission; enqueue I2C bus address: err: %i.", err); - goto cleanup_begin; - } - + i2c_tx_buffer_reset(&rsrc_obj->tx_buf); + rsrc_obj->tx_address = addr; rsrc_obj->transmitting_pid = term_from_local_process_id(ctx->process_id); - rsrc_obj->cmd = cmd; return OK_ATOM; - -cleanup_begin: - i2c_cmd_link_delete(cmd); - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - return OUT_OF_MEMORY_ATOM; - } - return create_error_tuple(ctx, esp_err_to_term(ctx->global, err)); } // @@ -721,17 +628,14 @@ static term nif_i2c_enqueue_write_bytes(Context *ctx, int argc, term argv[]) const uint8_t *buf = (const uint8_t *) term_binary_data(data); size_t len = term_binary_size(data); - esp_err_t err; - for (size_t i = 0; i < len; ++i) { - err = i2c_master_write_byte(rsrc_obj->cmd, buf[i], ACK_ENABLE); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_enqueue_write_bytes; enqueue i2c_master_write_byte: err: %i.", err); - reset_i2c_transmission_state(rsrc_obj); - if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { - return OUT_OF_MEMORY_ATOM; - } - return create_error_tuple(ctx, esp_err_to_term(ctx->global, err)); + esp_err_t err = i2c_tx_buffer_append(&rsrc_obj->tx_buf, buf, len); + if (UNLIKELY(err != ESP_OK)) { + ESP_LOGE(TAG, "nif_enqueue_write_bytes; failed to enqueue bytes: err: %i.", err); + reset_i2c_transmission_state(rsrc_obj); + if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { + return OUT_OF_MEMORY_ATOM; } + return create_error_tuple(ctx, esp_err_to_term(ctx->global, err)); } return OK_ATOM; @@ -778,22 +682,15 @@ static term nif_i2c_end_transmission(Context *ctx, int argc, term argv[]) } // - // Write the stop bit and flush the I2C command(s) that have been written + // Flush the accumulated write buffer as a single bus transaction // - esp_err_t err; - err = i2c_master_stop(rsrc_obj->cmd); - if (UNLIKELY(err != ESP_OK)) { - ESP_LOGE(TAG, "nif_end_transmission; enqueue stop bit: err: %i.", err); - goto cleanup_end; - } + esp_err_t err = i2c_bus_manager_transmit(rsrc_obj->bus_handle, rsrc_obj->tx_address, + rsrc_obj->clock_speed_hz, rsrc_obj->tx_buf.data, rsrc_obj->tx_buf.len, + rsrc_obj->xfer_timeout_ms); - err = i2c_master_cmd_begin(rsrc_obj->i2c_num, rsrc_obj->cmd, MS_TO_TICKS(rsrc_obj->send_timeout_ms)); + reset_i2c_transmission_state(rsrc_obj); -cleanup_end: - i2c_cmd_link_delete(rsrc_obj->cmd); - rsrc_obj->cmd = NULL; - rsrc_obj->transmitting_pid = term_invalid_term(); if (UNLIKELY(err != ESP_OK)) { if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { return OUT_OF_MEMORY_ATOM; diff --git a/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h b/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h new file mode 100644 index 0000000000..dd34f7c5d6 --- /dev/null +++ b/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h @@ -0,0 +1,117 @@ +/* + * This file is part of AtomVM. + * + * Copyright 2026 The AtomVM Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later + */ + +#ifndef _I2C_BUS_MANAGER_H_ +#define _I2C_BUS_MANAGER_H_ + +#include +#include + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +// Sentinel value for `xfer_timeout_ms` meaning "wait forever". +#define I2C_BUS_MANAGER_TIMEOUT_INFINITE (-1) + +/** + * @brief Create a new I2C master bus. + * + * This is a thin wrapper around `i2c_new_master_bus()`. Unlike the legacy + * `driver/i2c.h` API, the new driver does not associate a clock speed with + * the bus itself: clock speed is a property of each device (address) added + * to the bus. AtomVM's Erlang-level API only exposes a single clock speed + * per opened `i2c()` handle, which callers should pass to each of the + * transaction functions below. + */ +esp_err_t i2c_bus_manager_open( + int i2c_port, int scl_io_num, int sda_io_num, i2c_master_bus_handle_t *out_bus_handle); + +/** + * @brief Delete a previously created I2C master bus. + */ +esp_err_t i2c_bus_manager_close(i2c_master_bus_handle_t bus_handle); + +/** + * @brief Perform a single write transaction to a given address. + * + * AtomVM's I2C API takes the target address on every call rather than at + * open time (e.g., a bus scanner probes arbitrary addresses on one open + * bus). Since the new I2C driver requires a persistent per-address device + * handle for any transaction, this function adds a transient device handle + * for the given address, performs the transaction, and removes the device + * handle again before returning. + */ +esp_err_t i2c_bus_manager_transmit(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, const uint8_t *write_buffer, size_t write_size, + int xfer_timeout_ms); + +/** + * @brief Perform a single read transaction from a given address. + * + * See `i2c_bus_manager_transmit()` for a description of the transient + * device handle strategy used here. + */ +esp_err_t i2c_bus_manager_receive(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms); + +/** + * @brief Perform a write, followed by a (repeated-start) read, from a given + * address (typically used to read from a specific device register). + * + * See `i2c_bus_manager_transmit()` for a description of the transient + * device handle strategy used here. + */ +esp_err_t i2c_bus_manager_transmit_receive(i2c_master_bus_handle_t bus_handle, uint16_t address, + uint32_t clock_speed_hz, const uint8_t *write_buffer, size_t write_size, + uint8_t *read_buffer, size_t read_size, int xfer_timeout_ms); + +/** + * @brief A simple growable byte buffer, used by both the `i2c` port driver + * and the `i2c` NIF-resource driver to accumulate bytes written across a + * `begin_transmission/write_byte(s).../end_transmission` sequence. + * + * The legacy driver deferred the actual bus transaction until + * `i2c_master_cmd_begin()` was called at the end of such a sequence (via a + * command link); the new driver has no equivalent of a command link, so + * the bytes are accumulated here instead, and sent in one + * `i2c_bus_manager_transmit()` call from `end_transmission`. + */ +struct I2CTxBuffer +{ + uint8_t *data; + size_t len; + size_t cap; +}; + +void i2c_tx_buffer_init(struct I2CTxBuffer *buf); +void i2c_tx_buffer_reset(struct I2CTxBuffer *buf); +esp_err_t i2c_tx_buffer_append(struct I2CTxBuffer *buf, const uint8_t *data, size_t len); +esp_err_t i2c_tx_buffer_append_byte(struct I2CTxBuffer *buf, uint8_t byte); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/src/platforms/esp32/components/avm_builtins/include/i2c_driver.h b/src/platforms/esp32/components/avm_builtins/include/i2c_driver.h index bb200e68f9..707bdb6549 100644 --- a/src/platforms/esp32/components/avm_builtins/include/i2c_driver.h +++ b/src/platforms/esp32/components/avm_builtins/include/i2c_driver.h @@ -21,13 +21,11 @@ #ifndef _I2C_DRIVER_H_ #define _I2C_DRIVER_H_ -#include +#include #include #include -#define ATOMVM_ESP32_I2C_OLD_API 1 - enum I2CAcquireOpts { I2CAcquireNoOpts @@ -41,10 +39,17 @@ enum I2CAcquireResult typedef enum I2CAcquireResult I2CAcquireResult; -// These functions are meant for integrating native drivers with the I2C port driver -// defined as following only when ATOMVM_ESP32_I2C_OLD_API is set -// it will be changed in future. -I2CAcquireResult i2c_driver_acquire(term i2c_port, i2c_port_t *i2c_num, GlobalContext *global); +// These functions are meant for integrating other native drivers with the +// `i2c` port driver, allowing them to share its already-open +// `i2c_master_bus_handle_t` (e.g. to add their own devices to the bus) +// instead of opening a competing bus on the same I2C peripheral. +// +// NOTE: callers must not call `i2c_master_bus_rm_device()`-style APIs on +// devices they did not add themselves, and must not delete the bus handle; +// ownership of the bus remains with the `i2c` port that created it, and is +// only released back via `i2c_driver_release()`. +I2CAcquireResult i2c_driver_acquire( + term i2c_port, i2c_master_bus_handle_t *bus_handle, GlobalContext *global); void i2c_driver_release(term i2c_port, GlobalContext *global); #endif From d424a5a344660d1ce41999b103d75b4eb8e9aa0d Mon Sep 17 00:00:00 2001 From: harmon25 Date: Wed, 1 Jul 2026 18:43:55 -0400 Subject: [PATCH 2/3] i2c: fix zero-length write regression and minor follow-ups - i2c_bus_manager_transmit: the new driver's i2c_master_transmit() rejects zero-length write buffers with ESP_ERR_INVALID_ARG, unlike the legacy driver, which happily emitted a bare START/address/STOP sequence. This broke the address-only ACK/NACK probe idiom (e.g. i2c:write_bytes(I2C, Addr, <<>>), or an empty begin_transmission/end_transmission pair), commonly used for device presence or busy-polling checks. A write_size == 0 now falls back to i2c_master_probe(), normalizing ESP_ERR_NOT_FOUND to ESP_FAIL to match the existing {error, esp_fail} contract. - i2c_resource: close_i2c_resource now always releases the pending I2CTxBuffer, even if closing the underlying bus handle fails, avoiding a leak of the buffer across the resource's lifetime. - i2cdriver_write_bytes / nif_i2c_write_bytes: only allocate a scratch buffer when a register prefix is present; otherwise the caller's data pointer is passed directly to the transmit call, avoiding an unnecessary alloc + copy on the common register-less write path. Verified by building against ESP-IDF v5.5.4 for both esp32 and esp32s3 targets (no warnings). Signed-off-by: harmon25 --- .../components/avm_builtins/i2c_bus_manager.c | 24 ++++++++++++ .../components/avm_builtins/i2c_driver.c | 30 ++++++++------- .../components/avm_builtins/i2c_resource.c | 38 +++++++++++-------- .../avm_builtins/include/i2c_bus_manager.h | 5 +++ 4 files changed, 67 insertions(+), 30 deletions(-) diff --git a/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c b/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c index 375a1b06bf..c93de76d11 100644 --- a/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c +++ b/src/platforms/esp32/components/avm_builtins/i2c_bus_manager.c @@ -96,9 +96,33 @@ static esp_err_t remove_device(i2c_master_dev_handle_t dev_handle, esp_err_t tra return (transaction_result != ESP_OK) ? transaction_result : err; } +// `i2c_master_transmit()` rejects zero-length write buffers with +// `ESP_ERR_INVALID_ARG` (unlike the legacy driver, which happily emitted a +// START/address/STOP sequence with no data bytes in between). AtomVM's i2c +// API allows this (e.g. `i2c:write_bytes(I2C, Addr, <<>>)`, or an empty +// `begin_transmission/end_transmission` pair), commonly used as a +// device-presence/ACK probe. To preserve that behavior, a zero-length +// write is instead performed as an address-only probe via +// `i2c_master_probe()`, and its result is mapped onto the same +// `{ok, error}`/`{error, esp_fail}` contract a real transmit would have +// produced: `ESP_OK` on ACK, `ESP_FAIL` on NACK (`ESP_ERR_NOT_FOUND`). +static esp_err_t probe_address( + i2c_master_bus_handle_t bus_handle, uint16_t address, int xfer_timeout_ms) +{ + esp_err_t err = i2c_master_probe(bus_handle, address, xfer_timeout_ms); + if (err == ESP_ERR_NOT_FOUND) { + return ESP_FAIL; + } + return err; +} + esp_err_t i2c_bus_manager_transmit(i2c_master_bus_handle_t bus_handle, uint16_t address, uint32_t clock_speed_hz, const uint8_t *write_buffer, size_t write_size, int xfer_timeout_ms) { + if (write_size == 0) { + return probe_address(bus_handle, address, xfer_timeout_ms); + } + i2c_master_dev_handle_t dev_handle; esp_err_t err = add_device(bus_handle, address, clock_speed_hz, &dev_handle); if (UNLIKELY(err != ESP_OK)) { diff --git a/src/platforms/esp32/components/avm_builtins/i2c_driver.c b/src/platforms/esp32/components/avm_builtins/i2c_driver.c index 82c7d8e5d8..497354bbe9 100644 --- a/src/platforms/esp32/components/avm_builtins/i2c_driver.c +++ b/src/platforms/esp32/components/avm_builtins/i2c_driver.c @@ -397,24 +397,26 @@ static term i2cdriver_write_bytes(Context *ctx, term pid, term req) has_register = true; } - size_t write_size = (has_register ? 1 : 0) + data_len; - uint8_t *write_buf = NULL; - if (write_size > 0) { - write_buf = malloc(write_size); + // Only the register-prefixed case needs a scratch buffer to make the + // register address and data contiguous; otherwise `data` can be handed + // directly to the transmit call, avoiding an extra alloc + copy. + esp_err_t result; + if (has_register) { + size_t write_size = 1 + data_len; + uint8_t *write_buf = malloc(write_size); if (IS_NULL_PTR(write_buf)) { return make_esp_err_error(ctx, ESP_ERR_NO_MEM); } - size_t offset = 0; - if (has_register) { - write_buf[0] = register_address; - offset = 1; - } - memcpy(write_buf + offset, data, data_len); - } + write_buf[0] = register_address; + memcpy(write_buf + 1, data, data_len); - esp_err_t result = i2c_bus_manager_transmit(i2c_data->bus_handle, address, - i2c_data->clock_speed_hz, write_buf, write_size, I2C_PORT_DRIVER_TIMEOUT_MS); - free(write_buf); + result = i2c_bus_manager_transmit(i2c_data->bus_handle, address, + i2c_data->clock_speed_hz, write_buf, write_size, I2C_PORT_DRIVER_TIMEOUT_MS); + free(write_buf); + } else { + result = i2c_bus_manager_transmit(i2c_data->bus_handle, address, + i2c_data->clock_speed_hz, data, data_len, I2C_PORT_DRIVER_TIMEOUT_MS); + } if (UNLIKELY(result != ESP_OK)) { ESP_LOGE(TAG, "i2cdriver_write_bytes: transaction error: result was: %s", esp_err_to_name(result)); diff --git a/src/platforms/esp32/components/avm_builtins/i2c_resource.c b/src/platforms/esp32/components/avm_builtins/i2c_resource.c index a0aaedfd99..f3fbc80c26 100644 --- a/src/platforms/esp32/components/avm_builtins/i2c_resource.c +++ b/src/platforms/esp32/components/avm_builtins/i2c_resource.c @@ -110,9 +110,13 @@ static esp_err_t close_i2c_resource(struct I2CResource *rsrc_obj) } esp_err_t err = i2c_bus_manager_close(rsrc_obj->bus_handle); + // Always release any pending tx_buf allocation, even if closing the bus + // itself failed, so that a failed close (e.g. surfaced to `i2c:close/1`) + // doesn't leak the pending transmission buffer for the lifetime of the + // resource. + reset_i2c_transmission_state(rsrc_obj); if (err == ESP_OK) { rsrc_obj->i2c_port = I2C_RESOURCE_PORT_INVALID; - reset_i2c_transmission_state(rsrc_obj); } return err; @@ -389,30 +393,32 @@ static term nif_i2c_write_bytes(Context *ctx, int argc, term argv[]) } // - // Build the write buffer (optional register prefix + data) and send it + // Build the write buffer (optional register prefix + data) and send it. + // Only the register-prefixed case needs a scratch buffer to make the + // register address and data contiguous; otherwise `buf` can be handed + // directly to the transmit call, avoiding an extra alloc + copy. // - size_t write_size = (has_register ? 1 : 0) + data_len; - uint8_t *write_buf = NULL; - if (write_size > 0) { - write_buf = malloc(write_size); + esp_err_t err; + if (has_register) { + size_t write_size = 1 + data_len; + uint8_t *write_buf = malloc(write_size); if (IS_NULL_PTR(write_buf)) { if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { return OUT_OF_MEMORY_ATOM; } return create_error_tuple(ctx, esp_err_to_term(ctx->global, ESP_ERR_NO_MEM)); } - size_t offset = 0; - if (has_register) { - write_buf[0] = register_address; - offset = 1; - } - memcpy(write_buf + offset, buf, data_len); - } + write_buf[0] = register_address; + memcpy(write_buf + 1, buf, data_len); - esp_err_t err = i2c_bus_manager_transmit(rsrc_obj->bus_handle, addr, rsrc_obj->clock_speed_hz, - write_buf, write_size, rsrc_obj->xfer_timeout_ms); - free(write_buf); + err = i2c_bus_manager_transmit(rsrc_obj->bus_handle, addr, rsrc_obj->clock_speed_hz, + write_buf, write_size, rsrc_obj->xfer_timeout_ms); + free(write_buf); + } else { + err = i2c_bus_manager_transmit(rsrc_obj->bus_handle, addr, rsrc_obj->clock_speed_hz, buf, + data_len, rsrc_obj->xfer_timeout_ms); + } if (UNLIKELY(err != ESP_OK)) { if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { diff --git a/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h b/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h index dd34f7c5d6..5570ad9f5b 100644 --- a/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h +++ b/src/platforms/esp32/components/avm_builtins/include/i2c_bus_manager.h @@ -62,6 +62,11 @@ esp_err_t i2c_bus_manager_close(i2c_master_bus_handle_t bus_handle); * handle for any transaction, this function adds a transient device handle * for the given address, performs the transaction, and removes the device * handle again before returning. + * + * If `write_size` is 0, an address-only ACK/NACK probe is performed via + * `i2c_master_probe()` instead (the underlying driver rejects zero-length + * write buffers outright), matching the legacy driver's support for + * address-only transactions (e.g. device-presence or busy-polling checks). */ esp_err_t i2c_bus_manager_transmit(i2c_master_bus_handle_t bus_handle, uint16_t address, uint32_t clock_speed_hz, const uint8_t *write_buffer, size_t write_size, From 3eb7c5cb447e3f4601c8b293c7785a6d6e09d55d Mon Sep 17 00:00:00 2001 From: harmon25 Date: Thu, 2 Jul 2026 11:15:00 -0400 Subject: [PATCH 3/3] i2c_driver: address CodeQL term-use-after-gc finding CodeQL's atomvm/term-use-after-gc check flagged make_einprogress_error: it took the transmitting pid as a 'term owner_pid' parameter, which callers populated from i2c_data->transmitting_pid *before* the function's internal memory_ensure_free() call, then used *after* it. That capture-before-GC/use-after-GC shape is exactly the hazard the check looks for, since a GC can move/invalidate a previously read boxed term. transmitting_pid is in practice always immediate (a local pid or term_invalid_term()), so this was not an exploitable bug, but the pattern is fragile and worth fixing structurally rather than suppressing. make_einprogress_error now takes the owning 'struct I2CData *' instead, and reads i2c_data->transmitting_pid directly after the GC check, removing the flagged pattern entirely. All six call sites are updated accordingly. Addresses the github-advanced-security bot review comment on PR #2349. Signed-off-by: harmon25 --- .../components/avm_builtins/i2c_driver.c | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/platforms/esp32/components/avm_builtins/i2c_driver.c b/src/platforms/esp32/components/avm_builtins/i2c_driver.c index 497354bbe9..b7ed66995e 100644 --- a/src/platforms/esp32/components/avm_builtins/i2c_driver.c +++ b/src/platforms/esp32/components/avm_builtins/i2c_driver.c @@ -190,7 +190,14 @@ static NativeHandlerResult i2c_driver_unref(Context *ctx) return NativeTerminate; } -static term make_einprogress_error(Context *ctx, term owner_pid) +// Takes the owning `struct I2CData *` rather than a pre-extracted +// `transmitting_pid` term, and reads `i2c_data->transmitting_pid` only +// after the `memory_ensure_free()` call below. Capturing the term into a +// local/parameter *before* a GC-triggering call and using it *after* is a +// use-after-GC hazard for boxed terms; deferring the field read avoids the +// pattern entirely rather than relying on the fact that `transmitting_pid` +// happens to always be an immediate (a local pid or `term_invalid_term()`). +static term make_einprogress_error(Context *ctx, struct I2CData *i2c_data) { if (UNLIKELY(memory_ensure_free(ctx, 2 * TUPLE_SIZE(2)) != MEMORY_GC_OK)) { ESP_LOGW(TAG, "Failed to allocate memory: %s:%i.\n", __FILE__, __LINE__); @@ -199,7 +206,7 @@ static term make_einprogress_error(Context *ctx, term owner_pid) term reason_tuple = term_alloc_tuple(2, &ctx->heap); term_put_tuple_element(reason_tuple, 0, globalcontext_make_atom(ctx->global, einprogress_atom)); - term_put_tuple_element(reason_tuple, 1, owner_pid); + term_put_tuple_element(reason_tuple, 1, i2c_data->transmitting_pid); return port_create_error_tuple(ctx, reason_tuple); } @@ -220,7 +227,7 @@ static term i2cdriver_begin_transmission(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != term_invalid_term())) { // another process is already transmitting ESP_LOGE(TAG, "i2cdriver_begin_transmission: Another process is already transmitting"); - return make_einprogress_error(ctx, i2c_data->transmitting_pid); + return make_einprogress_error(ctx, i2c_data); } term address_term = term_get_tuple_element(req, 1); @@ -239,7 +246,7 @@ static term i2cdriver_end_transmission(Context *ctx, term pid) if (UNLIKELY(i2c_data->transmitting_pid != pid)) { // transaction owned from a different pid ESP_LOGE(TAG, "i2cdriver_end_transmission: Another process is already transmitting"); - return make_einprogress_error(ctx, i2c_data->transmitting_pid); + return make_einprogress_error(ctx, i2c_data); } esp_err_t result = i2c_bus_manager_transmit(i2c_data->bus_handle, i2c_data->tx_address, @@ -264,7 +271,7 @@ static term i2cdriver_write_byte(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != pid)) { // transaction owned from a different pid ESP_LOGE(TAG, "i2cdriver_write_byte: Another process is already transmitting"); - return make_einprogress_error(ctx, i2c_data->transmitting_pid); + return make_einprogress_error(ctx, i2c_data); } term data_term = term_get_tuple_element(req, 1); @@ -286,7 +293,7 @@ static term i2cdriver_qwrite_bytes(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != pid)) { // transaction owned from a different pid ESP_LOGE(TAG, "i2cdriver_qwrite_bytes: Another process is already transmitting"); - return make_einprogress_error(ctx, i2c_data->transmitting_pid); + return make_einprogress_error(ctx, i2c_data); } term data_term = term_get_tuple_element(req, 1); @@ -314,7 +321,7 @@ static term i2cdriver_read_bytes(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != term_invalid_term())) { ESP_LOGE(TAG, "i2cdriver_read_bytes: Another process is already transmitting"); - return make_einprogress_error(ctx, i2c_data->transmitting_pid); + return make_einprogress_error(ctx, i2c_data); } int arity = term_get_tuple_arity(req); @@ -368,7 +375,7 @@ static term i2cdriver_write_bytes(Context *ctx, term pid, term req) if (UNLIKELY(i2c_data->transmitting_pid != term_invalid_term())) { ESP_LOGE(TAG, "i2cdriver_write_bytes: Another process is already transmitting"); - return make_einprogress_error(ctx, i2c_data->transmitting_pid); + return make_einprogress_error(ctx, i2c_data); } int arity = term_get_tuple_arity(req);