Update README.md

megaavr/libraries/Wire/README.md

@@ -337,6 +337,12 @@ flush();
 
 ~A `flush()` method exists on all versions of Wire.h; indeed, `Stream` which it subclasses demands that - however very rarely is it actually implemented by anything other than Serial - (where there is a specific and very common reason use case) where one must clear the buffer in a specific way (by waiting for it to empty) before doing things like going to sleep or performing a software reset. Wire has rarely implemented this. In this case, it performs the functionality that the datasheets refer to as a "TWI_FLUSH" - this resets the internal state *of the master* - and at the Wire library level, the buffers are cleared; that command is apparently intended for error handling. The hardware keeps track of activity on the bus (as required by the protocol), but misbehaving devices can confuse the master - they might do something that the specification says a device will will not do, or generate electrical conditions that the master is unable to interpret in a useful way - pins not reaching the logic level thresholds, malformed data and which may in turn leave it confused as to whether the bus is available. It might be necessary to call in an attempt to recover from adverse events, which has historically been a challenge for the Wire library.~
 
+There are two issues that lead to this method being a stub (do-nothing method, required to subclass stream). First is the tension with between the hardware "flush" functionality, which according to the datasheet is intended to clear up detected bus errors, with the arduino API for HardwareSerial, the only stream where flush is generally implemented, where it instead waits for outgoing transmissions to finish. Secondly, either all, or all non-tinyAVR parts appear likely to be impacted by an erratum that renders the hardware mechanism less than useful: "Flush Non-Functional", which according to the errata sheets can in practice cause the very problem it was intended to solve, and advising us to disable and reenable TWI master instead. 
+
+So, using the hardware's flush function to clear the master's state it out. One can also argue that because of the precedent set by HardwareSerial, we should defer to it's well-defined behavior over the foggy definition of the arduino Stream API, and have it mimic that functionality. But in that case, for the I2C master, the methods to send data already block until the operation is completed, and on the slave side, the device is at the mercy of the master - we could be waiting literally an eternity (that is, it is equivalent to useing Serial.flush() when acting as a slave device in synchronous mode to transmit when the XCK pin has no clock signal - this is essentially the same situation with a different interface. Waiting for data to be clocked out, without control over that clock, is probably not the ideal API to provide either; the same behavior can be achieved with `while (slaveTransactionOpen()) {...}`, but this gives an opportunity to have a timeout after which the slave can decide that the master has fallen over and (for example) disable and reenable the interface, expecting that the next contact from the master will be a fresh start condition from a reinitialized master. 
+
+It is not hard to imagine a scenario where that would be relevant: Just imagine that you are writing application code for the master, and upload the new version of your code while it happens to be in the middle of a transaction. If you're waiting on the completion of transactions before taking some action, you probably want to make sure you are handling this case. That said,the most common action of concern here is putting the part into sleep mode, in which case, refer to the section below on Wire and Sleep - a better model is likely to simply have the device attempt to sleep during every pass through loop() if it has nothing else to do (as of 2.6.2). 
+
 ```c++
 uint8_t endTransmission(bool sendStop);
 ```
@@ -392,18 +398,28 @@ The implementation isn't identical, but the behaviour is unchanged, or is differ
 6. Slave releases clock and ack's.
 7. Master clocks in 1 while slave sends them, master ACKs each one before nacking when done and generating a stop condition.
 
-## Wire and Sleep
+## Wire and Sleep on Slave devices
 Between 2.5.0 and 2.6.1, the library was checking the BUSERR flag and aborted a transmission and the data if it there was an error. However, when a device was in Stand-by(SB) or Power-Down(PD) sleep mode, the BUSERR bit would be set on the STOP condition, rendering any received transmission like it never happened. Investigation of the issue concluded that the BUSERR detection circuitry relies on CLK_PER, which is generally disabled in sleep. Because of this it would not register the START condition and saw only the two STOP conditions on the bus - an invalid state. It was decided to not check for BUSERR at all, as often other errors would also cover the BUSERR cases.
+
 Furthermore, with SB and PD sleep, the library was prone to leave the TWI bus in an undefined state, where both lines would be kept pulled low. This happened because the TWI can wake the CPU from SB/PD only on an address match, not on a data interrupt. When code attempted to put the CPU to sleep between an address match and the data interrupt, the slave will keep the SCL line low and the TWI stays unresponsive. There was an attempt to mitigate this in 2.6.1 with `slaveTransactionOpen()` but due to the lack of control over when the master might choose to initiate communications, this did not always achieve satisfactory results, and made for ugly implementations.
+
 As of 2.6.2, the Wire library makes sure that the CPU can not enter SB/PD sleep mode between an address match interrupt and a STOP condition. This is achieved by buffering the current sleep setting and changing the sleep setting to IDLE - in this case, when the user tried to put the device to sleep  At a STOP condition, the buffered value is copied back to the register. This method was chosen, because it offers a further reduction of power consumption, as the CPU can stay in IDLE between data interrupts, as well as preventing the TWI bus of becoming unresponsive. Furthermore, the only thing the user has to do is to call `sleep_cpu()` repeatedly as long as no other jobs have to be executed. If not, the CPU will just wake up on the next data interrupt and stay awake, increasing power consumption.
 
+Note that this is applicable only to slave devices. When acting as I2C master, you cannot accidentally put the part to sleep during a transaction unless you call sleep_cpu() from within an interrupt that fires during a transaction. In that case, you deserve what you get, which is likely the device being hung until a powercycle or reset. (Seriously folks, don't put the part to sleep within an interrupt. Not just in the context of wire. Whatever you're hoping to achieve, that is not the way to do it, and you're not going to like what it does).
+
 ## Errata warning
-All modern AVRs, since the release of the first tinyAVR 0/1-series, through the AVR DB-series, have always had a silicon bug relating to the TWI pins. When the TWI peripheral takes control of the SCL and SDA, it correctly controls their being an INPUT or OUTPUT - but it fails to also take over the output value. That means that if the PORTx.OUT bit is 1 for either of the pins, it would be trying to drive the line high instead of low, and the I2C bus would be non-functional **and hardware damage - while unlikely - could result**. The errata for the 2-series and DD-series implies that this issue does not impact those parts. Nonetheless, we always clear those bits in begin(). Users should refrain from writing the I2C pins `HIGH` via `digitalWrite()` or any other means after calling `Wire.begin()`. Calling them before then is pointless too, since they will be superseded by begin(), and . *If you want to enable the internal pullups, use Wire.usePullups() - this should be done only as a debugging aid.*
+All modern AVRs, since the release of the first tinyAVR 0/1-series, through the AVR DB-series, have always had a silicon bug relating to the TWI pins. When the TWI peripheral takes control of the SCL and SDA, it correctly controls their being an INPUT or OUTPUT - but it fails to also take over the output value. That means that if the PORTx.OUT bit is 1 for either of the pins, it would be trying to drive the line high instead of low, the I2C bus would be non-functional **and hardware damage - while unlikely - could result**. The errata for the 2-series and DD-series implies that this issue does not impact those parts `*`. Nonetheless, we always clear those bits in begin(). Users should refrain from writing the I2C pins `HIGH` via `digitalWrite()` or any other means after calling `Wire.begin()`. Calling them before then is pointless too, since they will be superseded by begin(), and . *If you want to enable the internal pullups, use Wire.usePullups() - this should be done only as a debugging aid.*
+
+`*` - Presence of an issue in the errata is a reliable indicator of the bug being present, but not seeing the issue listed for that part don't mean it ain't there. Users routinely report entirely novel issues, and I am aware of two bugs that have not (yet) made it into *any* errata sheet despite being known to Microchip and impacting many - in some cases *all* modern AVRs - and which had just gone unnoticed until some Arduino user tried to make it do something obscure.
 
 ## Known incompatible devices
 For an unknown reason, the contactless IR-thermometer MLX90614 is not working out of the box with the modern AVRs. We suspect it is related to the new TWI module implementation interacting with a some aspect of the interface on the MLX90614 which does not follow the specifications. A workaround is to set the clock frequency to a value between 110-125kHz.
 
+I would also suggest testing this with `Wire.specialConfig(0,1,3)` with the default speed (which normally doesn't work). If that fixes it, see if either of those non-zero arguments can be zero'ed out. If the third argument is the key one, see how far you can lower it before it fails (note that this may depend on the physical electronic properties of the bus that determine the rise and fall times). If you do this experiment, please report your results to me so that I can update this section! 
+
+At least two of the rewrites of the clock caluculation algorithm that have been submitted, independently, by two different individuals (the second of whom didn't stop there and rewrote the whole library, slashing it's flash footprint while adding functionality) were prompted initially by users trying to get this specific part to work. So - really we're all in debt to this manufacturer, whose combination of compelling hardware saddled with a badly botched I2C interface has led to so much improvement in our Wire library.
+
 ## Why are there so many names for this protocol?
 Wire, TWI (Two Wire Interface), Two Wire, IIC, I2C-compatible, I2C, I<sup>2</sup>C... The reason for this is that I2C (and the explicitly formatted version of it, I<sup>2</sup>C) are trademarked by Phillips (now NXP) which has historically been very litigious, and would go after manufacturers of parts that didn't pay license fees. So devices that could communicate with and which were I2C in all but name proliferated. The last patent expired a while ago, but they still hold the trademarks, so other manufacturers persist in using their names. The actual terms described in the specification claim to cover to all devices that "can" communicate over I2C, and make exception only for FPGAs, where the person programming them also was supposed to get a license. It seems that the litigation wars have cooled somewhat now, though I still would be mighty careful if I was designing microcontrollers. In any event, Atmel always called it TWI, and that tradition was not lost when Microchip purchased them. The original name, I<sup>2</sup>C is a stylization of "IIC", for "Inter-Integrated-Circuit".
 
-Just don't get it confused with I2S (that's a specialized protocol for real-time transmission uncompressed digital audio - for example, it is (or was) often used in CD players between the disk reading circuitry and the DAC - as well as in professional audio equipment. I2S isn't supported by the AVR line - it's not a good match for the capabilities or intended use cases of AVR devices; it's for dedicated audio stuff, not general purpose microcontrollers). or I3C (a much faster superficially similar successor meant for much faster parts with more computational power - also not supported by AVR. While the I3C standard claims backwards compatibility, it does not support clock stretching, so an AVR acting as slave could not even reach user onRequest or onReceive code even if the I3C bus was running at only 1 MHz; it runs at 12.5 MHz).
+Just don't get it confused with I2S (that's a specialized protocol for real-time transmission uncompressed digital audio - for example, it is (or was) often used in CD players between the disk reading circuitry and the DAC - as well as in professional audio equipment. I2S isn't supported by the AVR line - it's not a good match for the capabilities or intended use cases of AVR devices; it's for dedicated audio stuff, not general purpose microcontrollers). or I3C (a much faster superficially similar successor meant for much faster parts with far more computational power - also not supported by AVR). Even calling it superficially similar is being rather generous, as the resemblance seems to be largely window dressing: there is a data line called SDA and a clock line called SCL, devices can be master or slave, and the slave devices have 7 bit addresses - and that's about the end of the resemblance. While the I3C standard claims backwards compatibility, it does not support clock stretching (which it calls "rarely used" - I'm not sure what gave them that idea, but the initiative is being spearheaded by Google, so it should surprise nobody that they are out of touch with reality). An AVR acting as slave could not even reach user onRequest or onReceive code even if the I3C bus was running at only 1 MHz. It runs at 12.5 MHz - in other words, these parts are between one and two orders of magnitude too slow to meet the timing constraints of I3C).