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 <noreply@anthropic.com>

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<std::uint8_t>((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<std::uint8_t>(control | intcn | enable_bit);
+	}
+	return static_cast<std::uint8_t>(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<std::uint8_t>((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<date_time> 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<date_time> read_alarm1()
@@ -261,34 +276,34 @@ class device {
 
 	[[nodiscard]] static result<bool> alarm1_fired()
 	{
-		return flag_set(a1f);
+		return flag_set(detail::a1f);
 	}
 
 	[[nodiscard]] static result<bool> 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<bool> 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
+	// INTCN routes the alarms to INT/SQW, so arming an alarm's interrupt takes
-	// would otherwise drive it, so arming an alarm's interrupt takes the pin
+	// the pin and muting it gives up only that alarm's claim on it.
-	// 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<std::uint8_t>((*control & ~bbsqw) | intcn | enable_bit);
+		return dev::write_reg(reg_control, detail::control_with(*control, enable_bit, interrupt));
-		const auto muted = static_cast<std::uint8_t>(*control & ~enable_bit);
-		return dev::write_reg(reg_control, interrupt == alarm_interrupt::on ? wired : muted);
 	}
 
 	static result<bool> 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<std::uint8_t>(*flags & ~bit));
+		return dev::write_reg(reg_status, detail::status_clearing(*flags, bit));
 	}
 };
 

test/consteval.cpp

@@ -79,4 +79,22 @@ static_assert(static_cast<std::uint8_t>(alarm2_rate::once_per_minute) ==
 static_assert(static_cast<std::uint8_t>(alarm2_rate::weekday_time_match) ==
               static_cast<std::uint8_t>(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