deps: the pin advances to the reviewed libavr, and the clock example is 6 B smaller

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.clangd

@@ -1,13 +1,13 @@
 # Editor accommodations for the second frontend. No compilation database is
 # named here: this driver is made to be vendored, and this file travels with
-# it — a consumer's own database then covers these headers, at the consumer's
+# it - a consumer's own database then covers these headers, at the consumer's
 # chip. The checkout that is opened as a folder names its build tree in
 # .vscode/settings.json instead.
 CompileFlags:
   Add:
     # clang has no 24-bit integer and GCC's are keywords, not macros, so the
     # editor needs a stand-in for avr::uint24_t. The next width up is the only
-    # one available — clang rejects _BitInt(24) on this target.
+    # one available - clang rejects _BitInt(24) on this target.
     - -D__uint24=unsigned long
     - -D__int24=long
     # clangd forwards the driver's system includes but not its own header
@@ -16,7 +16,7 @@ CompileFlags:
     - -D__STDC_LIMIT_MACROS
     - -D__STDC_CONSTANT_MACROS
     # isr::emit spells a vector number into [[gnu::signal(N)]], which clang
-    # rejects rather than ignores — enough of them in one TU to reach the
+    # rejects rather than ignores - enough of them in one TU to reach the
     # default limit of 19 inside the headers and truncate the parse.
     - -ferror-limit=0
   Remove:

.gitattributes

@@ -1,12 +1,8 @@
-# Line endings are the repository's, not the editing machine's: this checkout
+# Line endings are the repository's, not the editing tool's (libavr guidance
-# is reached from two hosts, and a file rewritten by a Windows tool comes back
+# rule 50): a file rewritten by a Windows tool must not come back with every
-# with every line changed unless something says otherwise. Naming the source
+# line changed.
-# extensions left Markdown, Python, shell and CMake to whatever the writing
-# tool defaulted to, which is CRLF on one of the two.
 * text=auto eol=lf
 
 # Atmel Studio writes these and expects them back.
-*.vcxproj*  eol=crlf
 *.cppproj   eol=crlf
-*.sln       eol=crlf
 *.atsln     eol=crlf

.vscode/settings.json

@@ -7,25 +7,28 @@
 	// --query-driver lets clangd ask the cross compiler for its own system
 	// includes and target. The database is named here rather than in .clangd
 	// because that file travels with the driver into a consumer's submodule,
-	// where a build tree of this repo's own need not exist.
+	// where a build tree of this repo's own need not exist. It sits one level
+	// above the trees, and CMake Tools copies the selected preset's into it on
+	// every configure: the database carries `-mmcu`, so a fixed tree would
+	// check the driver against one chip.
 	"clangd.arguments": [
-		"--compile-commands-dir=${workspaceFolder}/build/atmega328p-generated",
+		"--compile-commands-dir=${workspaceFolder}/build",
 		"--query-driver=**avr-g++*",
 		"--header-insertion=never"
 	],
+	"cmake.copyCompileCommands": "${workspaceFolder}/build/compile_commands.json",
 
 	// The presets are the build interface, and the toolchain file inside the
-	// libavr submodule is the one place the compiler is chosen. **No prefix is
+	// libavr submodule is the one place the compiler is chosen. No prefix is
-	// named here**: a committed file may not name a path that is true of one
+	// named here: the gitignored local/machine.cmake at this repository's root
-	// machine (libavr guidance rule 50), so the gitignored local/machine.cmake
+	// is where a checkout says where its toolchain is (libavr guidance rule 50).
-	// at this repository's root is where a checkout says where its toolchain
-	// is - one file, and it answers for both hosts.
 	"cmake.useCMakePresets": "always",
 	"cmake.configureOnOpen": true,
 	"cmake.options.statusBarVisibility": "compact",
 
 	"files.watcherExclude": {
 		"**/build/**": true,
+		"**/local/**": true,
 		"**/libavr/**": true
 	},
 

README.md

@@ -14,7 +14,7 @@ auto now = rtc::read_clock();       // result<date_time>
 auto armed = rtc::set_alarm1({}, ds3231::alarm1_rate::once_per_second);
 ```
 
-`example/main.cpp` is the full tour. libavr rides as the `libavr/` submodule,
+`example/main.cpp` is a complete program. libavr rides as the `libavr/` submodule,
 pinned to the commit this driver builds against; `LIBAVR_ROOT` (cache or
 environment) overrides it for development against a working tree:
 
@@ -26,9 +26,10 @@ cmake --build --preset attiny85-generated
 
 Presets cover attiny85/atmega328p in both libavr modes (generated and
 reflect); `ctest` runs the consteval battery over the BCD, the hours
-register's two formats, the weekday and the alarm rate encodings, and holds
+register's two formats, the weekday, the alarm registers each rate writes and
-the example to its stated size — **1480 B** on the mega328P and **1270 B**
+the control and status write-backs, and holds the example to its stated size —
-on the tiny85, byte-identical across the two modes. The legacy
+**1462 B** on the mega328P and **1256 B** on the tiny85, byte-identical across
+the two modes. The legacy
 yazoalfa-based driver lives on the `master` branch.
 
 The weekday register is derived from the date by default. A program that
@@ -91,6 +92,6 @@ One reference describes one part, hence `--project`. Release is what the gate
 compares, the presets defining no debug build; Debug carries the `-Og
 -gdwarf-4` pair libavr's own debug preset uses.
 
-Differences from legacy: weekday-rate alarms now actually set the DY bit
+Differences from legacy: weekday-rate alarms set the DY bit (the legacy
-(the old `setAlarmHelper` always cleared it), reads/writes are single
+driver always cleared it), each register block is read or written in one
-coherent bus transactions, and errors are reported instead of ignored.
+coherent bus transaction, and errors are reported instead of ignored.

example/main.cpp

@@ -4,9 +4,7 @@
 using namespace avr::literals;
 
 // One source for the tiny85 (open-drain software I2C on the USI pins) and
-// the mega328P (TWI hardware). On first power-up the clock is seeded and
+// the mega328P (TWI hardware).
-// alarm 1 armed; then the LED mirrors the seconds parity - a stuck LED
-// means bus errors.
 using dev = avr::device<{.clock = 8_MHz}>;
 using bus = dev::i2c<{.frequency = 100_kHz}>;
 using rtc = ds3231::device<bus>;
@@ -17,8 +15,6 @@ int main()
 	avr::init<bus, led>();
 
 	if (auto stopped = rtc::oscillator_stopped(); stopped.value_or(false)) {
-		// A failed seed leaves the LED dark before the loop ever runs - the
-		// same signal a stuck bus gives it below.
 		const bool seeded = rtc::write_clock({{2026, 1, 1}, {12, 0, 0}}) &&
 		                    rtc::set_alarm1({}, ds3231::alarm1_rate::once_per_second) &&
 		                    rtc::clear_oscillator_stopped();
@@ -30,7 +26,7 @@ int main()
 			led::write(now->second % 2 == 0);
 		}
 		if (auto fired = rtc::alarm1_fired(); fired.value_or(false) && !rtc::clear_alarm1()) {
-			led::clear(); // a clear that failed holds the LED dark, like any bus error
+			led::clear();
 		}
 		dev::delay<100_ms>();
 	}

include/ds3231/ds3231.hpp

@@ -1,9 +1,11 @@
 #pragma once
 
 #include <array>
+#include <cstddef>
 #include <cstdint>
 #include <expected>
 #include <span>
+#include <utility>
 
 #include <libavr/i2c.hpp>
 
@@ -65,6 +67,19 @@ enum class alarm2_rate : std::uint8_t {
 
 namespace detail {
 
+// The register map, datasheet 19-5170 Figure 1.
+inline constexpr std::uint8_t reg_seconds = 0x00, reg_day = 0x03, reg_date = 0x04;
+inline constexpr std::uint8_t reg_alarm1 = 0x07, reg_alarm2 = 0x0b;
+inline constexpr std::uint8_t reg_control = 0x0e, reg_status = 0x0f, reg_temp = 0x11;
+
+inline constexpr std::uint8_t alarm_mask = 0x80, day_of_week = 0x40;
+inline constexpr std::uint8_t twelve_hour = 0x40, pm = 0x20;
+inline constexpr std::uint8_t century = 0x80;
+inline constexpr std::uint8_t a1ie = 0x01, a2ie = 0x02, intcn = 0x04;
+inline constexpr std::uint8_t a1f = 0x01, a2f = 0x02, osf = 0x80;
+
+inline constexpr std::uint16_t first_year = 2000;
+
 constexpr std::uint8_t to_bcd(std::uint8_t value)
 {
 	return static_cast<std::uint8_t>(((value / 10) << 4) | (value % 10));
@@ -75,19 +90,18 @@ constexpr std::uint8_t from_bcd(std::uint8_t value)
 	return static_cast<std::uint8_t>((value >> 4) * 10 + (value & 0x0f));
 }
 
-// Hours register: bit 6 selects 12-hour mode, bit 5 is then PM. Writes
+// Writes always use the 24-hour form; reads accept either.
-// always use 24-hour form; reads accept either.
 constexpr std::uint8_t hours_from_reg(std::uint8_t reg)
 {
-	if (reg & 0x40) {
+	if (reg & twelve_hour) {
-		auto hour = from_bcd(reg & 0x1f);
+		const auto hour = from_bcd(reg & ~(alarm_mask | twelve_hour | pm));
-		bool pm = reg & 0x20;
+		const bool after_noon = reg & pm;
 		if (hour == 12) {
-			return pm ? 12 : 0;
+			return after_noon ? 12 : 0;
 		}
-		return static_cast<std::uint8_t>(pm ? hour + 12 : hour);
+		return static_cast<std::uint8_t>(after_noon ? hour + 12 : hour);
 	}
-	return from_bcd(reg & 0x3f);
+	return from_bcd(reg & ~(alarm_mask | twelve_hour));
 }
 
 // Sakamoto's method; the device counts weekdays 1..7 with a free epoch,
@@ -101,16 +115,56 @@ 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);
 }
 
+constexpr time_of_day time_from_registers(std::span<const std::uint8_t> raw)
+{
+	return {hours_from_reg(raw[2]), from_bcd(raw[1] & ~alarm_mask), from_bcd(raw[0] & ~alarm_mask)};
+}
+
+// Rate bit n is the mask bit of the alarm's n-th register and the bit above
+// them is DY/DT (Table 2). Alarm 2 has no seconds register, so its registers
+// are alarm 1's last three.
+inline constexpr std::size_t alarm1_registers = reg_alarm2 - reg_alarm1;
+inline constexpr std::size_t alarm2_registers = reg_control - reg_alarm2;
+
+template <std::size_t Count>
+constexpr std::array<std::uint8_t, Count> alarm_registers(const date_time &at, std::uint8_t rate)
+{
+	const bool weekday_mode = (rate >> Count) & 1;
+	const std::array<std::uint8_t, alarm1_registers> fields{
+	    to_bcd(at.second),
+	    to_bcd(at.minute),
+	    to_bcd(at.hour),
+	    weekday_mode ? static_cast<std::uint8_t>(day_of_week | at.day) : to_bcd(at.day),
+	};
+	return [&]<std::size_t... I>(std::index_sequence<I...>) {
+		return std::array{
+		    static_cast<std::uint8_t>(fields[alarm1_registers - Count + I] | (((rate >> I) & 1) ? alarm_mask : 0))...};
+	}(std::make_index_sequence<Count>{});
+}
+
+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
 // address pins (datasheet 19-5170, "I2C interface").
 inline constexpr std::uint8_t bus_address = 0x68;
 
-// Maxim DS3231 RTC on any libavr i2c bus (register map: datasheet
+// Maxim DS3231 RTC on any libavr i2c bus: the master role is taken from the
-// 19-5170 Table 1): the master role is taken from the bus's own
+// bus's own resolution, so a declared `dev::i2c<...>` is handed over whole.
-// resolution, so a declared `dev::i2c<...>` is handed over whole. Every
+// Every read is one bus transaction; errors surface as std::expected. Weekday
-// call is one bus transaction; errors surface as std::expected. Weekday
 // registers are derived from the date unless the program owns them.
 template <typename Bus, weekday_source Weekday = weekday_source::computed>
 class device {
@@ -118,28 +172,18 @@ class device {
 
 	using dev = avr::i2c::device<typename Bus::master, bus_address>;
 
-	static constexpr std::uint8_t reg_clock = 0x00;
-	static constexpr std::uint8_t reg_alarm1 = 0x07;
-	static constexpr std::uint8_t reg_alarm2 = 0x0b;
-	static constexpr std::uint8_t reg_control = 0x0e;
-	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()
 	{
 		std::array<std::uint8_t, 7> raw;
-		if (auto s = dev::read_regs(reg_clock, raw); !s) {
+		if (auto s = dev::read_regs(detail::reg_seconds, raw); !s) {
 			return std::unexpected(s.error());
 		}
 		date_time now;
-		static_cast<time_of_day &>(now) = decode_time(raw);
+		static_cast<time_of_day &>(now) = detail::time_from_registers(raw);
-		now.day = detail::from_bcd(raw[4] & 0x3f);
+		now.day = detail::from_bcd(raw[4]);
-		now.month = detail::from_bcd(raw[5] & 0x1f);
+		now.month = detail::from_bcd(raw[5] & ~detail::century);
-		now.year = static_cast<std::uint16_t>(2000 + detail::from_bcd(raw[6]));
+		now.year = static_cast<std::uint16_t>(detail::first_year + detail::from_bcd(raw[6]));
 		return now;
 	}
 
@@ -155,50 +199,50 @@ class device {
 	[[nodiscard]] static result<time_of_day> read_time()
 	{
 		std::array<std::uint8_t, 3> raw;
-		if (auto s = dev::read_regs(reg_clock, raw); !s) {
+		if (auto s = dev::read_regs(detail::reg_seconds, raw); !s) {
 			return std::unexpected(s.error());
 		}
-		return decode_time(raw);
+		return detail::time_from_registers(raw);
 	}
 
 	[[nodiscard]] static status write_clock(const date_time &now)
 	{
-		if constexpr (!computes_weekday) {
+		if constexpr (computes_weekday) {
-			// Two transactions, because the weekday register sits between the
-			// time and the date and this mode may not write it.
-			if (auto s = write_time(now); !s) {
-				return s;
-			}
-			return write_date(now);
-		} else {
 			std::array raw{detail::to_bcd(now.second),
 			               detail::to_bcd(now.minute),
 			               detail::to_bcd(now.hour),
 			               detail::weekday(now.year, now.month, now.day),
 			               detail::to_bcd(now.day),
 			               detail::to_bcd(now.month),
-			               detail::to_bcd(static_cast<std::uint8_t>(now.year % 100))};
+			               detail::to_bcd(static_cast<std::uint8_t>(now.year - detail::first_year))};
-			return dev::write_regs(reg_clock, raw);
+			return dev::write_regs(detail::reg_seconds, raw);
+		} else {
+			// Two transactions, because the weekday register sits between the
+			// time and the date and this mode may not write it.
+			if (auto s = write_time(now); !s) {
+				return s;
+			}
+			return write_date(now);
 		}
 	}
 
 	[[nodiscard]] static status write_date(const date &value)
 	{
+		const auto year = detail::to_bcd(static_cast<std::uint8_t>(value.year - detail::first_year));
 		if constexpr (computes_weekday) {
 			std::array raw{detail::weekday(value.year, value.month, value.day), detail::to_bcd(value.day),
-			               detail::to_bcd(value.month), detail::to_bcd(static_cast<std::uint8_t>(value.year % 100))};
+			               detail::to_bcd(value.month), year};
-			return dev::write_regs(reg_clock + 3, raw);
+			return dev::write_regs(detail::reg_day, raw);
 		} else {
-			std::array raw{detail::to_bcd(value.day), detail::to_bcd(value.month),
+			std::array raw{detail::to_bcd(value.day), detail::to_bcd(value.month), year};
-			               detail::to_bcd(static_cast<std::uint8_t>(value.year % 100))};
+			return dev::write_regs(detail::reg_date, raw);
-			return dev::write_regs(reg_clock + 4, raw);
 		}
 	}
 
 	[[nodiscard]] static status write_time(const time_of_day &value)
 	{
 		std::array raw{detail::to_bcd(value.second), detail::to_bcd(value.minute), detail::to_bcd(value.hour)};
-		return dev::write_regs(reg_clock, raw);
+		return dev::write_regs(detail::reg_seconds, raw);
 	}
 
 	// Alarm times use .day as the date of month, or as weekday 1..7 with
@@ -206,96 +250,64 @@ class device {
 	[[nodiscard]] static status set_alarm1(const date_time &at, alarm1_rate rate,
 	                                       alarm_interrupt interrupt = alarm_interrupt::on)
 	{
-		auto m = static_cast<std::uint8_t>(rate);
+		return set_alarm<detail::alarm1_registers>(detail::reg_alarm1, at, std::to_underlying(rate), detail::a1ie,
-		std::array raw{static_cast<std::uint8_t>(detail::to_bcd(at.second) | ((m & 1) << 7)),
+		                                           interrupt);
-		               static_cast<std::uint8_t>(detail::to_bcd(at.minute) | (((m >> 1) & 1) << 7)),
-		               static_cast<std::uint8_t>(detail::to_bcd(at.hour) | (((m >> 2) & 1) << 7)),
-		               static_cast<std::uint8_t>(day_date(at.day, rate == alarm1_rate::weekday_time_match) |
-		                                         (((m >> 3) & 1) << 7))};
-		if (auto s = dev::write_regs(reg_alarm1, raw); !s) {
-			return s;
-		}
-		return set_alarm_interrupt(a1ie, interrupt);
 	}
 
 	[[nodiscard]] static status set_alarm2(const date_time &at, alarm2_rate rate,
 	                                       alarm_interrupt interrupt = alarm_interrupt::on)
 	{
-		auto m = static_cast<std::uint8_t>(rate);
+		return set_alarm<detail::alarm2_registers>(detail::reg_alarm2, at, std::to_underlying(rate), detail::a2ie,
-		std::array raw{static_cast<std::uint8_t>(detail::to_bcd(at.minute) | ((m & 1) << 7)),
+		                                           interrupt);
-		               static_cast<std::uint8_t>(detail::to_bcd(at.hour) | (((m >> 1) & 1) << 7)),
-		               static_cast<std::uint8_t>(day_date(at.day, rate == alarm2_rate::weekday_time_match) |
-		                                         (((m >> 2) & 1) << 7))};
-		if (auto s = dev::write_regs(reg_alarm2, raw); !s) {
-			return s;
-		}
-		return set_alarm_interrupt(a2ie, interrupt);
 	}
 
 	[[nodiscard]] static result<date_time> read_alarm1()
 	{
-		std::array<std::uint8_t, 4> raw;
+		return read_alarm<detail::alarm1_registers>(detail::reg_alarm1);
-		if (auto s = dev::read_regs(reg_alarm1, raw); !s) {
-			return std::unexpected(s.error());
-		}
-		date_time at{};
-		at.second = detail::from_bcd(raw[0] & 0x7f);
-		at.minute = detail::from_bcd(raw[1] & 0x7f);
-		at.hour = detail::hours_from_reg(raw[2] & 0x7f);
-		at.day = (raw[3] & 0x40) ? (raw[3] & 0x0f) : detail::from_bcd(raw[3] & 0x3f);
-		return at;
 	}
 
 	[[nodiscard]] static result<date_time> read_alarm2()
 	{
-		std::array<std::uint8_t, 3> raw;
+		return read_alarm<detail::alarm2_registers>(detail::reg_alarm2);
-		if (auto s = dev::read_regs(reg_alarm2, raw); !s) {
-			return std::unexpected(s.error());
-		}
-		date_time at{};
-		at.minute = detail::from_bcd(raw[0] & 0x7f);
-		at.hour = detail::hours_from_reg(raw[1] & 0x7f);
-		at.day = (raw[2] & 0x40) ? (raw[2] & 0x0f) : detail::from_bcd(raw[2] & 0x3f);
-		return at;
 	}
 
 	[[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).
 	[[nodiscard]] static result<std::int16_t> temperature_quarters()
 	{
 		std::array<std::uint8_t, 2> raw;
-		if (auto s = dev::read_regs(reg_temp, raw); !s) {
+		if (auto s = dev::read_regs(detail::reg_temp, raw); !s) {
 			return std::unexpected(s.error());
 		}
 		return static_cast<std::int16_t>((static_cast<std::int16_t>(static_cast<std::int8_t>(raw[0])) << 2) |
@@ -307,55 +319,67 @@ 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:
-	// Seconds, minutes and hours decode the same way wherever they are read
+	template <std::size_t Count>
-	// from - the high bit of the first two is reserved, and the hours register
+	static status set_alarm(std::uint8_t reg, const date_time &at, std::uint8_t rate, std::uint8_t enable_bit,
-	// carries its own 12/24-hour flag.
+	                        alarm_interrupt interrupt)
-	static constexpr time_of_day decode_time(std::span<const std::uint8_t> clock)
 	{
-		return {detail::hours_from_reg(clock[2]), detail::from_bcd(clock[1] & 0x7f), detail::from_bcd(clock[0] & 0x7f)};
+		if (auto s = dev::write_regs(reg, detail::alarm_registers<Count>(at, rate)); !s) {
+			return s;
+		}
+		return set_alarm_interrupt(enable_bit, interrupt);
 	}
 
-	static constexpr std::uint8_t day_date(std::uint8_t day, bool weekday_mode)
+	template <std::size_t Count>
+	static result<date_time> read_alarm(std::uint8_t reg)
 	{
-		if (weekday_mode) {
+		std::array<std::uint8_t, Count> raw;
-			return static_cast<std::uint8_t>(0x40 | (day & 0x0f));
+		if (auto s = dev::read_regs(reg, raw); !s) {
+			return std::unexpected(s.error());
 		}
-		return detail::to_bcd(day) & std::uint8_t{0x3f};
+		constexpr auto first = detail::alarm1_registers - Count;
+		date_time at{};
+		if constexpr (first == 0) {
+			at.second = detail::from_bcd(raw[0] & ~detail::alarm_mask);
+		}
+		at.minute = detail::from_bcd(raw[1 - first] & ~detail::alarm_mask);
+		at.hour = detail::hours_from_reg(raw[2 - first]);
+		at.day = detail::from_bcd(raw[3 - first] & ~(detail::alarm_mask | detail::day_of_week));
+		return at;
 	}
 
-	// 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);
+		auto control = dev::read_reg(detail::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(detail::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)
 	{
-		auto flags = dev::read_reg(reg_status);
+		auto flags = dev::read_reg(detail::reg_status);
 		if (!flags) {
 			return std::unexpected(flags.error());
 		}
@@ -364,11 +388,11 @@ class device {
 
 	static status clear_flag(std::uint8_t bit)
 	{
-		auto flags = dev::read_reg(reg_status);
+		auto flags = dev::read_reg(detail::reg_status);
 		if (!flags) {
 			return std::unexpected(flags.error());
 		}
-		return dev::write_reg(reg_status, static_cast<std::uint8_t>(*flags & ~bit));
+		return dev::write_reg(detail::reg_status, detail::status_clearing(*flags, bit));
 	}
 };
 

test/CMakeLists.txt

@@ -3,9 +3,9 @@ libavr_format_test()
 # The example's size is stated in the README, per chip, and a library advance
 # that moves it silently is what this catches.
 if(LIBAVR_MCU STREQUAL "atmega328p")
-	libavr_size_claim_test(clock 1480)
+	libavr_size_claim_test(clock 1462)
 elseif(LIBAVR_MCU STREQUAL "attiny85")
-	libavr_size_claim_test(clock 1270)
+	libavr_size_claim_test(clock 1256)
 endif()
 
 # The README says the two modes emit the same image, and only a tree with
@@ -21,7 +21,5 @@ add_test(NAME ds3231.consteval
          COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target consteval_tests)
 
 # A check this host cannot run reads as Skipped rather than Failed: the marker
-# a stubbed launcher prints, over every test registered here. Nothing in this
+# a stubbed launcher prints, over every test registered here.
-# suite is host-gated today, and this is what keeps that a property of the
-# suite rather than of whoever adds the next check.
 libavr_skip_unverified()

test/consteval.cpp

@@ -1,9 +1,10 @@
 // Compile-only battery, built with the cross compiler so the target's 16-bit
 // int is exercised. Everything in this driver that is arithmetic rather than a
 // bus transaction lives here: the BCD both ways, the hours register's two
-// formats, Sakamoto's weekday, and the alarm rate encodings as the datasheet
+// formats, Sakamoto's weekday, the alarm registers each rate writes, and the
-// tabulates them.
+// control and status values written back.
 
+#include <array>
 #include <cstdint>
 
 #include <ds3231/ds3231.hpp>
@@ -53,30 +54,58 @@ static_assert(weekday(2024, 3, 1) == 6);  // Friday, the day after it
 static_assert(weekday(2100, 3, 1) == 2);  // Monday - 2100 is not a leap year
 static_assert(weekday(2026, 8, 12) == 4); // Wednesday
 
-// The alarm rate encodings (19-5170 Table 2), transcribed: the low bits are
+// The alarm registers each rate writes (19-5170 Table 2): bit 7 of the n-th
-// A1M4..A1M1 in register order and the high one is DY/DT. A rate is the mask
+// register is AxM(n+1), and DY/DT is bit 6 of the day register. Rows are read
-// pattern its row names, and a transcription slip here arms the wrong alarm.
+// off the table, so a slip in the rate values or in their distribution over
+// the registers arms the wrong alarm here first.
 using ds3231::alarm1_rate;
-static_assert(static_cast<std::uint8_t>(alarm1_rate::once_per_second) == 0b01111);
-static_assert(static_cast<std::uint8_t>(alarm1_rate::seconds_match) == 0b01110);
-static_assert(static_cast<std::uint8_t>(alarm1_rate::minutes_seconds_match) == 0b01100);
-static_assert(static_cast<std::uint8_t>(alarm1_rate::time_match) == 0b01000);
-static_assert(static_cast<std::uint8_t>(alarm1_rate::date_time_match) == 0b00000);
-static_assert(static_cast<std::uint8_t>(alarm1_rate::weekday_time_match) == 0b10000);
-
 using ds3231::alarm2_rate;
-static_assert(static_cast<std::uint8_t>(alarm2_rate::once_per_minute) == 0b0111);
+using ds3231::detail::alarm_registers;
-static_assert(static_cast<std::uint8_t>(alarm2_rate::minutes_match) == 0b0110);
+constexpr ds3231::date_time at{{2026, 1, 15}, {12, 30, 45}};
-static_assert(static_cast<std::uint8_t>(alarm2_rate::time_match) == 0b0100);
+constexpr ds3231::date_time on_weekday{{2026, 1, 3}, {7, 5, 0}};
-static_assert(static_cast<std::uint8_t>(alarm2_rate::date_time_match) == 0b0000);
-static_assert(static_cast<std::uint8_t>(alarm2_rate::weekday_time_match) == 0b1000);
 
-// Alarm 2 has no seconds register, so its mask bits are one place lower than
+template <typename Rate>
-// alarm 1's throughout - the property that makes one distribution loop wrong
+constexpr auto alarm1(const ds3231::date_time &value, Rate rate)
-// for the other.
+{
-static_assert(static_cast<std::uint8_t>(alarm2_rate::once_per_minute) ==
+	return alarm_registers<ds3231::detail::alarm1_registers>(value, static_cast<std::uint8_t>(rate));
-              static_cast<std::uint8_t>(alarm1_rate::once_per_second) >> 1);
+}
-static_assert(static_cast<std::uint8_t>(alarm2_rate::weekday_time_match) ==
+
-              static_cast<std::uint8_t>(alarm1_rate::weekday_time_match) >> 1);
+template <typename Rate>
+constexpr auto alarm2(const ds3231::date_time &value, Rate rate)
+{
+	return alarm_registers<ds3231::detail::alarm2_registers>(value, static_cast<std::uint8_t>(rate));
+}
+
+static_assert(alarm1(at, alarm1_rate::once_per_second) == std::to_array<std::uint8_t>({0xc5, 0xb0, 0x92, 0x95}));
+static_assert(alarm1(at, alarm1_rate::seconds_match) == std::to_array<std::uint8_t>({0x45, 0xb0, 0x92, 0x95}));
+static_assert(alarm1(at, alarm1_rate::minutes_seconds_match) == std::to_array<std::uint8_t>({0x45, 0x30, 0x92, 0x95}));
+static_assert(alarm1(at, alarm1_rate::time_match) == std::to_array<std::uint8_t>({0x45, 0x30, 0x12, 0x95}));
+static_assert(alarm1(at, alarm1_rate::date_time_match) == std::to_array<std::uint8_t>({0x45, 0x30, 0x12, 0x15}));
+static_assert(alarm1(on_weekday, alarm1_rate::weekday_time_match) ==
+              std::to_array<std::uint8_t>({0x00, 0x05, 0x07, 0x43}));
+
+static_assert(alarm2(at, alarm2_rate::once_per_minute) == std::to_array<std::uint8_t>({0xb0, 0x92, 0x95}));
+static_assert(alarm2(at, alarm2_rate::minutes_match) == std::to_array<std::uint8_t>({0x30, 0x92, 0x95}));
+static_assert(alarm2(at, alarm2_rate::time_match) == std::to_array<std::uint8_t>({0x30, 0x12, 0x95}));
+static_assert(alarm2(at, alarm2_rate::date_time_match) == std::to_array<std::uint8_t>({0x30, 0x12, 0x15}));
+static_assert(alarm2(on_weekday, alarm2_rate::weekday_time_match) == std::to_array<std::uint8_t>({0x05, 0x07, 0x43}));
+
+// 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

tools/check.sh

@@ -1,11 +1,11 @@
 #!/usr/bin/env bash
 # The task gate: every configure preset built, and only then every one tested.
 #
-# The loop itself is libavr's (tools/check-presets.sh) because the order in it
+# The loop itself is libavr's (tools/check-presets.sh): the mode-identity
-# is subtle and was got wrong by hand more than once - the mode-identity checks
+# checks read the sibling mode's tree, so a build-then-test-per-preset run
-# read the sibling mode's tree, so a build-then-test-per-preset run compares a
+# would compare a fresh image against a stale sibling. Anything this repository
-# fresh image against a stale sibling. Anything this repository needs beyond
+# needs beyond the presets is written after the call, where every tree is built
-# the presets is written after the call, where every tree is built and tested.
+# and tested.
 set -euo pipefail
 cd "$(dirname "${BASH_SOURCE[0]}")/.."
 "${LIBAVR_ROOT:-libavr}/tools/check-presets.sh" "$@"