terrypin999 commented 6 months ago

Hobbyist here, not a programmer/developer/techie, so hope you’ll excuse this submission as an ‘Issue’.

I believe that, like me, other RTClib users get clock inaccuracy with the DS3231. Arduino Forum contributor @gfvalvo helped me to implement its adjustment after a simple addition to RTClib.h. So directly below the section

/**/ /! @brief RTC based on the DS3231 chip connected via I2C and the Wire library / /**/ (which in the current release 2.1.3 ends with empty line 400), I’ve inserted the following five lines:

// Add Aging Register Access static constexpr uint8_t agingRegister = 0x10; int8_t readAging(){ return read_register(agingRegister); } void setAging(int8_t newAgingValue) { write_register(agingRegister, newAgingValue); } }; My own sketch already includes the RTClib library and its minimal initialisation

// Initialize the rtc object rtc.begin(); to which I added the following five lines in setup(), to compensate for my clock’s current slowness.

// Using edited RTClib.h from gfvalvo to adjust AgingRegister int8_t aging = rtc.readAging(); Serial.println(aging); int8_t newAging = -50; rtc.setAging(newAging); Trial and error will be needed (before hopefully reducing drift to a couple of seconds a week at most), but I already believe it to be working satisfactorily. The method seems significantly simpler than some I’ve seen, so I’d suggest its serious consideration for a future release please.

edgar-bonet commented 6 months ago

@terrypin999: I added the methods you are requesting to my fork of RTClib, but I don't have the hardware to test them. Would you mind testing that yourself? It is in the branch “aging-offset” of my fork. The new methods are called getAgingOffset() and setAgingOffset():

constexpr int8_t calibratedAgingOffset = -50;

void setup() {
    rtc.begin();
    Serial.begin(9600);
    Serial.print("Initial aging offset: ");
    Serial.println(rtc.getAgingOffset());
    rtc.setAgingOffset(calibratedAgingOffset);
    Serial.print("New aging offset:     ");
    Serial.println(rtc.getAgingOffset());
}

If this works for you, I will submit a pull request here with the change.

terrypin999 commented 6 months ago

Ok, I’ll do that Sunday or Monday .--TerryOn 23 Mar 2024, at 21:35, Edgar Bonet @.***> wrote: @terrypin999: I added the methods you are requesting to my fork of RTClib, but I don't have the hardware to test them. Would you mind testing that yourself? It is in the branch “aging-offset” of my fork. The new methods are called getAgingOffset() and setAgingOffset(): constexpr int8_t calibratedAgingOffset = -50;

void setup() { rtc.begin(); Serial.begin(9600); Serial.print("Initial aging offset: "); Serial.println(rtc.getAgingOffset()); rtc.setAgingOffset(calibratedAgingOffset); Serial.print("New aging offset: "); Serial.println(rtc.getAgingOffset()); } If this works for you, I will submit a pull request here with the change.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

terrypin999 commented 6 months ago

Hi Edgar,

I'd like to help but I'm way out of my depth. As I said in my Issue

299: "Hobbyist here, not a programmer/developer/techie...". So github

is largely alien territory for me.

After prefixing your link with 'github.com/' I reached the page you presumably intended. https://www.dropbox.com/scl/fi/da3gq3segzsfurbkfzb35/EdgarFork.jpg?rlkey=ddwki3dhwuub9894irnbugtv6&raw=1

But without a foolproof step-by step guide I've no idea how to proceed to do what you require, sorry!

Best wishes,

Terry

"Hobbyist here, not a programmer/developer/techie..."

On Sat, 23 Mar 2024 14:35:13 -0700, you wrote:

@terrypin999: I added the methods you are requesting to my fork of RTClib, but I don't have the hardware to test them. Would you mind testing that yourself? It is in the branch aging-offset of my fork. The new methods are called getAgingOffset() and setAgingOffset():
constexpr int8_t calibratedAgingOffset = -50;

void setup() {
   rtc.begin();
   Serial.begin(9600);
   Serial.print("Initial aging offset: ");
   Serial.println(rtc.getAgingOffset());
   rtc.setAgingOffset(calibratedAgingOffset);
   Serial.print("New aging offset:     ");
   Serial.println(rtc.getAgingOffset());
}
If this works for you, I will submit a pull request here with the change.

edgar-bonet commented 6 months ago

@terrypin999: I found a tutorial with good step-by-step instructions: Install an Arduino library from GitHub. It is provided both as a Web page and as a video. Note that this tutorial offers two options:

Download a specific version of the library (from a “tag”)
download the latest source code

Please, use the second option (latest source code), as there is no tag on this specific branch.

terrypin999 commented 6 months ago

I know how to install a library from github and have now duly installed yours. But as you see from my screenshot I am unable to use it. Can you therefore tell me the full file and path of its .json file so that I can try the preferences.txt method.

https://www.dropbox.com/scl/fi/atofyz9y7ckmv8qood88g/EdgarBonet-aging.jpg?rlkey=09utumhhdlmb2tmy2ly2im9zn&raw=1

I'm working with several sketches at present, or I'd also try restarting the IDE to see if that lets me include your library. I will do so later.

Terry

On Sun, 24 Mar 2024 06:41:26 -0700, you wrote:

@terrypin999: I found a tutorial with good step-by-step instructions: Install an Arduino library from GitHub. It is provided both as a Web page and as a video. Note that this tutorial offers two options:

Download a specific version of the library (from a tag)

download the latest source code

Please, use the second option (latest source code), as there is no tag on this specific branch.

edgar-bonet commented 6 months ago

@terrypin999: I guess this may be due to both libraries having the same name, as it appears in the name field of library.properties. I suggest you move your current RTClib directory out of …\libraries, and rename RTClib-aging-offset as RTClib.

terrypin999 commented 6 months ago

Following on from my earlier reply, restarting the IDE did not help.

And when I tried the Add.ZIP Library method I got the error message "A subfolder of your sketchbook is not a valid library"

I assume you can successfully download and compile it?

On Sun, 24 Mar 2024 06:41:26 -0700, you wrote:

@terrypin999: I found a tutorial with good step-by-step instructions: Install an Arduino library from GitHub. It is provided both as a Web page and as a video. Note that this tutorial offers two options:

Download a specific version of the library (from a tag)

download the latest source code

Please, use the second option (latest source code), as there is no tag on this specific branch.

terrypin999 commented 6 months ago

But that longer name is the one you gave your ZIP, i didn’t name it.Why don’t you give your library a unique name, as there are already several confusingly called RTClib.What about the ‘json/preferences’ route, is that feasible?On 24 Mar 2024, at 16:15, Edgar Bonet @.***> wrote: @terrypin999: I guess this may be due to both libraries having the same name, as it appears in the name field of library.properties. I suggest you move your current RTClib directory out of …\libraries, and rename RTClib-aging-offset as RTClib.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

terrypin999 commented 6 months ago

I meant to also ask what needs testing? The simple edit to RTClib.h edit described is working successfully for me….On 24 Mar 2024, at 17:02, terrypin @.> wrote:But that longer name is the one you gave your ZIP, i didn’t name it.Why don’t you give your library a unique name, as there are already several confusingly called RTClib.What about the ‘json/preferences’ route, is that feasible?On 24 Mar 2024, at 16:15, Edgar Bonet @.> wrote: @terrypin999: I guess this may be due to both libraries having the same name, as it appears in the name field of library.properties. I suggest you move your current RTClib directory out of …\libraries, and rename RTClib-aging-offset as RTClib.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

edgar-bonet commented 6 months ago

@terrypin999: I did not name the ZIP file: the name was automatically generated by GitHub. You may have to change it to RTClib.zip to make it work.

Why don’t you give your library a unique name

It's not my library. :stuck_out_tongue_closed_eyes:

My fork is not a competing project: it is a tool meant for submitting pull requests in order to help make this version of the library (which is the official one) better. This is the “GitHub way” of collaborating on open source projects. See the introduction to the guide Contributing to a project.

what needs testing?

My implementation of the idea you suggested. I did not copy the “simple edit to RTClib.h” you posted. Instead, I re-implemented the same functionality in a way that is consistent with the style and coding conventions of this library.

terrypin999 commented 6 months ago

OK, understood. I think we're now close to singing from the same hymn sheet. I've renamed your 'fork' ('version' in my terminology) to RTClib as you suggested. (And as a check I've attached a comparison of the .h and .cpp files). But when I then substitute that in my fairly complex working project, (the only one I have using the DS3231), I get the error: 'class RTC_DS3231' has no member named 'readAging'

I won't paste the entire 300 or so line error report, but here's a link: https://www.dropbox.com/scl/fi/1qoy9j66y887uuwp31t5v/ErrorReport-Edgar.txt?rlkey=4hgejw9rcq7dyqlppaqtwd8r8&raw=1

If you can write a minimalised demo sketch that you think should compile with your version of RTClib then I'll try that.

Comparison of .cpp and .h: https://www.dropbox.com/scl/fi/xbju9vph423uqkcz2pfym/CompareKeyFiles.jpg?rlkey=vjl7kmelhg8mlecsifh3esukf&raw=1

====================

On Sun, 24 Mar 2024 12:41:41 -0700, you wrote:

@terrypin999: I did not name the ZIP file: the name was automatically generated by GitHub. You may have to change it to RTClib.zip to make it work.

Why dont you give your library a unique name

It's not my library. :stuck_out_tongue_closed_eyes:

My fork is not a competing project: it is a tool meant for submitting pull requests in order to help make this version of the library (which is the official one) better. This is the GitHub way of collaborating on open source projects. See the introduction to the guide Contributing to a project.

what needs testing?

My implementation of the idea you suggested. I did not copy the simple edit to RTClib.h you posted. Instead, I re-implemented the same functionality in a way that is consistent with the style and coding conventions of this library.

edgar-bonet commented 6 months ago

@terrypin999 wrote:

here's a link [to the error report]

Thanks for that report! One noteworthy thing I see there is:

Multiple libraries were found for "RTClib.h"
 Used: C:\Users\terry\Dropbox\Electronics\Arduino\SKETCHES\libraries\RTClib
 Not used: C:\Users\terry\Dropbox\Electronics\Arduino\SKETCHES\libraries\RTClib-MyEdit
 Not used: C:\Users\terry\Dropbox\Electronics\Arduino\SKETCHES\libraries\RTClib-aging-offset

So it found three alternatives, and chose the one in the folder named like the library: “RTClib”. If this is a copy of RTClib-aging-offset, it should work.

error: 'class RTC_DS3231' has no member named 'readAging'
error: 'class RTC_DS3231' has no member named 'setAging'

I named the methods getAgingOffset() and setAgingOffset(), for consistency with the current naming conventions and the datasheet.

If you can write a minimalised demo sketch that you think should compile with your version of RTClib then I'll try that.

This compiles:

#include <RTClib.h>

constexpr int8_t calibratedAgingOffset = -50;

RTC_DS3231 rtc;

void setup() {
    rtc.begin();
    Serial.begin(9600);
    Serial.print("Initial aging offset: ");
    Serial.println(rtc.getAgingOffset());
    rtc.setAgingOffset(calibratedAgingOffset);
    Serial.print("New aging offset:     ");
    Serial.println(rtc.getAgingOffset());
}

void loop(){}

Comparison of .cpp and .h: [...]

OK, as expected.

For the record, here is the diff between adafruit:master and edgar-bonet:aging-offset, as computed by GitHub itself.

terrypin999 commented 6 months ago

Yes, that was a copy of RTClib-aging-offset. Your sketch compiles here too.

Sudden family emergency an hour or so ago. Will be out for most of afternoon at least, so unsure when I can resume contact.

What's next step? You don't have a DS3231, right? I do, but it's my only one and it's in that 'finished', cased project. Can you, perhaps with help from another expert, suggest a sketch and schematic I can test that would give you confidence?

Mon 25 Mar 2024 1148, UK.

On Mon, 25 Mar 2024 04:16:37 -0700, you wrote:

@terrypin999 wrote:

here's a link [to the error report]

Thanks for that report! One noteworthy thing I see there is:
Multiple libraries were found for "RTClib.h"
 Used: C:\Users\terry\Dropbox\Electronics\Arduino\SKETCHES\libraries\RTClib
 Not used: C:\Users\terry\Dropbox\Electronics\Arduino\SKETCHES\libraries\RTClib-MyEdit
 Not used: C:\Users\terry\Dropbox\Electronics\Arduino\SKETCHES\libraries\RTClib-aging-offset
So it found three alternatives, and chose the one in the folder named like the library: RTClib. If this is a copy of RTClib-aging-offset, it should work.
error: 'class RTC_DS3231' has no member named 'readAging'
error: 'class RTC_DS3231' has no member named 'setAging'
I named the methods getAgingOffset() and setAgingOffset(), for consistency with the current naming conventions and the datasheet.

If you can write a minimalised demo sketch that you think should compile with your version of RTClib then I'll try that.

This compiles:
#include <RTClib.h>

constexpr int8_t calibratedAgingOffset = -50;

RTC_DS3231 rtc;

void setup() {
   rtc.begin();
   Serial.begin(9600);
   Serial.print("Initial aging offset: ");
   Serial.println(rtc.getAgingOffset());
   rtc.setAgingOffset(calibratedAgingOffset);
   Serial.print("New aging offset:     ");
   Serial.println(rtc.getAgingOffset());
}

void loop(){}
Comparison of .cpp and .h: [...]

OK, as expected.

For the record, here is the diff between adafruit:master and edgar-bonet:aging-offset, as computed by GitHub itself.

edgar-bonet commented 6 months ago

I indeed do not have a DS3231.

Can you, perhaps with help from another expert, suggest a sketch and schematic I can test that would give you confidence?

I just wrote the test sketch below. The required connections are:

Arduino's I2C pins to a DS3231
Arduino's USB port to a computer running either the Arduino's serial monitor or a terminal emulator

Source code (click to expand/collapse)

```c++ #include #ifdef MOCK_RTC /* Mock DS3231 for testing this sketch without the actual hardware. */ class Mock_RTC_DS3231 : public RTC_Micros { public: void begin() { RTC_Micros::begin(DateTime(F(__DATE__), F(__TIME__))); } int8_t getAgingOffset() { return aging_offset; } void setAgingOffset(int8_t offset) { aging_offset = offset; adjustDrift(-aging_offset * 200); // exaggerate drift } private: int8_t aging_offset; }; #define RTC_DS3231 Mock_RTC_DS3231 #endif // defined(MOCK_RTC) RTC_DS3231 rtc; const char help[] = "Available commands:\n\r" " h: print this help\n\r" " t: toggle printing RTC times\n\r" " f: make the RTC fast\n\r" " s: make the RTC slow\n\r" " p: print the aging offset"; void setup() { rtc.begin(); Serial.begin(9600); Serial.println("Ready."); Serial.println(help); } void loop() { /* Print RTC updates if asked to do so. */ static bool do_print_times = false; if (do_print_times) { static DateTime last_printed_time; DateTime now = rtc.now(); if (now != last_printed_time) { char buffer[] = "hh:mm:ss"; Serial.println(now.toString(buffer)); last_printed_time = now; } } /* Execute commands from the serial port. */ switch (Serial.read()) { case 'h': Serial.println(help); break; case 't': do_print_times = !do_print_times; break; case 'f': Serial.println("setting aging offset to -128 (fast)"); rtc.setAgingOffset(-128); break; case 's': Serial.println("setting aging offset to +127 (slow)"); rtc.setAgingOffset(+127); break; case 'p': Serial.print("aging offset: "); Serial.println(rtc.getAgingOffset()); } } ```

The sketch is controlled by single-character commands sent through the serial port. Send h to get a summary of the available commands. You may check that:

printing the aging offset gets what you would expect
setting the RTC to run fast makes it run fast
setting it to run slow makes it run slow

I tested the sketch using a mock RTC which I left there for the record. It will not be used unless you #define MOCK_RTC.

terrypin999 commented 6 months ago

OK, hope to be able to look at that tonight.

But yesterday I found I DID have another DS3231, as my AZ delivery last year was a pack of two. So I assume I just run your sketch with this single line commented out?

define RTC_DS3231 Mock_RTC_DS3231

====================

On Tue, 26 Mar 2024 02:37:33 -0700, you wrote:

I indeed do not have a DS3231.

Can you, perhaps with help from another expert, suggest a sketch and schematic I can test that would give you confidence?

I just wrote the test sketch below. The required connections are:

Arduino's I2C pins to a DS3231

Arduino's USB port to a computer running either the Arduino's serial monitor or a terminal emulator

Source code (click to expand/collapse)
```c++ #include #ifdef MOCK_RTC /* Mock DS3231 for testing this sketch without the actual hardware. */ class Mock_RTC_DS3231 : public RTC_Micros { public: void begin() { RTC_Micros::begin(DateTime(F(__DATE__), F(__TIME__))); } int8_t getAgingOffset() { return aging_offset; } void setAgingOffset(int8_t offset) { aging_offset = offset; adjustDrift(-aging_offset * 200); // exaggerate drift } private: int8_t aging_offset; }; #define RTC_DS3231 Mock_RTC_DS3231 #endif // defined(MOCK_RTC) RTC_DS3231 rtc; const char help[] = "Available commands:\n\r" " h: print this help\n\r" " t: toggle printing RTC times\n\r" " f: make the RTC fast\n\r" " s: make the RTC slow\n\r" " p: print the aging offset"; void setup() { rtc.begin(); Serial.begin(9600); Serial.println("Ready."); Serial.println(help); } void loop() { /* Print RTC updates if asked to do so. */ static bool do_print_times = false; if (do_print_times) { static DateTime last_printed_time; DateTime now = rtc.now(); if (now != last_printed_time) { char buffer[] = "hh:mm:ss"; Serial.println(now.toString(buffer)); last_printed_time = now; } } /* Execute commands from the serial port. */ switch (Serial.read()) { case 'h': Serial.println(help); break; case 't': do_print_times = !do_print_times; break; case 'f': Serial.println("setting aging offset to -128 (fast)"); rtc.setAgingOffset(-128); break; case 's': Serial.println("setting aging offset to +127 (slow)"); rtc.setAgingOffset(+127); break; case 'p': Serial.print("aging offset: "); Serial.println(rtc.getAgingOffset()); } } ```

The sketch is controlled by single-character commands sent through the serial port. Send h to get a summary of the available commands. You may check that:

printing the aging offset gets what you would expect

setting the RTC to run fast makes it run fast

setting it to run slow makes it run slow

I tested the sketch using a mock RTC which I left there for the record. It will not be used unless you #define MOCK_RTC.

edgar-bonet commented 6 months ago

@terrypin999: No need to comment-out anything. The line you quoted is already hidden behind #ifdef MOCK_RTC.

terrypin999 commented 6 months ago

Your sketch runs OK. My cased project uses a Nano. So I initially started testing on my spare Nano. But I could not get a port recognised, even with a basic sketch like Blink no hw. So concluded that the Nano was faulty and now using a Mega.

Running without my DS3231 in circuit, to test your special sketch gave this output:

11:52:43.488 -> Ready. 11:52:43.488 -> Available commands: 11:52:43.488 -> h: print this help 11:52:43.488 -> t: toggle printing RTC times 11:52:43.488 -> f: make the RTC fast 11:52:43.488 -> s: make the RTC slow 11:52:43.488 -> p: print the aging offset 11:52:50.848 -> 40:30:00 11:52:50.848 -> 40:30:00

With the DS3231 in circuit:

11:55:52.925 -> Ready. 11:55:52.925 -> Available commands: 11:55:52.925 -> h: print this help 11:55:52.925 -> t: toggle printing RTC times 11:55:52.925 -> f: make the RTC fast 11:55:52.925 -> s: make the RTC slow 11:55:52.925 -> p: print the aging offset 11:56:00.037 -> 00:51:12 11:56:00.506 -> 00:51:13 11:56:01.537 -> 00:51:14 11:56:02.521 -> 00:51:15 11:56:03.506 -> 00:51:16 11:56:04.537 -> 00:51:17 11:56:05.508 -> 00:51:18 11:56:06.539 -> 00:51:19 etc

And similar later tests.

I then applied aging of -128 with the f key. After a while, with output reporting 01:03:00 at a timestamp of 12:07:47.52 I started a manual analog stop watch. (I don't have time to work on a more elegant digital method, for the reason I mentioned earlier.) I'll leave that running and check occasionally. It's obviously going to take a long time even to prove that it does run fast, and ACCURACY tests could take days/weeks.

So what evidence do you think sufficient before you proceed to publish?

====================

On Tue, 26 Mar 2024 02:37:33 -0700, you wrote:

I indeed do not have a DS3231.

Can you, perhaps with help from another expert, suggest a sketch and schematic I can test that would give you confidence?

I just wrote the test sketch below. The required connections are:

Arduino's I2C pins to a DS3231

Arduino's USB port to a computer running either the Arduino's serial monitor or a terminal emulator

Source code (click to expand/collapse)
```c++ #include #ifdef MOCK_RTC /* Mock DS3231 for testing this sketch without the actual hardware. */ class Mock_RTC_DS3231 : public RTC_Micros { public: void begin() { RTC_Micros::begin(DateTime(F(__DATE__), F(__TIME__))); } int8_t getAgingOffset() { return aging_offset; } void setAgingOffset(int8_t offset) { aging_offset = offset; adjustDrift(-aging_offset * 200); // exaggerate drift } private: int8_t aging_offset; }; #define RTC_DS3231 Mock_RTC_DS3231 #endif // defined(MOCK_RTC) RTC_DS3231 rtc; const char help[] = "Available commands:\n\r" " h: print this help\n\r" " t: toggle printing RTC times\n\r" " f: make the RTC fast\n\r" " s: make the RTC slow\n\r" " p: print the aging offset"; void setup() { rtc.begin(); Serial.begin(9600); Serial.println("Ready."); Serial.println(help); } void loop() { /* Print RTC updates if asked to do so. */ static bool do_print_times = false; if (do_print_times) { static DateTime last_printed_time; DateTime now = rtc.now(); if (now != last_printed_time) { char buffer[] = "hh:mm:ss"; Serial.println(now.toString(buffer)); last_printed_time = now; } } /* Execute commands from the serial port. */ switch (Serial.read()) { case 'h': Serial.println(help); break; case 't': do_print_times = !do_print_times; break; case 'f': Serial.println("setting aging offset to -128 (fast)"); rtc.setAgingOffset(-128); break; case 's': Serial.println("setting aging offset to +127 (slow)"); rtc.setAgingOffset(+127); break; case 'p': Serial.print("aging offset: "); Serial.println(rtc.getAgingOffset()); } } ```

The sketch is controlled by single-character commands sent through the serial port. Send h to get a summary of the available commands. You may check that:

printing the aging offset gets what you would expect

setting the RTC to run fast makes it run fast

setting it to run slow makes it run slow

I tested the sketch using a mock RTC which I left there for the record. It will not be used unless you #define MOCK_RTC.

edgar-bonet commented 6 months ago

@terrypin999: Hello, and thanks for the test!

Before submitting a pull request, I would prefer to have clear evidence that the setting does affect the RTC’s drift rate. Rather than a stopwatch, I would rely on the Arduino serial monitor: it is already timestamping the received messages, with something like 30 ms of jitter. With this resolution, I would expect the RTC's drift to be visible after about 4 hours, and very clear after 8 hours. Do you think you could let it run for at least 8 hours in “fast” mode, and the same is “slow” mode? If so the procedure would be as follows:

hit f to make the RTC fast
hit t to start printing the RTC times
let the serial monitor capture a dozen or so timestamps
hit t again to avoid spamming the serial monitor
let it sit there for 8+ hours
repeat 2–4 to get a new series of timestamps
hit s to make the RTC slow
let it sit there for 8+ hours
repeat 2–4 to get a new series of timestamps

If it works, the received serial messages (timestamped by the serial monitor) should contain evidence that the adjustment works.

Regards,

terrypin999 commented 6 months ago

Ok. It is underway, although my precise steps were not identical. What does ‘spamming the serial monitor’ mean? Just sending too fast? If so how does that repeat toggling affect that?

Basically I was just planning to see if I can detect any fast running after a few hours. It’s been a couple of hours so far and I’ll be checking it again shortly. If you wish I’ll restart and follow your steps exactly after that.

-- Terry

On 27 Mar 2024, at 14:16, Edgar Bonet @.***> wrote:

spamming the serial monitor

edgar-bonet commented 6 months ago

What does ‘spamming the serial monitor’ mean? Just sending too fast?

Sending too much. If the Arduino kept printing the time every second for 8+ hours, you would end up with so much stuff in that window that it would be difficult to locate the place where you changed from “fast” mode to “slow” mode. And you would not want to post the whole capture here.

If you wish I’ll restart and follow your steps exactly after that.

If you are confident that the test you are doing is essentially equivalent to what I wrote, then there is no need to restart.

terrypin999 commented 6 months ago

On Wed, 27 Mar 2024 07:48:48 -0700, you wrote:

You're right about the redundancy of my stop watch, timestamps rule!

Here's my latest check, some three hours in.. The first two lines are: 12:03:08.038 -> setting aging offset to -128 (fast) 12:03:08.506 -> 00:58:21

The last line is 15:22:31.800 -> 04:17:44

That tells us that to nearest second, in 222s of real time, the DS3231 reported 222s. As expected, it's going to be a long test!

Just wondered if I'd not properly set it, but a check with your 'p' command still correctly shows -128.

I'll leave it running - and try not to accidentally stop it! Maybe by tomorrow morning...?

BTW, I note that the most I can copy/paste from the monitor is about 6,000 lines. I estimate there are about 15,000 lines now (15:48), as I didn't grasp the importance of toggling off. I assume that with the sketch now toggled off there's no risk of its not responding many hours later to my next t command?

What does spamming the serial monitor mean? Just sending too fast?

Sending too much. If the Arduino kept printing the time every second for 8+ hours, you would end up with so much stuff in that window that it would be difficult to locate the place where you changed from fast mode to slow mode. And you would not want to post the whole capture here.

If you wish Ill restart and follow your steps exactly after that.

If you are confident that the test you are doing is essentially equivalent to what I wrote, then there is no need to restart.

terrypin999 commented 6 months ago

Checked again after toggling reporting back on again.

I see my previous (flawed!) calculation of total seconds was unnecessary. We can simply subtract the first pair of readings from the last and look at the seconds element.

In this case subtract: 12:03:08.506 -> 00:58:21 from 18:41:07.113 -> 07:36:19 giving elapsed times of 06:37:59.607 -> 6:37:58

In the absence of decimal seconds, to nearest second that appears to show a difference of between 1s and 2s. But it looks SLOW rather than fast?

edgar-bonet commented 6 months ago

@terrypin999: Two things

You have to ignore the time that gets printed right after hitting the t key: this one is printed as you send the keystroke, not at a seconds boundary.

I think you got the subtraction wrong. I get:

18:41:07.113 -> 07:36:19
12:03:08.506 -> 00:58:21
────────────────────────
06:37:58.607 -> 06:37:58

This shows the RTC is slow by 0.6 s in 23878 s, which is about 25 ppm. This looks suspicious. Was any of those times caught right after hitting t? Could your computer's clock be running fast? Is it properly synchronized to an NTP server?

terrypin999 commented 6 months ago

You’re right about the subtraction. But I don’t see why you assume the reported time is EXACTLY.06:37:58.000On 27 Mar 2024, at 20:00, Edgar Bonet @.***> wrote: @terrypin999: Two things

You have to ignore the time that gets printed right after hitting the t key: this one is printed as you send the keystroke, not at a seconds boundary.

I think you got the subtraction wrong. I get: 18:41:07.113 -> 07:36:19 12:03:08.506 -> 00:58:21 ──────────────────────── 06:37:58.607 -> 06:37:58

This shows the RTC is slow by 0.6 s in 23878 s, which is about 25 ppm. This looks suspicious. Was any of those times caught right after hitting t? Could your computer's clock be running fast? Is it properly synchronized to an NTP server?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

edgar-bonet commented 6 months ago

I don’t see why you assume the reported time is EXACTLY 06:37:58.000

Not quite exactly, as there is about 30 ms of jitter. See my previous point number 1: if you ignore the time stamps printed after hitting t, all other timestamps are printed at full-second boundaries. This is because the Arduino is repeatedly (and very quickly) executing this piece of code:

DateTime now = rtc.now();
if (now != last_printed_time) {
    char buffer[] = "hh:mm:ss";
    Serial.println(now.toString(buffer));
    last_printed_time = now;
}

The RTC repeats the same time over and over again, and the sketch does not print it because it is always equal to last_printed_time. Then, once we cross a full-second boundary, the RTC returns a different time, and that one gets printed. The jitter is due in part to the time taken to go through the loop() (mostly the I2C communication time) and the handling of the serial data on the computer.

terrypin999 commented 6 months ago

Ok, thanks, broadly understood. I’ll leave it running overnight.On 27 Mar 2024, at 20:39, Edgar Bonet @.***> wrote:

I don’t see why you assume the reported time is EXACTLY 06:37:58.000

Not quite exactly, as there is about 30 ms of jitter. See my previous point number 1: if you ignore the time stamps printed after hitting t, all other timestamps are printed at full-second boundaries. This is because the Arduino is repeatedly (and very quickly) executing this piece of code: DateTime now = rtc.now(); if (now != last_printed_time) { char buffer[] = "hh:mm:ss"; Serial.println(now.toString(buffer)); last_printed_time = now; } The RTC repeats the same time over and over again, and the sketch does not print it because it is always equal to last_printed_time. Then, once we cross a full-second boundary, the RTC returns a different time, and that one gets printed. The jitter is due in part to the time taken to go through the loop() (mostly the I2C communication time) and the handling of the serial data on the computer.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

terrypin999 commented 6 months ago

On Wed, 27 Mar 2024 13:00:34 -0700, you wrote:

This shows the RTC is slow by 0.6 s in 23878 s, which is about 25 ppm. This looks suspicious. Was any of those times caught right after hitting t? Could your computer's clock be running fast? Is it properly synchronized to an NTP server?

Forgot to answer those in my last post. "This looks suspicious." Suspicious because? Slow instead of fast as specified? Too large?

"Was any of those times caught right after hitting t?" Yes, I took the first time reported after using the t command. I'll avoid that in future.

"Could your computer's clock be running fast?" If so then by a tiny degree if I understand Win10 Pro 'Date & Time' settings right: https://www.dropbox.com/scl/fi/57whbwupl1y82bjt86hab/DateTimeSettings.jpg?rlkey=8jkxexsekfrj6tzmj2eczsqe4&raw=1

"Is it properly synchronized to an NTP server?" Same comment.

I have not yet used the option to 'Sync now.

Here is a report taken about 40 mins since toggling: 10:42:28.471 -> 23:37:39 10:42:29.503 -> 23:37:40 10:42:30.488 -> 23:37:41 10:42:31.465 -> 23:37:42 10:42:32.496 -> 23:37:43 10:42:33.481 -> 23:37:44 10:42:34.465 -> 23:37:45

So the latest subtraction is 10:42:34.465 -> 23:37:45 12:03:08.606 -> 00:58:21

22:39:25.859 -> 22:39:24

If I correctly follow your earlier example, that implies it is running about 1.8s slow (not fast), yes?

What's your next suggestion? (I'd like to apply 5V externally to the Mega's 5V pin now, and remove the USB supply so that I can use that cable for other work. OK?)

Thu 28 Mar 2024 1057, UK

edgar-bonet commented 6 months ago

[The RTC being slow by 0.6 s] looks suspicious because?

Because it is slow instead of fast.

I took the first time reported after using the t command.

OK. Then a drift computed using that time would be invalid.

If [my computer's clock is running fast] then by a tiny degree if I understand Win10 Pro 'Date & Time' settings right

I am not fluent with Windows. However, a synchronization period of 11+ hours seems to long for an NTP client. Mine (systemd-timesyncd) uses a poll interval of 32 seconds, which grows progressively as as the synchronization improves, up to a maximum of 34 min 8 s. This can keep the local clock to within a handful milliseconds from the reference server.

I suspect Windows may be periodically stepping the clock here, instead of fine-tuning its frequency in a step-less fashion, like an NTP server would do. If this is the case, maybe it would be better to temporarily disable the synchronization, and let the computer's clock run free, just to avoid unexpected clock discontinuities.

terrypin999 commented 6 months ago

On Thu, 28 Mar 2024 09:33:21 -0700, you wrote:

[The RTC being slow by 0.6 s] looks suspicious because?

Because it is slow instead of fast.

I took the first time reported after using the t command.

OK. Then a drift computed using that time would be invalid.

If [my computer's clock is running fast] then by a tiny degree if I understand Win10 Pro 'Date & Time' settings right

I am not fluent with Windows. However, a synchronization period of 11+ hours seems to long for an NTP client. Mine (systemd-timesyncd) uses a poll interval of 32 seconds, which grows progressively as as the synchronization improves, up to a maximum of 34 min 8 s. This can keep the local clock to within a handful milliseconds from the reference server.

I suspect Windows may be periodically stepping the clock here, instead of fine-tuning its frequency in a step-less fashion, like an NTP server would do. If this is the case, maybe it would be better to temporarily disable the synchronization, and let the computer's clock run free, just to avoid unexpected clock discontinuities.

OK, made changes shown here: https://www.dropbox.com/scl/fi/57whbwupl1y82bjt86hab/DateTimeSettings.jpg?rlkey=8jkxexsekfrj6tzmj2eczsqe4&raw=1

Do you have enough data for time being? Let me know if you want it run overnight again. If so specify any settings, and whether a continuation or a restart.

I'd like to use the USB for other stuff tomorrow. Obviously my idea of substituting an external 5V is not possible if I want monitor usage. (Three buttons instead of serial inputs t, f?)

User input aside, does it even need continuous running? I could just reconnect the USB and take a pair of readings at any time, yes?

Here's the latest: 10:42:33.481 -> 23:37:44 10:42:34.465 -> 23:37:45 ------------------------ toggled reporting on at about 21:40 20:45:35.064 -> 09:40:45 20:45:35.205 -> 09:40:46 20:45:36.236 -> 09:40:47 20:45:37.221 -> 09:40:48 20:45:38.205 -> 09:40:49 20:45:39.234 -> 09:40:50 20:45:40.218 -> 09:40:51

So that gives 20:45:40.218 09:40:51.000 i.e. 21:40:51.000 in 24 hr format 12:03:08.038 00:58:21.000

08:42:32.180 08:42:30.000 Implying a falling behind of about 6 seconds daily. Not a desirable result ;-)

terrypin999 commented 6 months ago

Meant to include this: https://www.dropbox.com/scl/fi/n24qbj7b4r0r6f0zfss0r/Win10Settings.jpg?rlkey=hwv70ffgjl4bd44tolbvz63mg&raw=1

edgar-bonet commented 6 months ago

User input aside, does it even need continuous running? I could just reconnect the USB and take a pair of readings at any time, yes?

Yes. As long as the Arduino and the RTC are continuously powered, the USB link can be removed and re-established at will.

terrypin999 commented 6 months ago

On Fri, 29 Mar 2024 04:20:07 -0700, you wrote:

User input aside, does it even need continuous running? I could just reconnect the USB and take a pair of readings at any time, yes?

Yes. As long as the Arduino and the RTC are continuously powered, the USB link can be removed and re-established at will.

Had no further suggestions yesterday so last night changed aging to +127, to reverse the previous -128 session. Toggled on again at 12:48 today to extract these results:

20:46:55.213 -> 09:42:06 20:46:56.245 -> 09:42:07 22:09:46.465 -> setting aging offset to -128 (fast) 22:10:02.874 -> setting aging offset to +127 (slow) 22:10:16.891 -> 11:05:27 22:10:17.359 -> 11:05:28 22:10:18.344 -> 11:05:29 22:10:19.328 -> 11:05:30 22:10:20.360 -> 11:05:31 22:10:21.344 -> 11:05:32 22:10:22.329 -> 11:05:33 22:10:23.360 -> 11:05:34 22:10:24.345 -> 11:05:35 22:10:25.329 -> 11:05:36 12:48:19.439 -> 01:43:28 12:48:19.580 -> 01:43:29 12:48:20.564 -> 01:43:30

I'll leave you to extract anything of interest.

Visual inspection of the running sketch compared with an accurate online clock is crude but encouraging. Here's a video: https://www.dropbox.com/s/y9oyf1sg36b2t5m/EdgarLastResults.mp4?raw=1

Finished all testing for now. Let me know specific details if you need more.

BTW, is that fast/slow reversal a code error or what?

edgar-bonet commented 6 months ago

@terrypin999: I checked your data, converted everything to seconds to make the calculations easier, and here is what I see:

During the period around 22:10, the RTC was behind the PC by 39,889.344 seconds. During the next recorded period (around 12:48), the RTC was behind the PC by 39,891.528 seconds. Thus, the RTC lost 2.184 seconds over the course of 52,682.184 seconds. This is a drift rate of −41.4 ppm.

To me, this looks reasonable for a “slow” mode.

I will prepare a pull request.

terrypin999 commented 6 months ago

On Fri, 29 Mar 2024 13:10:58 -0700, you wrote:

@terrypin999: I checked your data, converted everything to seconds to make the calculations easier, and here is what I see:

During the period around 22:10, the RTC was behind the PC by 39,889.344 seconds. During the next recorded period (around 12:48), the RTC was behind the PC by 39,891.528 seconds. Thus, the RTC lost 2.184 seconds over the course of 52,682.184 seconds. This is a drift rate of ?41.4 ppm.

To me, this looks reasonable for a slow mode.

I will prepare a pull request.

Thanks, good work.

adafruit / RTClib

Aging Register adjustment: suggested addition #299

299: "Hobbyist here, not a programmer/developer/techie...". So github

Terry

Terry

define RTC_DS3231 Mock_RTC_DS3231

So the latest subtraction is 10:42:34.465 -> 23:37:45 12:03:08.606 -> 00:58:21

So that gives 20:45:40.218 09:40:51.000 i.e. 21:40:51.000 in 24 hr format 12:03:08.038 00:58:21.000