ArduinoCore-renesas icon indicating copy to clipboard operation
ArduinoCore-renesas copied to clipboard

Add timeout handling for I2C in Wire

Open HanzHager opened this issue 2 months ago • 9 comments

Adding timeout handling for I2C in Wire according to previously defined solution on other platforms Also reduced the default timeout from 1000ms till 25ms Also created src directory as well as adding keywords.txt and library.properties so that the "Wire" part of the repository can be handled as a separate library

HanzHager avatar Oct 08 '25 16:10 HanzHager

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Oct 08 '25 16:10 CLAassistant

This should solve the issue #492

HanzHager avatar Oct 08 '25 16:10 HanzHager

The actions to reset the hardware in handleTimeout() is not well tested. It would be great if someone who knows better can take a look at it to see if it makes sense and if there should be changes.

HanzHager avatar Oct 08 '25 16:10 HanzHager

Fixed a bug in timeout handling in read_from() and write_to() and made the commit cleaner

HanzHager avatar Oct 09 '25 17:10 HanzHager

After some more testing I conclude that I need even more testing to make sure it works as intended. I think that in the current solution, the master may not be able to correctly handle when a device on the bus is stuck in clock stretching. This should most likely be fixed before ever releasing this pull request.

HanzHager avatar Oct 11 '25 15:10 HanzHager

Thanks for your pull request, and for taking the time to provide an update on the situation @HanzHager!

I see that one of Arduino's developers also submitted a similar pull request: https://github.com/arduino/ArduinoCore-renesas/pull/494

It would be great if you would review that prior pull request and help us to understand whether your PR provides any advancements when compared to the other, and whether the other might also suffer from any of the potential problems you have identified in testing of this PR.

per1234 avatar Oct 11 '25 15:10 per1234

Thanks for your pull request, and for taking the time to provide an update on the situation @HanzHager!

I see that one of Arduino's developers also submitted a similar pull request: #494

It would be great if you would review that prior pull request and help us to understand whether your PR provides any advancements when compared to the other, and whether the other might also suffer from any of the potential problems you have identified in testing of this PR.

I looked at this PR and it looks like it is using setTimeout() instead of setWireTimeout() for the implementation. There is already a defined way to handle setting timeout for "Wire" and this includes a function called setWireTimeout() as well as other functions described in https://docs.arduino.cc/language-reference/en/functions/communication/wire/setWireTimeout/

I have tried to make a complete implementation that harmonizes best with this and the AVR implementation. Unfortunately, I am not fully confident that I am handling the Renesas api properly and I have realized that there needs to be some considerations for timeout situations where some device on the bus is keeping the SCL line low as result of clock stretching or some fault. So, I guess I need more testing and potentially some help in reviewing and making additional testing my solution, before it is deployed.

I am quite new in making contributions like this, so I probably need quite some guidance to get it right.

HanzHager avatar Oct 11 '25 17:10 HanzHager

I just noticed that https://github.com/arduino/ArduinoCore-renesas/pull/494 have renamed the setTimeout() to setWireTimeout(). The function sets timeout in milliseconds, but the referred implementation https://docs.arduino.cc/language-reference/en/functions/communication/wire/setWireTimeout/ are using microseconds as well as a boolean second parameter that tells the implementation to reset the bus on timeout or not. So, even if https://github.com/arduino/ArduinoCore-renesas/pull/494 is small and simple solution, it is not compatible with other implementations and the reference documentation.

HanzHager avatar Oct 11 '25 17:10 HanzHager

I have now updated with a new better tested version, but more testing and review would be great I order to make testing easier, I have prepared a standalone library that can be downloaded and easily used in the Arduino IDE. It can be found at https://github.com/HanzHager/Wire (same identical files Wire.cpp and Wire.h)

HanzHager avatar Oct 13 '25 02:10 HanzHager