Repository navigation
I2C frequency & dev->mutex #19
Description
Hello, I am creating this issue which actually integrates two issues related to i2cdev and sensor libraries. I am working on a project which uses two sensors to measure temperature, humidity and pressure (AHT10 and BMP180). There is support for these components on the esp-idf-lib repository, so I installed them on my ESP-IDF project using the ESP-IDF Component Manager.
I created a device descriptor for each sensor and initialized them calling aht_init_desc() and bmp180_init_desc() functions. These functions receive several parameters, namely the device descriptor itself (dev), the port number and the GPIO pins for SDA and SCL lines. The arguments passed to the function are assigned to the members of the device descriptor. Nevertheless, these functions don't receive I2C frequency as a parameter; instead, a frequency of 400 kHz is assigned by default to i2c_dev.cfg.master.clk_speed. I have realized that in some cases it would be better to set a custom frequency instead of a default one; for example, when using cables that might be quite long for I2C (3-4 m), it is recommendable to lower I2C bus frequency so that rise time is much lower than SCL period. I tried decreasing I2C frequency to 50 kHz, manually assigning the frequency to i2c_dev.cfg.master.clk_speed member. However, I don't consider this a good programming practice, as the intern variables of a library should not be set externally by user. That's why I suggest adding a parameter to set I2C frequency to aht_init_desc() and bmp180_init_desc() functions, as follows:
esp_err_t bmp180_init_desc(bmp180_dev_t *dev, i2c_port_t port, gpio_num_t sda_gpio, gpio_num_t scl_gpio, uint32_t clk_speed)
esp_err_t aht_init_desc(aht_t *dev, uint8_t addr, i2c_port_t port, gpio_num_t sda_gpio, gpio_num_t scl_gpio, uint32_t clk_speed)
The same can be done with the init_desc functions of the other sensor libraries.
On the other hand, I have found another issue related to the device mutex. In i2c_dev_create_mutex() function (inside i2cdev) there is a conditional statement that checks the state of dev-mutex. That implicit condition is true when mutex is not empty; if so, a log "Device mutex already exists" is printed and the function returns ESP_OK without creating a new device mutex. This is ok when the function is called after the device mutex has been created; but when it is called for the first time (at the end of init_desc() functions) the mutex is not created and so the functions for read/write operations throw errors. The solution I found for this problem was to set to zero all the members of the device descriptor, calling memset inside the init_desc() functions before setting the members of the structure. In this way, the condition dev->mutex assessed in i2c_dev_create_mutex() is false, the conditional statement is skipped and the mutex is created.
The final code that solves the two issues explained above is the following:
esp_err_t bmp180_init_desc(bmp180_dev_t *dev, i2c_port_t port, gpio_num_t sda_gpio, gpio_num_t scl_gpio, uint32_t clk_speed)
{
CHECK_ARG(dev);
memset(dev, 0, sizeof(dev));
dev->i2c_dev.port = port;
dev->i2c_dev.addr = BMP180_DEVICE_ADDRESS;
dev->i2c_dev.cfg.sda_io_num = sda_gpio;
dev->i2c_dev.cfg.scl_io_num = scl_gpio;
dev->i2c_dev.cfg.master.clk_speed = clk_speed;
return i2c_dev_create_mutex(&dev->i2c_dev);
}
I hope you can analyse the issues and correct them if you consider necessary. Thank you very much.
Activity
UncleRus commentedon Jul 23, 2026
Hello!
Thank you for taking the time to look into this piece of code and for sharing your concerns. Your observation is perfectly valid from a formal safety standpoint, but I’d like to explain why the current behavior is deliberate and why the proposed changes, in my opinion, are not the best way forward.
1. Zeroing the descriptor structure
You suggest zeroing the memory inside
bmp180_init_desc(). That would indeed ensure a clean state without any extra effort from the caller. However, within our ecosystem, there is a well‑established convention: all*_init_desc()functions (for all drivers) do not zero the provided structure. That responsibility is left entirely to the calling code. This approach gives the caller explicit control over initialization and avoids hidden side effects (for example, if the structure already contains important fields that should not be cleared).To confirm this, you can check the official esp-idf-lib examples – everywhere you will see an explicit
memset()or initialization with{}before callingsomething_init_desc(). Therefore, the current implementation follows the general pattern, and changing it for just this one driver would break consistency.2. Adding a clock‑speed parameter
You propose adding an extra parameter to the function signature to set the I2C clock frequency directly. That would certainly make the interface more self‑contained, but unfortunately it would break backward compatibility for all existing projects that use the current version of the library. Changing a public API is always a last resort, and here it is not justified because there is a standard way to change the frequency after initialising the descriptor.
According to our FAQ, you simply do:
This is a common practice for all drivers based on
i2c_dev_t, and it requires no recompilation or code changes.I fully understand your wish to make initialisation safer and more convenient, but in this case the proposed modifications would cause more trouble than benefit, breaking both the established style and compatibility. If you have additional arguments or real‑world scenarios where the current behaviour is truly problematic, I’m happy to discuss them in more detail.
Once again, thank you for your interest in the project and for providing constructive feedback!