Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DS3231: Add support for fetching alarm values #257

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

justinnewitter
Copy link
Contributor

@justinnewitter justinnewitter commented Feb 9, 2022

Summary

This commit introduces 4 new methods for DS3231 devices:

  • DateTime getAlarm1();
  • DateTime getAlarm2();
  • Ds3231Alarm1Mode getAlarm1Mode();
  • Ds3231Alarm2Mode getAlarm2Mode();

The getAlarm methods assume that the alarms are stored
using 24 hour format since this library doesn't support
storing alarms in 12 hour format (DS3231 hardware supports
this).

Updated the DS3231_alarm example to fetch alarm1. Also
improved the formatting of the serial logging.

Improved logging from the example sketch

03:39:35 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 0
03:39:37 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 0
03:39:39 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 0
03:39:41 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 0
03:39:43 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 1 - Alarm cleared
03:39:45 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 0
03:39:47 [Alarm1: 09 03:39:43, Mode: Second] SQW: 0 Fired: 0

Misc

  • I find it more intuitive to use binary values directly in code when doing bit twiddly stuff like this, but this codebase seems to use hex values instead. Because of this, I made my code match the surrounding code by using hex values.
  • I ran clang-format on RTC_DS3231.cpp, but not on DS3231_alarm.ino since it ends up making a bunch of changes to existing code.
  • Fixes Add getAlarm support? #221

This commit introduces 4 new methods for DS3231 devices:
  - DateTime getAlarm1();
  - DateTime getAlarm2();
  - Ds3231Alarm1Mode getAlarm1Mode();
  - Ds3231Alarm2Mode getAlarm2Mode();

The getAlarm methods assume that the alarms are stored
using 24 hour format since this library doesn't support
storing alarms in 12 hour format (DS3231 hardware supports
this).

Updated the DS3231_alarm example to fetch alarm1. Also
improved the formatting of the serial logging.
Copy link
Contributor

@edgar-bonet edgar-bonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice addition! I think I found a glitch though, in the handling of weekly alarms.

src/RTC_DS3231.cpp Outdated Show resolved Hide resolved
src/RTC_DS3231.cpp Outdated Show resolved Hide resolved
Handle edge case where the result of calling getAlarm is used to set the alarm
and the mode is set to "Day"

Co-authored-by: Edgar Bonet <edgar-bonet@users.noreply.github.com>
@caternuson
Copy link
Contributor

There's some minor code dupe here that could potentially be refactored. But I think OK to go ahead and merge as is for now.

Thanks for the updates.

@caternuson caternuson merged commit ce95623 into adafruit:master Aug 4, 2022
@JoseCintra
Copy link

Thanks for the updates. This library is great!!!

@techniciansoft
Copy link

Hi!
Thank you for perfect library!
Is it possible to add one more method to DS3231.cpp to read A1INT and A2INT bit from CONTROL_REG?
For example,
bool RTC_DS3231::getAlarmStatus(uint8_t alarm_num) { return (read_register(DS3231_CONTROL) >> (alarm_num - 1)) & 0x1; }

@justinnewitter
Copy link
Contributor Author

Hi! Thank you for perfect library! Is it possible to add one more method to DS3231.cpp to read A1INT and A2INT bit from CONTROL_REG? For example, bool RTC_DS3231::getAlarmStatus(uint8_t alarm_num) { return (read_register(DS3231_CONTROL) >> (alarm_num - 1)) & 0x1; }

@techniciansoft I posted a PR based on your suggestion. It might be a while before someone at Adafruit accepts it. They aren't obligated to accept it if they don't want the functionality in the lib. I suspect they will accept this one because it is useful and is a very simple change. Next time feel free to submit a PR or open a new issue rather than commenting on a merged PR 😄

@techniciansoft
Copy link

@techniciansoft I posted a PR based on your suggestion. It might be a while before someone at Adafruit accepts it. They aren't obligated to accept it if they don't want the functionality in the lib. I suspect they will accept this one because it is useful and is a very simple change. Next time feel free to submit a PR or open a new issue rather than commenting on a merged PR smile

@justinnewitter Thank you for your help!
It's all new stuff to me, I'm not very confident in that sort of thing yet. I apologize if my post was in the wrong place :)

@justinnewitter
Copy link
Contributor Author

@techniciansoft no worries! You'll be an expert in no time 😄

@jieggii
Copy link

jieggii commented Dec 30, 2023

Hi everyone, I have a question about the Alarm1 and Alarm2 method. I am not very familiar with DS3231 RTC and cpp and may sound stupid.

Why "month" field is set to 5 when returning DateTime object instead of setting to an actual month?

@edgar-bonet
Copy link
Contributor

@jieggii:

Why "month" field is set to 5 when returning DateTime object instead of setting to an actual month?

What do you mean by an “actual month”? 5 means “May”, which is an actual month.

The alarm date and time, as stored in the DS3231, has no month field, and no year field: year and month are then meaningless in the context of these alarms. The DateTime object, on the other hand, always has those fields, so something has to be put there. You should thus consider those fields as holding arbitrary, meaningless data.

The why May 2000 was chosen is an implementation detail. It turns out it is the first month, within the supported date/time range, where the day-of-month number matches the day-of-week number. This helps keeping the code simple.

@jieggii
Copy link

jieggii commented Dec 30, 2023

Thank you for the quick and clear response. Now I see.

I wondered because I wanted to set up an alarm to fire only once a year on the first of January. I did not pay attention that alarm does not care about the month number. Now I cannot find a way to achieve desired behavior. Is it even possible with this library and DS3231?

@edgar-bonet
Copy link
Contributor

@jieggii: The DS3231 hardware cannot do that. What you can do is set the alarm to fire on the first day of every month, and ignore it except on January.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add getAlarm support?
6 participants