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

Wire: Enable a timeout by default

Open matthijskooijman opened this issue 4 years ago • 4 comments

When no timeout is explicitly configured, Wire now uses a timeout of 25ms, resetting the Wire hardware if a timeout occurs.

The timeout length matches the timeout you get when you call Wire.setWireTimeout() without arguments, and is loosely based on the SMBus timeout and maximum clock stretching time (though it works quite differently) and is rather long. In practice, this means that even if another master is on the bus, or slaves are using significant clock stretching, the timeout will probably not trigger unless there really is a lockup.

This also enables a reset of the Wire hardware on a timeout (unlike Wire.setWireTimeout() without arguments), under the assumption that if a sketch has not set up timeout settings explicitly, it probably just wants things to keep running as well as possible and resetting allows recovering from most transient lockups.

See #42 for earlier discussion of this.

As discussed before, this is not intended to be merged directly, but only after the timeout feature has been available for a while. However, having this PR might help not forgetting this later, and serves as a place for discussing the exact values of the timeout already.

Note this PR also includes the commits from #362, but those should be ignored here. Only the last commit is relevant here.

matthijskooijman avatar Sep 17 '20 20:09 matthijskooijman

I'm in favor of making 25ms the default timeout. Though I don't believe this is conformant to NXP's I2C spec, it does seem to conform to the SMBus spec1.

greyltc avatar Jan 09 '21 11:01 greyltc

Though on second thought we should probably make sure we understand the throughput implications for turning this on by default. I'll try to put some measurements together.

greyltc avatar Jan 11 '21 15:01 greyltc

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 09 '21 15:04 CLAassistant

It has been nearly two years since the timeout feature was merged. I think it could be time to enable it by default?

matthijskooijman avatar Apr 13 '22 07:04 matthijskooijman