From 4df536d0a5f4eb224929bc0370356761f844238f Mon Sep 17 00:00:00 2001 From: BlackMark Date: Fri, 23 Oct 2026 21:42:32 +0200 Subject: [PATCH] fix: an alarm clear keeps the other alarm's flag, and arming keeps BBSQW A1F and A2F ignore a written 1, so the status write-back now writes the flag not being cleared as 1: an alarm firing between the read and the write was cleared with the other. BBSQW gates the alarm interrupt on battery as well as the square wave (19-5170 rev. 4), so arming an alarm no longer clears it. Both register computations are consteval-tested. Co-Authored-By: Claude Opus 5.5 --- include/ds3231/ds3231.hpp | 56 ++++++++++++++++++++++++--------------- test/consteval.cpp | 18 +++++++++++++ 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/include/ds3231/ds3231.hpp b/include/ds3231/ds3231.hpp index 76d677d..86966a4 100644 --- a/include/ds3231/ds3231.hpp +++ b/include/ds3231/ds3231.hpp @@ -101,6 +101,24 @@ constexpr std::uint8_t weekday(std::uint16_t year, std::uint8_t month, std::uint return static_cast((year + year / 4 - year / 100 + year / 400 + offsets[month - 1] + day) % 7 + 1); } +// Control and status bits, datasheet 19-5170 Figure 1. +inline constexpr std::uint8_t a1ie = 0x01, a2ie = 0x02, intcn = 0x04; +inline constexpr std::uint8_t a1f = 0x01, a2f = 0x02, osf = 0x80; + +constexpr std::uint8_t control_with(std::uint8_t control, std::uint8_t enable_bit, alarm_interrupt interrupt) +{ + if (interrupt == alarm_interrupt::on) { + return static_cast(control | intcn | enable_bit); + } + return static_cast(control & ~enable_bit); +} + +// A1F and A2F ignore a written 1, so writing the other one as 1 cannot lose it. +constexpr std::uint8_t status_clearing(std::uint8_t status, std::uint8_t flag) +{ + return static_cast((status | a1f | a2f) & ~flag); +} + } // namespace detail // The one bus address a DS3231 answers on - hardwired in the die, no @@ -125,9 +143,6 @@ class device { static constexpr std::uint8_t reg_status = 0x0f; static constexpr std::uint8_t reg_temp = 0x11; - static constexpr std::uint8_t a1ie = 0x01, a2ie = 0x02, intcn = 0x04, bbsqw = 0x40; - static constexpr std::uint8_t a1f = 0x01, a2f = 0x02, osf = 0x80; - public: [[nodiscard]] static result read_clock() { @@ -215,7 +230,7 @@ class device { if (auto s = dev::write_regs(reg_alarm1, raw); !s) { return s; } - return set_alarm_interrupt(a1ie, interrupt); + return set_alarm_interrupt(detail::a1ie, interrupt); } [[nodiscard]] static status set_alarm2(const date_time &at, alarm2_rate rate, @@ -229,7 +244,7 @@ class device { if (auto s = dev::write_regs(reg_alarm2, raw); !s) { return s; } - return set_alarm_interrupt(a2ie, interrupt); + return set_alarm_interrupt(detail::a2ie, interrupt); } [[nodiscard]] static result read_alarm1() @@ -261,34 +276,34 @@ class device { [[nodiscard]] static result alarm1_fired() { - return flag_set(a1f); + return flag_set(detail::a1f); } [[nodiscard]] static result alarm2_fired() { - return flag_set(a2f); + return flag_set(detail::a2f); } [[nodiscard]] static status clear_alarm1() { - return clear_flag(a1f); + return clear_flag(detail::a1f); } [[nodiscard]] static status clear_alarm2() { - return clear_flag(a2f); + return clear_flag(detail::a2f); } // True after the oscillator was ever stopped (first power-up, battery // ran out): the clock needs to be set. [[nodiscard]] static result oscillator_stopped() { - return flag_set(osf); + return flag_set(detail::osf); } [[nodiscard]] static status clear_oscillator_stopped() { - return clear_flag(osf); + return clear_flag(detail::osf); } // Die temperature in quarter C (updated every 64 s by the device). @@ -307,19 +322,19 @@ class device { // other's route intact. [[nodiscard]] static status listen_alarm1() { - return set_alarm_interrupt(a1ie, alarm_interrupt::on); + return set_alarm_interrupt(detail::a1ie, alarm_interrupt::on); } [[nodiscard]] static status mute_alarm1() { - return set_alarm_interrupt(a1ie, alarm_interrupt::off); + return set_alarm_interrupt(detail::a1ie, alarm_interrupt::off); } [[nodiscard]] static status listen_alarm2() { - return set_alarm_interrupt(a2ie, alarm_interrupt::on); + return set_alarm_interrupt(detail::a2ie, alarm_interrupt::on); } [[nodiscard]] static status mute_alarm2() { - return set_alarm_interrupt(a2ie, alarm_interrupt::off); + return set_alarm_interrupt(detail::a2ie, alarm_interrupt::off); } private: @@ -339,18 +354,15 @@ class device { return detail::to_bcd(day) & std::uint8_t{0x3f}; } - // INTCN routes the alarms to INT/SQW and BBSQW is the square wave that - // would otherwise drive it, so arming an alarm's interrupt takes the pin - // and muting it gives up only that alarm's claim on it. + // INTCN routes the alarms to INT/SQW, so arming an alarm's interrupt takes + // the pin and muting it gives up only that alarm's claim on it. static status set_alarm_interrupt(std::uint8_t enable_bit, alarm_interrupt interrupt) { auto control = dev::read_reg(reg_control); if (!control) { return std::unexpected(control.error()); } - const auto wired = static_cast((*control & ~bbsqw) | intcn | enable_bit); - const auto muted = static_cast(*control & ~enable_bit); - return dev::write_reg(reg_control, interrupt == alarm_interrupt::on ? wired : muted); + return dev::write_reg(reg_control, detail::control_with(*control, enable_bit, interrupt)); } static result flag_set(std::uint8_t bit) @@ -368,7 +380,7 @@ class device { if (!flags) { return std::unexpected(flags.error()); } - return dev::write_reg(reg_status, static_cast(*flags & ~bit)); + return dev::write_reg(reg_status, detail::status_clearing(*flags, bit)); } }; diff --git a/test/consteval.cpp b/test/consteval.cpp index 64b1f9a..dc581e0 100644 --- a/test/consteval.cpp +++ b/test/consteval.cpp @@ -79,4 +79,22 @@ static_assert(static_cast(alarm2_rate::once_per_minute) == static_assert(static_cast(alarm2_rate::weekday_time_match) == static_cast(alarm1_rate::weekday_time_match) >> 1); +// A1F and A2F ignore a written 1 and OSF holds until written 0 (19-5170, +// Status Register): clearing one alarm must write the other as 1, or an alarm +// firing between the read and the write is lost. +using ds3231::detail::status_clearing; +static_assert(status_clearing(0x00, 0x01) == 0x02); // A2F set after the read survives +static_assert(status_clearing(0x03, 0x02) == 0x01); +static_assert(status_clearing(0x88, 0x01) == 0x8a); // OSF and EN32kHz kept as read +static_assert(status_clearing(0x80, 0x80) == 0x03); + +// BBSQW keeps INT/SQW driven on battery for the alarm interrupt as well as the +// square wave (19-5170, Control Register and rev. 4), so arming an alarm must +// leave it as found. +using ds3231::alarm_interrupt; +using ds3231::detail::control_with; +static_assert(control_with(0x40, 0x01, alarm_interrupt::on) == 0x45); +static_assert(control_with(0x00, 0x02, alarm_interrupt::on) == 0x06); +static_assert(control_with(0x47, 0x01, alarm_interrupt::off) == 0x46); + } // namespace