Skip to content

I2c common hal write read #5958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 1, 2022
Merged

Conversation

dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented Feb 1, 2022

I believe this fixes all these ESP32-S2 I2C issues:

As suggested by @tannewt in #5680 (comment), this PR provides a new common_hal_busio_i2c_write_read() operation. shared-bindings/busio/I2C.c no longer calls write and read separately itself, but calls the combined routine.

  • On ESP32-S2, the common-hal I2C routines use the new ESP-IDF i2c_master_device...() wrapper routines, which set things up properly for I2C transactions.
  • On other ports, common-hal/busio/I2C.c simply calls the old write and read routines, and do not send a stop bit on combined write/read operations (as was already done previously).
  • The I2C common-hal I2C API no longer allows you to specify the value of the stop bit. Proper use of the stop bit is handled inside each implementation.
  • IS31FL3741.c is revised to use the new API.
  • Native adafruit_bus_device now puts a 0x prefix on the hex address it prints if it can't find the device. The Python library already does this.

Tested on all ports. Tested with these I2C devices, except where noted: BNO055, DS3231, LC709203F, MSA301 (all at once, in a STEMMA/QT string).

atmel-samd SAMD21: Metro M0: due to limited RAM, devices tested separately, and not BNO055
atmel-samd SAMD51: Metro M4
broadcom: Pi Zero 2W
cxd56: Spresense
espressif: Metro ESP32-S2
mimxrt10xx: Teensy4.1
nrf: Feather nRF52840. IS31FL3741 (LED glasses) also tested on this board.
raspberrypi: Feather RP2040
stm: Feather F405

I did not yet change bitbangio to do anything combined. It could be fixed for consistency later, but it's not necessary right now.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this! It looks good. Nice to be rid of the stop param.

@tannewt tannewt merged commit 2964e96 into adafruit:main Feb 1, 2022
@dhalbert dhalbert deleted the i2c-common-hal-write-read branch February 1, 2022 19:11
@prplz
Copy link

prplz commented Feb 1, 2022

The reduced timeout in espressif common_hal_busio_i2c_probe doesn't work. From what I can tell, the esp-idf won't do a timeout lower than 1s: https://github.com/adafruit/esp-idf/blob/f30a865fd1a44d880b909b84112f74741412c2ce/components/driver/i2c.c#L1168.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants