Review 2.1.3: broken legacy build, bus/device lifecycle, argument validation #22

Problems found in i2cdev 2.1.3 (2c88fba) that are fixed by the pull request referencing this issue. Checked against ESP-IDF v6.1 sources and on an ESP32 board (BMP180 + HD44780 via PCF8574 on one bus).

Legacy build

1. i2cdev_legacy.c includes <driver/i2c.h>, but driver is missing from the component requirements: CONFIG_I2CDEV_USE_LEGACY_DRIVER=y fails with fatal error: driver/i2c.h: No such file or directory. The requirements are evaluated on the first CMake pass, before sdkconfig exists, so a list built from CONFIG_I2CDEV_USE_LEGACY_DRIVER never contains it.
2. i2cdev_get_shared_handle() is declared in i2cdev.h for both implementations but defined only in i2cdev.c, so the legacy build fails to link.

Bus and device lifecycle

3. i2cdev_init() keeps a static bool initialized latch that i2cdev_done() never clears. After done() + init() the function returns ESP_OK without creating anything and every following call fails with ESP_ERR_INVALID_STATE.
4. Teardown errors drop resources. If i2c_del_master_bus() fails (for example ESP_ERR_INVALID_STATE because devices are still attached), i2c_dev_delete_mutex() clears bus_handle and installed anyway: the bus stays occupied inside the IDF driver, i2cdev loses it, and no later i2c_new_master_bus() succeeds - the port is dead until reboot. Same for a failed i2c_master_bus_rm_device(): the handle is lost for good.
5. i2cdev_done() deletes the port mutex even when it failed to take it, i.e. while another task holds it.
6. The retry loop removed the device handle on ESP_ERR_INVALID_ARG / ESP_ERR_INVALID_STATE without the port mutex, without freeing the device slot and without keeping the handle when the removal failed. The trigger is wrong as well: ESP_ERR_INVALID_ARG from i2c_master_transmit() is a caller error, not a stale handle.
7. A device could be on the bus without being tracked: registration lived in i2c_dev_create_mutex(), its failure was only logged ("device will work but cleanup tracking disabled"), and a device created before i2cdev_init() was never registered. i2cdev_done() cannot remove such devices, so i2c_del_master_bus() fails and the bus leaks.
8. CONFIG_I2CDEV_NOLOCK=1 disabled the whole bus lifecycle instead of the per-device mutex: devices were not tracked, i2c_dev_delete_mutex() did nothing, i2cdev_done() could not delete the bus.
9. Adding a device was racy: dev->dev_handle was checked before the port mutex was taken and i2c_master_bus_add_device() does not detect duplicates. Two tasks doing the first I/O on one descriptor added the device twice: the first handle leaked and the reference count became 2.
10. The bus was deleted when the last i2cdev device was removed, although the handle can be shared with external components through i2cdev_get_shared_handle() (esp_lcd and similar). With an external device attached i2c_del_master_bus() fails and leaves the port occupied; without one the external component keeps a freed handle.
11. CONFIG_I2CDEV_MAX_DEVICES_PER_PORT (8) was a hard limit for the new driver, and exceeding it silently disabled tracking.

Argument validation

12. Negative port numbers passed the port >= I2C_NUM_MAX check and were used as indexes into internal arrays.
13. sda_pin == scl_pin was not rejected, so a zero-initialized descriptor (i2c_dev_t dev = {0}) configured GPIO0 as both SDA and SCL.
14. sda_pullup_en / scl_pullup_en were collapsed into a single enable_internal_pullup flag: enabling one line enabled both.
15. i2c_dev_check_present() returned ESP_ERR_INVALID_ARG when the port was not initialized, hiding the real reason.
16. i2c_dev_read_reg() / i2c_dev_write_reg() accepted NULL buffers and zero sizes, unlike the legacy implementation of the same API.
17. i2c_dev_read() selected the transaction from out_data alone, so a half-filled (out_data, out_size) pair produced a zero-length prefix transfer and four retries.
18. malloc() / free() were used without #include <stdlib.h>.
19. The header documentation did not match the implementation: return codes, retry policy, lazy bus and handle creation, side effects of check_present() / probe(), descriptor lifetime, macro requirements, cleanup order for a shared bus. Also dead CONFIG_FREERTOS_HZ / CONFIG_LOG_MAXIMUM_LEVEL fallbacks and a dead clk_flags field presented as functional.