search

sleemanj / DS3231_Simple

An Arduino Library for EASY communication with DS3231 I2C RTC Clock and Atmel AT24C32 I2C EEPROM commonly found on the same board. Implements setting, getting the time/date, setting, checking and clearing alarms, and dead-easy circular-buffered logging of data with timestamp.
MIT License
82 stars 25 forks source link

Wire.requestFrom() should not be followed by Wire.endTransmission(). #11

Closed Koepel closed 6 years ago

Koepel commented 6 years ago

In the file "DS3231_Simple.cpp", in the function "readEEPROMByte", there is Wire.endTransmission() after the Wire.requestFrom(). That Wire.endTransmission() is currently in the line 125 and can be removed.

The Wire.endTransmission() should only be used when writing data. It does not just send a stop, it does not just finish something, it can not correct something that went wrong. It should only be used when writing data and that is all it is supposed to do.

sleemanj commented 6 years ago

On this occasion you are not correct.

To quote from the datasheet ( http://www.switchdoc.com/wp-content/uploads/2015/01/doc0336.pdf )

STOP CONDITION: A low-to-high transition of SDA with SCL high is a stop condition. After a read sequence, the stop command will place the EEPROM in a standby power mode (refer to Start and Stop Definition timing diagram)

RANDOM READ: A random read requires a “dummy” byte write sequence to load in the data word address. Once the device address word and data word address are clocked in and acknowledged by the EEPROM, the microcontroller must generate another start condition

And further figure 5 of the datasheet which illustrates the above.

You can see in the code that on line 115 endTransmission is used to send the repeated start after the dummy write, by setting the false parameter it causes Wire library to NOT send the STOP but to just restart, in accordance with the datasheet.

On line 125 the endTransmission is called (with default parameter true) to now send the STOP after the read is complete, again in accordance with the datasheet.

Koepel commented 6 years ago

This is about the way the Wire library is implemented. The Wire.endTransmission() on its own can not be used to stop or not stop something. That is not what it does.

You did not specify the 'stop' parameter for the Wire.requestFrom(), therefor the Wire.requestFrom() will do that stop condition by default.

I say it again: "The Wire.endTransmission() should only be used when writing data. It does not just send a stop, it does not just finish something, it can not correct something that went wrong. It should only be used when writing data and that is all it is supposed to do."

Can you read about the Arduino Wire library ? You have closed this issue, but it would be not correct to keep the wrong use of the Arduino Wire library in the code.

Koepel commented 6 years ago

Hi, are you willing to look into it and adjust your code to make it according to the datasheet ? I have made a wiki to explain the usage of the Wire library: Common-mistakes#2

Koepel commented 5 years ago

The wrong Wire.endTransmission() is still there. At the moment at line 124. You may remove that line.