Wire: Enable a timeout by default #363
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In commit deea929 (Introduce non compulsory Wire timeout), some timeout-related functions were added. To allow writing portable sketches, it is important for those sketch to know whether they are using a Wire library version that is new enough to offer these functions without having to rely on version number checking (since other Arduino cores have their own versioning schemes). This adds a WIRE_HAS_TIMEOUT macro, similar to the existing WIRE_HAS_END macro, to facilitate that. This relates to arduino#42.
Previously, these were implicit in the default values of some global variables. Now, the default timeout is defined in Wire.h, which: - Makes it easier to change later - Allows sketches to detect the default timeout value
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 arduino#42 for earlier discussion of this.
|
I'm in favor of making 25ms the default timeout. |
|
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. |
|
|
|
It has been nearly two years since the timeout feature was merged. I think it could be time to enable it by default? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 theSMBus 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 ifa 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.