search

CommunityGD32Cores / ArduinoCore-GD32

Arduino core for GD32 devices, community developed, based on original GigaDevice's core
Other
86 stars 33 forks source link

The underlying i2c implementation would always block infinitely on reads #45

Closed obra closed 2 years ago

obra commented 2 years ago

The underlying i2c implementation would always block infinitely on read events if it didn't get an answer from the i2c device.

This change adds the same timeout countdown we use elsewhere in the library.

obra commented 2 years ago

We should discuss before merging this. It's specced as a blocking read in the code comments, but I can't believe it's supposed to be a "lock your program hard" blocking read.

obra commented 2 years ago

Ah, no. The upstream source function was specced as a blocking read. https://github.com/ARMmbed/mbed-os/blob/4587080dbb470294f623054db2a272f1570be327/targets/TARGET_GigaDevice/TARGET_GD32F30X/i2c_api.c#L267

maxgerhardt commented 2 years ago

It's good that there's a timeout now, eventually we should move to specifying timeouts as microseconds because that's what e.g. the AVR Wire library is doing

https://github.com/arduino/ArduinoCore-avr/blob/master/libraries/Wire/src/utility/twi.c#L216-L223

The Wire library is riddled with simply decrement-counter CPU delays which are extremely CPU clock sensitive.