I'm using the following code to make aging offset adjustment to my DS3231 module (ZS-042 module).
It appears that the wiring and code is fine. The DS3231 seems to have the same address for aging register as the DS3232 which this code was originally meant for. So it should work for DS3231 too. At least, that is what the author of the code says.
When I open the serial monitor, if I press the adjustment buttons, I can indeed see the offset adjustment changes (as: +1, +2, +3 or -1, -2, -3 etc.) but I'm a bit confused, because the module doesn't retain the value if I cycle the power to arduino. When I re-open the serial monitor, I'm back to "0" on the adjustment value.
Also, even if I don't cycle the power, if I only close and re-open the serial monitor, again, I'm back to "0". Even without turning the power off. Is that how it is supposed to work?
The module's backup battery is fine.
What do you think is wrong?
/**
* DS3232 TCXO Calibration
*
* Kerry D. Wong
* http://www.kerrywong.com
* 7/2014
*/
#include <Wire.h>
#define DS3232_I2C_ADDRESS B01101000
const int btn1 = 0;
const int btn2 = 1;
const int pinBtn1 = 2;
const int pinBtn2 = 3;
const int BTN_PINS[] = {
pinBtn1, pinBtn2};
boolean btnPressed[] = {
false, false};
int currentOffset = 0;
void delayMilliseconds(int ms) {
for (int i = 0; i < ms; i++) {
delayMicroseconds(1000);
}
}
boolean buttonPressed(int btnIdx) {
if (digitalRead(BTN_PINS[btnIdx]) == LOW && !btnPressed[btnIdx]) {
delayMilliseconds(20);
if (digitalRead(BTN_PINS[btnIdx]) == LOW) {
btnPressed[btnIdx] = true;
return true;
}
}
return false;
}
boolean buttonRleased(int btnIdx) {
if (digitalRead(BTN_PINS[btnIdx]) == HIGH && btnPressed[btnIdx]) {
delayMilliseconds(20);
if (digitalRead(BTN_PINS[btnIdx]) == HIGH) {
btnPressed[btnIdx] = false;
return true;
}
}
return false;
}
void setControlRegisters(){
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x0E);
Wire.write(B00011100);
Wire.write(B11001000);
Wire.endTransmission();
}
void forceConversion()
{
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x0E);
Wire.write(B00111100);
Wire.endTransmission();
}
double getTemperature()
{
byte th = 0;
byte tl = 0;
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x11);
Wire.endTransmission();
Wire.requestFrom(DS3232_I2C_ADDRESS,2);
while (Wire.available()) {
th = Wire.read();
tl = Wire.read();
}
double temp = 0;
if (th>127) {
temp = th - 256 + (double) (tl >> 6)/4.0;
}
else {
temp = th + (double) (tl >> 6)/4.0;
}
return temp;
}
int getAgingOffset()
{
byte b = 0;
int r = 0;
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x10);
Wire.endTransmission();
Wire.requestFrom(DS3232_I2C_ADDRESS,1);
while (Wire.available()) {
b = Wire.read();
}
double temp = 0;
if (b>127) {
r = b - 256;
}
else {
r = b;
}
return r;
}
void setAgingOffset(int offset)
{
if (offset < 0) offset += 256;
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x10);
Wire.write(offset);
Wire.endTransmission();
}
void setupRTC3232(){
Wire.begin();
setControlRegisters();
}
void setup()
{
Serial.begin(9600);
for (int i = 0; i < 2; i++) {
pinMode(BTN_PINS[i], INPUT);
digitalWrite(BTN_PINS[i], HIGH);
}
setupRTC3232();
}
void loop()
{
if (buttonPressed(btn1)) {
currentOffset++;
if (currentOffset > 127) currentOffset = 127;
setAgingOffset(currentOffset);
Serial.println(getAgingOffset());
forceConversion();
}
else if (buttonPressed(btn2)) {
currentOffset--;
if (currentOffset < -128) currentOffset = -128;
setAgingOffset(currentOffset);
Serial.println(getAgingOffset());
forceConversion();
}
for (int i = 0; i < 2; i++) {
if (buttonRleased(i)) {
//the button is released, no action is necessary.
}
}
}
You initialise currentOffset to zero when it is declared. Surely you need to read the current offset in setup(0 and set the variable currentOffset at that point to the value read !
Edit : I would read the value from the clock each time I pressed a button and modify that value before writing it back.
stowite:
You initialise currentOffset to zero when it is declared. Surely you need to read the current offset in setup(0 and set the variable currentOffset at that point to the value read !
Edit : I would read the value from the clock each time I pressed a button and modify that value before writing it back.
Sorry, but I don't understand anything in what you said. Not too knowledgeable about coding. unfortunately. Are you suggesting the offset adjustment as it is right now doesn't work or are you saying it works, but everytime the final value is declared as 0?
I did not write this code. Just hoping to use it in my case to correct the slight ppm drift of my ds3231.
What is wrong with this code? Can anyone please help figure out?
You have battery-backed SRAM. You can store any calibration values in SRAM.
I would use another portion of SRAM to contain a magic key e.g. "Open Sesame"
Then in setup() you can read the magic key. If valid, write to the register.
If not valid, do the Calibration and write the magic key.
I have always been suspicious of a DS3231 module arriving at my front door for less than £1.
The DS3231 spec claims to be +-2 ppm 0C to 40C and the Termperature +-3C
This means +- 0.17 seconds a day or +- 1 minute a year.
My experience is that Ebay modules are about +- 0.8 seconds a day
And the Temperature sensor is +- 5C
Likewise, the cheap DS1307 modules may have good chip but out-of-spec crystal.
Someone has to buy reject chips and crystals. What did you think Ebay was for?
JohnElectronics:
Sorry, but I don't understand anything in what you said. Not too knowledgeable about coding. unfortunately. Are you suggesting the offset adjustment as it is right now doesn't work or are you saying it works, but everytime the final value is declared as 0?
I did not write this code. Just hoping to use it in my case to correct the slight ppm drift of my ds3231.
What is wrong with this code? Can anyone please help figure out?
Thanks.
Each time you start the CPU it sets the variable 'currentOffset' to zero. This means it does not reflect the offset value stored in the ds3231 so when you first detect a button has been pressed you increment or decrement the value of 'currentOffset' rather than the current value of the offset stored in the ds3231 ! You then write the value of 'currentOffset' to the current offset value in the ds3231 throwing away the previous value.
The simplest, though in my view not the best, solution to your problem is in the setup() method to initialise 'currentOffset' to the value read from the ds3231. You already have a method to read the current offset so the change is just adding one line of code.
You will have to do more than just copy other people's code. You might be able to get though your school or college course doing that but you will not have learned anything and will be useless to an employer.
stowite:
You will have to do more than just copy other people's code. You might be able to get though your school or college course doing that but you will not have learned anything and will be useless to an employer.
What school? I'm not a student. I have a completely different profession. I'm only trying to make a project which is going to be a gift for a friend of mine!
david_prentice:
I have always been suspicious of a DS3231 module arriving at my front door for less than £1.
The DS3231 spec claims to be +-2 ppm 0C to 40C and the Termperature +-3C
This means +- 0.17 seconds a day or +- 1 minute a year.
My experience is that Ebay modules are about +- 0.8 seconds a day
And the Temperature sensor is +- 5C
These are likely cheap because 1) they are purchased in fairly large quantities as a comment in the article mentions they are used in China in their utility meters so are already cheaper than list prices, and 2) Like most chips they have a shelf life and humidity storage requirements that if exceeded invalidate the whole batch, so these are likely auctioned off specifically for the hobbyist community. It's a bit like a best before date, in many cases the product is absolutely fine, but in the case of chips absorbing water they can pop when re-flowed. This would be a major problem for any manufacturer when building up a complex circuit board to then have to rework even a small percentage of them and see their yields drop so they don't take the risk. When added to small break out board to sell to hobbyists it doesn't matter too much, they just throw the entire board away if the chip pops!
I suspect it's a similar case for a lot the cheap breakout boards with this or that chip on that come out of China. Personally out of dozens and dozens of different breakout boards I've ordered from China I've not had a single problem, and even if I do they are so cheap I normally always order a spare!
Would I trust them with my life or use them for products I was then selling on, absolutely not, but for us as a community of hobbyists and makers, we've never had so much choice at so little cost.
david_prentice:
You have battery-backed SRAM. You can store any calibration values in SRAM.
I would use another portion of SRAM to contain a magic key e.g. "Open Sesame"
Then in setup() you can read the magic key. If valid, write to the register.
If not valid, do the Calibration and write the magic key.
I have always been suspicious of a DS3231 module arriving at my front door for less than £1.
The DS3231 spec claims to be +-2 ppm 0C to 40C and the Termperature +-3C
This means +- 0.17 seconds a day or +- 1 minute a year.
My experience is that Ebay modules are about +- 0.8 seconds a day
And the Temperature sensor is +- 5C
Likewise, the cheap DS1307 modules may have good chip but out-of-spec crystal.
Someone has to buy reject chips and crystals. What did you think Ebay was for?
David.
Thanks for your feedback. But I must correct you about eBay modules.
I do in fact have an eBay version of the DS3231. And it only drifts for 1 second in 6 days. I have done drift check against a reliable time reference. So, it does indeed meet the 0.17 second per day requirement which is well within 2 ppm range. The reason I'm trying to use calibration is so that perhaps I can try and make it even more accurate than that.
You seem to have wrong information about how those modules end up on eBay. But I will gladly explain that here, since I also found out about it recently. The chips that are in those cheap eBay modules, are exactly the same as those marketted and sold by Maxim Integrated and other reputable companies. But then a question rises about how come they are 10 times as cheap?
I will explain. The thing is that even those chips (that peope refer to as originals) are made in china by manufacturers. Sometimes, companies order say, 5000 chips, for example. But for the manufacturers, it costs the same amount to make 10000 pieces. so they make more than ordered and "sell" the remaining quantity for free. Which means they only charge local buyers for shipping. That is how they end up on ebay cheap! exactly the same chips! That is how communism works.
stowite:
The simplest, though in my view not the best, solution to your problem is in the setup() method to initialise 'currentOffset' to the value read from the ds3231. You already have a method to read the current offset so the change is just adding one line of code.
So, what line of code do I need please? and where in the code? Please elaborate.
stowite:
The simplest, though in my view not the best, solution to your problem is in the setup() method to initialise 'currentOffset' to the value read from the ds3231. You already have a method to read the current offset so the change is just adding one line of code.
So, what line of code do I need please? and where in the code? Can somebody actually help??
JohnElectronics:
So, what line of code do I need please? and where in the code? Can somebody actually help??
As mentioned back in Reply #1, you should read the offset value stored in the RTC device every time a button is pressed. Then increment / decrement as necessary and write it back to the RTC.
You already have everything you need to do this. Your ‘getAgingOffset()’ function reads the offset value from the hardware. Store that in a variable. In fact, you already have a variable you can use: ‘currentOffset’.
You also already have the code to increment / decrement and limit check the ‘currentOffset’ variable.
Finally, you already have a function to write it back to the hardware after modifying it: ‘setAgingOffset()’.
So, take a crack at implementing these changes. Post back (providing your updated code) with problems or additional questions.
gfvalvo:
As mentioned back in Reply #1, you should read the offset value stored in the RTC device every time a button is pressed. Then increment / decrement as necessary and write it back to the RTC.
You already have everything you need to do this. Your ‘getAgingOffset()’ function reads the offset value from the hardware. Store that in a variable. In fact, you already have a variable you can use: ‘currentOffset’.
You also already have the code to increment / decrement and limit check the ‘currentOffset’ variable.
Finally, you already have a function to write it back to the hardware after modifying it: ‘setAgingOffset()’.
So, take a crack at implementing these changes. Post back (providing your updated code) with problems or additional questions.
Here is the updated code. In the global area, I changed: "int currentOffset = 0;" to "int currentOffset;" and added "currentOffset = getAgingOffset();" to the end of the LOOP section. I don't know if I should add this to the beginning or end of LOOP section though.
/**
DS3232 TCXO Calibration
Kerry D. Wong
http://www.kerrywong.com
7/2014
*/
#include <Wire.h>
#define DS3232_I2C_ADDRESS B01101000
const int btn1 = 0;
const int btn2 = 1;
const int pinBtn1 = 2;
const int pinBtn2 = 3;
const int BTN_PINS[] = {
pinBtn1, pinBtn2
};
boolean btnPressed[] = {
false, false
};
int currentOffset;
void delayMilliseconds(int ms) {
for (int i = 0; i < ms; i++) {
delayMicroseconds(1000);
}
}
boolean buttonPressed(int btnIdx) {
if (digitalRead(BTN_PINS[btnIdx]) == LOW && !btnPressed[btnIdx]) {
delayMilliseconds(20);
if (digitalRead(BTN_PINS[btnIdx]) == LOW) {
btnPressed[btnIdx] = true;
return true;
}
}
return false;
}
boolean buttonRleased(int btnIdx) {
if (digitalRead(BTN_PINS[btnIdx]) == HIGH && btnPressed[btnIdx]) {
delayMilliseconds(20);
if (digitalRead(BTN_PINS[btnIdx]) == HIGH) {
btnPressed[btnIdx] = false;
return true;
}
}
return false;
}
void setControlRegisters() {
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x0E);
Wire.write(B00011100);
Wire.write(B11001000);
Wire.endTransmission();
}
void forceConversion()
{
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x0E);
Wire.write(B00111100);
Wire.endTransmission();
}
double getTemperature()
{
byte th = 0;
byte tl = 0;
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x11);
Wire.endTransmission();
Wire.requestFrom(DS3232_I2C_ADDRESS, 2);
while (Wire.available()) {
th = Wire.read();
tl = Wire.read();
}
double temp = 0;
if (th > 127) {
temp = th - 256 + (double) (tl >> 6) / 4.0;
}
else {
temp = th + (double) (tl >> 6) / 4.0;
}
return temp;
}
int getAgingOffset()
{
byte b = 0;
int r = 0;
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x10);
Wire.endTransmission();
Wire.requestFrom(DS3232_I2C_ADDRESS, 1);
while (Wire.available()) {
b = Wire.read();
}
double temp = 0;
if (b > 127) {
r = b - 256;
}
else {
r = b;
}
return r;
}
void setAgingOffset(int offset)
{
if (offset < 0) offset += 256;
Wire.beginTransmission(DS3232_I2C_ADDRESS);
Wire.write(0x10);
Wire.write(offset);
Wire.endTransmission();
}
void setupRTC3232() {
Wire.begin();
setControlRegisters();
}
void setup()
{
Serial.begin(9600);
for (int i = 0; i < 2; i++) {
pinMode(BTN_PINS[i], INPUT);
digitalWrite(BTN_PINS[i], HIGH);
}
setupRTC3232();
}
void loop()
{
if (buttonPressed(btn1)) {
currentOffset++;
if (currentOffset > 127) currentOffset = 127;
setAgingOffset(currentOffset);
Serial.println(getAgingOffset());
forceConversion();
}
else if (buttonPressed(btn2)) {
currentOffset--;
if (currentOffset < -128) currentOffset = -128;
setAgingOffset(currentOffset);
Serial.println(getAgingOffset());
forceConversion();
}
for (int i = 0; i < 2; i++) {
if (buttonRleased(i)) {
//the button is released, no action is necessary.
}
}
currentOffset = getAgingOffset();
}
gfvalvo:
Why? Reading it from the hardware every time guarantees it will always be in sync.
I would move the call to getAgingOffset() inside each of the ‘if’ statements. That way you won’t be reading it unless you need it.
I tried to move the "currentOffset = getAgingOffset();" inside each of "if" statements. But it didn't work. So, I moved it back to general LOOP section, and placed it right before LOOP function is closed and it works as intended.
