From 5c2ee7ceb1d6cf876d3663f62d67399d4974ce3c Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 17 Sep 2020 21:16:56 +0200 Subject: [PATCH 1/5] Wire: Define WIRE_HAS_TIMEOUT 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 #42. --- libraries/Wire/src/Wire.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/Wire/src/Wire.h b/libraries/Wire/src/Wire.h index e70d72edb..c946b0486 100644 --- a/libraries/Wire/src/Wire.h +++ b/libraries/Wire/src/Wire.h @@ -30,6 +30,9 @@ // WIRE_HAS_END means Wire has end() #define WIRE_HAS_END 1 +// WIRE_HAS_TIMEOUT means Wire has setWireTimeout(), getWireTimeoutFlag +// and clearWireTimeoutFlag() +#define WIRE_HAS_TIMEOUT 1 class TwoWire : public Stream { From d739b3f6f7c7c9e7f0c2dc11cc91057fef164b3d Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 17 Sep 2020 21:33:44 +0200 Subject: [PATCH 2/5] Wire: Expose default timeout settings in Wire.h Previously, these were implicit in the default values of some global variables. Now, the default timeout is defined in twi.h and exposed through Wire.h, which: - Makes it easier to change later - Allows sketches to detect the default timeout value Note that this definition is split between twi.h and Wire.h to ensure that the default values are available as compile-time constants in twi.c (which allows for more efficient initialization than e.g. writing these values from the Wire constructor or from the begin() method, where the latter has the extra downside of potentially overwriting values previously set with setWireTimeout() if begin() is called again). Additionally, since twi does not currently depend on Wire.h, defining these values in Wire.h and using them in twi.c was not feasible without breaking adding this extra dependency, so instead this commit defines the values in twi and then re-exposes them in Wire.h (with the intention of sketches referring to the Wire.h versions, not the twi.h versions). See #362 for additional discussion. --- libraries/Wire/src/Wire.h | 5 +++++ libraries/Wire/src/utility/twi.c | 4 ++-- libraries/Wire/src/utility/twi.h | 8 ++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/libraries/Wire/src/Wire.h b/libraries/Wire/src/Wire.h index c946b0486..008a68f68 100644 --- a/libraries/Wire/src/Wire.h +++ b/libraries/Wire/src/Wire.h @@ -25,6 +25,7 @@ #include #include "Stream.h" +#include "utility/twi.h" #define BUFFER_LENGTH 32 @@ -34,6 +35,10 @@ // and clearWireTimeoutFlag() #define WIRE_HAS_TIMEOUT 1 +// When not configured, these settings are used for the timeout +#define WIRE_DEFAULT_TIMEOUT TWI_DEFAULT_TIMEOUT +#define WIRE_DEFAULT_RESET_ON_TIMEOUT TWI_DEFAULT_RESET_ON_TIMEOUT + class TwoWire : public Stream { private: diff --git a/libraries/Wire/src/utility/twi.c b/libraries/Wire/src/utility/twi.c index d223760d8..05c414132 100644 --- a/libraries/Wire/src/utility/twi.c +++ b/libraries/Wire/src/utility/twi.c @@ -51,9 +51,9 @@ static volatile uint8_t twi_inRepStart; // in the middle of a repeated start // and twi_do_reset_on_timeout could become true // to conform to the SMBus standard // http://smbus.org/specs/SMBus_3_1_20180319.pdf -static volatile uint32_t twi_timeout_us = 0ul; +static volatile uint32_t twi_timeout_us = TWI_DEFAULT_TIMEOUT; static volatile bool twi_timed_out_flag = false; // a timeout has been seen -static volatile bool twi_do_reset_on_timeout = false; // reset the TWI registers on timeout +static volatile bool twi_do_reset_on_timeout = TWI_DEFAULT_RESET_ON_TIMEOUT; // reset the TWI registers on timeout static void (*twi_onSlaveTransmit)(void); static void (*twi_onSlaveReceive)(uint8_t*, int); diff --git a/libraries/Wire/src/utility/twi.h b/libraries/Wire/src/utility/twi.h index 85b983794..9391b0fcc 100644 --- a/libraries/Wire/src/utility/twi.h +++ b/libraries/Wire/src/utility/twi.h @@ -34,6 +34,14 @@ #define TWI_BUFFER_LENGTH 32 #endif + #ifndef TWI_DEFAULT_TIMEOUT + #define TWI_DEFAULT_TIMEOUT 0 + #endif + + #ifndef TWI_DEFAULT_RESET_ON_TIMEOUT + #define TWI_DEFAULT_RESET_ON_TIMEOUT 0 + #endif + #define TWI_READY 0 #define TWI_MRX 1 #define TWI_MTX 2 From 80c25548348b19600d2a7959e02cd6285a4c75f9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sat, 19 Sep 2020 19:57:31 +0200 Subject: [PATCH 3/5] Wire: Rename reset_with_timeout parameter to reset_on_timeout This parameter was recently added, but the name was not ideal. Renaming it would make it more clear, and also more consistent with the variable it eventually sets and the comments. --- libraries/Wire/src/Wire.cpp | 6 +++--- libraries/Wire/src/Wire.h | 2 +- libraries/Wire/src/utility/twi.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/Wire/src/Wire.cpp b/libraries/Wire/src/Wire.cpp index c407776e7..5d1375a40 100644 --- a/libraries/Wire/src/Wire.cpp +++ b/libraries/Wire/src/Wire.cpp @@ -109,12 +109,12 @@ void TwoWire::setClock(uint32_t clock) * the default. * * @param timeout a timeout value in microseconds, if zero then timeout checking is disabled - * @param reset_with_timeout if true then TWI interface will be automatically reset on timeout + * @param reset_on_timeout if true then TWI interface will be automatically reset on timeout * if false then TWI interface will not be reset on timeout */ -void TwoWire::setWireTimeout(uint32_t timeout, bool reset_with_timeout){ - twi_setTimeoutInMicros(timeout, reset_with_timeout); +void TwoWire::setWireTimeout(uint32_t timeout, bool reset_on_timeout){ + twi_setTimeoutInMicros(timeout, reset_on_timeout); } /*** diff --git a/libraries/Wire/src/Wire.h b/libraries/Wire/src/Wire.h index 008a68f68..6b6f9cdfd 100644 --- a/libraries/Wire/src/Wire.h +++ b/libraries/Wire/src/Wire.h @@ -63,7 +63,7 @@ class TwoWire : public Stream void begin(int); void end(); void setClock(uint32_t); - void setWireTimeout(uint32_t timeout = 25000, bool reset_with_timeout = false); + void setWireTimeout(uint32_t timeout = 25000, bool reset_on_timeout = false); bool getWireTimeoutFlag(void); void clearWireTimeoutFlag(void); void beginTransmission(uint8_t); diff --git a/libraries/Wire/src/utility/twi.c b/libraries/Wire/src/utility/twi.c index 05c414132..b48afc7a1 100644 --- a/libraries/Wire/src/utility/twi.c +++ b/libraries/Wire/src/utility/twi.c @@ -451,13 +451,13 @@ void twi_releaseBus(void) * Function twi_setTimeoutInMicros * Desc set a timeout for while loops that twi might get stuck in * Input timeout value in microseconds (0 means never time out) - * Input reset_with_timeout: true causes timeout events to reset twi + * Input reset_on_timeout: true causes timeout events to reset twi * Output none */ -void twi_setTimeoutInMicros(uint32_t timeout, bool reset_with_timeout){ +void twi_setTimeoutInMicros(uint32_t timeout, bool reset_on_timeout){ twi_timed_out_flag = false; twi_timeout_us = timeout; - twi_do_reset_on_timeout = reset_with_timeout; + twi_do_reset_on_timeout = reset_on_timeout; } /* From 9415e0fc7640a7a481672f90e77085a99fc7a9ff Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sat, 19 Sep 2020 20:11:55 +0200 Subject: [PATCH 4/5] Wire: Remove empty line in comment --- libraries/Wire/src/Wire.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/Wire/src/Wire.cpp b/libraries/Wire/src/Wire.cpp index 5d1375a40..ff33d2032 100644 --- a/libraries/Wire/src/Wire.cpp +++ b/libraries/Wire/src/Wire.cpp @@ -111,7 +111,6 @@ void TwoWire::setClock(uint32_t clock) * @param timeout a timeout value in microseconds, if zero then timeout checking is disabled * @param reset_on_timeout if true then TWI interface will be automatically reset on timeout * if false then TWI interface will not be reset on timeout - */ void TwoWire::setWireTimeout(uint32_t timeout, bool reset_on_timeout){ twi_setTimeoutInMicros(timeout, reset_on_timeout); From fd0b3ee2177d5d608bfab61678a0876d5fca1796 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sat, 26 Sep 2020 20:39:13 +0200 Subject: [PATCH 5/5] Wire: Fix method name in comment This method was renamed shortly before merging this code and it seems some comments were note updated. Fixes: #353 --- libraries/Wire/src/Wire.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/Wire/src/Wire.cpp b/libraries/Wire/src/Wire.cpp index ff33d2032..92e957675 100644 --- a/libraries/Wire/src/Wire.cpp +++ b/libraries/Wire/src/Wire.cpp @@ -97,7 +97,7 @@ void TwoWire::setClock(uint32_t clock) * master that has claimed the bus). * * When a timeout is triggered, a flag is set that can be queried with `getWireTimeoutFlag()` and is cleared - * when `clearWireTimeoutFlag()` or `setWireTimeoutUs()` is called. + * when `clearWireTimeoutFlag()` or `setWireTimeout()` is called. * * Note that this timeout can also trigger while waiting for clock stretching or waiting for a second master * to complete its transaction. So make sure to adapt the timeout to accomodate for those cases if needed. @@ -105,7 +105,7 @@ void TwoWire::setClock(uint32_t clock) * but (much) shorter values will usually also work. * * In the future, a timeout will be enabled by default, so if you require the timeout to be disabled, it is - * recommended you disable it by default using `setWireTimeoutUs(0)`, even though that is currently + * recommended you disable it by default using `setWireTimeout(0)`, even though that is currently * the default. * * @param timeout a timeout value in microseconds, if zero then timeout checking is disabled