DS3231 to add get_aging, set_aging to adjust the internal xtal capacitor to refine time drift

[PeteB27]

Is there any interest in adding the read/write of DS3231 aging register (register 0x10) to the RTClib codeset ?

I have prepared a working (and tested) form using release RTClib v2.1.4 that I'm happy to share.
Changes made are contained in the following files:
RTC_DS3231.cpp
RTClib.h

Target system is Arduino MEGA, so the AVR systems like MEGA and NANO etc.

• Arduino board: Arduino MEGA

• Arduino IDE version (found in Arduino -> About Arduino menu): 1.8.19

• List the steps to reproduce the problem below (if possible attach a sketch or
  copy the sketch code in too):
  Not really relevant as the RTClib codeset does not currently have handlers for reading or adjusting the RTC DS3231 aging register, but as a code snippet, please look at the following sample project file lines:
  int8_t prevDrift=rtc.get_aging(); //to read the aging register value
  rtc.set_aging(newDrift); //to set a new value to the drift register, eg -17, and thus refine the RTC clock drift (speed up the clock by using -ve numbers in ppm, or slow down using +ve numbers).
  Code changes came from DS3231 library, written by Petre Rodan in 2018 and modified to suit your library.

The reason why I changed to your code; you have a working UNIX seconds counter, so for me, that's a big tick.

[edgar-bonet]

Is there any interest in adding the read/write of DS3231 aging register (register 0x10) to the RTClib codeset?

See issue #299, which I believe this is a duplicate of.

See also pull requests #270 and #300.

Given that the former PR has been stale for 17 months, it looks to me like the maintainers are not exceptionally eager to add this functionality. Also, this repo has been in low-intensity maintenance for almost two years.

[PeteB27]

Thank you edgar-bonet, yes, #300, #299 and #270 are all aiming in the same direction, so of course you wouldn't want another variant. I'll continue to use my cut but will keep an eye on any future updates in this space in RTClib. I see lots of work and tests with terrypin999; well done.
An aspect I had in my code for the aging set is to do a value check -128 to +127 and to discard any out of bounds attempts.
I've used the aging process for several years now to fine tune my various DS3231's, tuned using HF radio time signal from WWV (15.000MHz) and I'm approaching 1 sec drift in 6 months. I use a spreadsheet to use previous time error to determine an improved aging factor. The process was a weekly chore initially but now, 6 monthly. As I've found , all my DS3231's use different aging factors.
For what it's worth, my RTClib code changes (ref v2.1.4) are attached.
RTClib_DS3231_aging_PBS.zip

[edgar-bonet]

Your code does not discard out of bounds attempts. If you rtc.set_aging(150);, it will happily set the aging register to −106. For reference, here is your implementation:

void RTC_DS3231::set_aging(int8_t aged) {  //20240630 PBS added aging
  if ((aged < 128)&&(aged >= -128)) {      //only allow legal aging value ranges
    write_register(DS3231_AGING_OFFSET_ADDR, aged);
  }
}

As aged is of type int8_t, it is always within the range −128..+127. The test has no effect, and is actually discarded by the compiler. If you ask the Arduino IDE to show you all the compiler warnings, you should see this:

RTC_DS3231.cpp: In member function ‘void RTC_DS3231::set_aging(int8_t)’:
RTC_DS3231.cpp:401:13: warning: comparison is always true due to limited range of data type [-Wtype-limits]
   if ((aged < 128)&&(aged >= -128)) {      //only allow legal aging value range
             ^
RTC_DS3231.cpp:401:27: warning: comparison is always true due to limited range of data type [-Wtype-limits]
   if ((aged < 128)&&(aged >= -128)) {      //only allow legal aging value range
                           ^

I recommend you always enable all compiler warnings: this can help you catch many programmer errors like this one.

[PeteB27]

Very good points edgar-bonet, thank you. I'll revise the .ino to check the set_aging value before the call.
I see (in your reply thanks) the useless range test within the function so I pulled up my compiler warnings list and oddly I'm not seeing the "limited range of data type" warning you describe.
I'm (still) mainly using the older 1.8.19 IDE but with verbose compiler output and compiler warnings [All] set (please see below).
Now you've got me nervous that I might have other warnings amongst my 4000 line project that I'm oblivious to.
I have now tried IDE 2.3.2 on another PC but I also do not see the "limited range of data type" warnings.

[edgar-bonet]

@PeteB27: Oh, sorry, I was most likely wrong about the warnings on the IDE. 🙁 I seldom use the IDE myself, as I generally prefer using a good old Makefile. It is likely that this IDE setting affects the building of the sketch only, and is not honored when building the Arduino core and the libraries.

[PeteB27]

Ah cool, thanks edgar-bonet, I'll relax (for a while) :-). I haven't used Makefile directly since my VS working days.
Mind you, I do have a swag of warnings in my project IDE output from library issues from others, so there are plenty of bugs out there.
Cheers.