deps: the pins advance to the reviewed libavr and pureboot, and the image is smaller

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

.clangd

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

CMakeLists.txt

@@ -37,14 +37,12 @@ add_custom_command(TARGET fantemp POST_BUILD
                    COMMAND ${CMAKE_OBJCOPY} -O ihex -R .eeprom
                            $<TARGET_FILE:fantemp> $<TARGET_FILE_DIR:fantemp>/fantemp.hex)
 
-# The board's loader, built here rather than named from memory: consuming
-# pureboot for its geometry alone left the one image this deployment cannot be
-# recovered without outside the repository that pins it. Every parameter is
-# pureboot's own default for this chip - USART0 at 115200 on the 16 MHz crystal
-# `src/board.hpp` declares - and that is measured rather than assumed, the
-# image being byte-for-byte the 386 B the board's slot reads back. It goes on
-# over the wire (`--update-loader`), which is why there is no flash target
-# beside it: no programmer has ever been in this board's path.
+# The board's loader, built from the pin, so the one image this deployment
+# cannot be recovered without is rebuilt by the commit that names it. Every
+# parameter is pureboot's own default for this chip - USART0 at 115200 on the
+# 16 MHz crystal `src/board.hpp` declares. It goes on over the wire
+# (`--update-loader`), which is why there is no flash target beside it: the
+# board has no programming header.
 pureboot_add_loader(pureboot)
 
 enable_testing()

README.md

@@ -18,7 +18,7 @@ Commands may be abbreviated to any unambiguous-by-order prefix, as the legacy
 firmware allowed: `up` is `uptime`, `st` is `statistics`, `sa` is `save`. The
 table order resolves ties, so `s` is `show` — and `reset` is deliberately the one
 command that cannot be abbreviated, because `r` should not be able to wipe the
-histogram. `save` (new) forces a writeback, which otherwise happens every 30
+histogram. `save` forces a writeback, which otherwise happens every 30
 minutes and on the way into the bootloader.
 
 `show`, `statistics` and the histogram print one value per line behind a dotted
@@ -28,9 +28,8 @@ from 10 to 60 with a bar, because the curve is a cubic and five-degree samples
 without a graph show none of its shape.
 
 **Ctrl+C** abandons a half-typed line and gives a fresh prompt, echoing `^C`, and
-it is what stops `monitor`. Stopping on *any* byte, which is what the port did
-first, reads well right up until a host sends a line ending: `monitor\r\n` then
-stopped itself on the `\n` it arrived with, one reading in.
+it is what stops `monitor`. Stopping on *any* byte would not do: a host's line
+ending would stop `monitor\r\n` on the `\n` it arrived with, one reading in.
 
 ## Reaching the bootloader
 
@@ -47,9 +46,10 @@ stopped itself on the `\n` it arrived with, one reading in.
   that bit-bangs the same pin receives perfectly and answers into nothing.
 
 `ctest` reads all three back out of the emitted image (`test/check_reachability.py`),
-because none of them is visible from the source alone and the failure mode is an
-unreflashable board. Both the address and the watchdog checks are red-proven
-against the legacy behaviour they exist to catch.
+with the image's clearance below the boot section, because none of them is
+visible from the source alone and the failure mode is an unreflashable board.
+Each is red-proven: the address and the watchdog checks against the legacy
+behaviour they exist to catch, the USART check against the write removed.
 
 The Steinhart–Hart math of the legacy firmware (runtime doubles + libm
 log) is gone: the Beta equation and the cubic fan curve are evaluated
@@ -66,7 +66,7 @@ cmake --preset atmega328p-generated
 cmake --build --preset atmega328p-generated
 ```
 
-The firmware is **8004 B** of flash, byte-identical between the generated and
+The firmware is **7836 B** of flash, byte-identical between the generated and
 reflect modes, and `ctest` holds it to that number.
 
 ## Atmel Studio

dev/tasks.md

@@ -16,8 +16,8 @@ and re-propose work the owner has already refused.
 ### The millisecond clock stays 64 bits
 
 Narrowing `uptime::millis()` and the three timestamps that hold its value from
-`uint64_t` to `uint32_t` is **608 B of flash, 7.6 % of the image** (8004 B down
-to 7396), plus 16 B of RAM.
+`uint64_t` to `uint32_t` is **602 B of flash, 7.7 % of the image** (7838 B down
+to 7236), plus 16 B of RAM.
 
 **Refused, owner-stated: this board runs continuously and an uptime that
 restarts every 49.7 days is not acceptable.** That is what a 32-bit

src/bootloader.hpp

@@ -4,24 +4,12 @@
 
 #include "board.hpp"
 
-// Reaching the resident bootloader from the console.
-//
-// The legacy firmware did this with a watchdog reset: `bootloader` armed the
-// watchdog and hung, and the next boot noticed WDRF and jumped to the boot
-// section. That works for TinySafeBoot and **does not work for pureboot**, which
-// deliberately hands straight back to the application on WDRF - an unattended
-// board that watchdog-resets in a loop must not sit in a loader instead of
-// running. So a reset-based route into pureboot opens no window at all, and on a
-// board whose only way in is the firmware that is a lockout.
-//
-// This route therefore never resets. It jumps, with the reset flags already
-// clear, so the loader starts as if from a clean power-on and opens its window.
-//
-// The address is this board's, and it is not the legacy one: the loader lives in
-// the top 512 bytes at 0x7e00 (`hfuse d4` puts the boot section at 0x7c00 with
-// pureboot's staging slot below its own slot). The legacy firmware probed 0x7800
-// - a 2 KB boot section's base - which on this board reads erased, so its check
-// was always false and its `bootloader` command never actually arrived anywhere.
+// Reaching the resident bootloader from the console. pureboot hands straight
+// back to the application on WDRF - an unattended board that watchdog-resets in
+// a loop must not sit in a loader instead of running - so a reset-based route
+// opens no window at all, and on a board whose only way in is the firmware that
+// is a lockout. This route jumps instead, with the reset flags already clear,
+// so the loader starts as if from a clean power-on and opens its window.
 namespace app {
 
 class bootloader {
@@ -29,17 +17,10 @@ class bootloader {
 
 	using guard = dev::watchdog<{.timeout = 16_ms}>;
 
-	// The top 512 bytes. An erased slot reads 0xffff, which is not an
-	// instruction any loader begins with - so this asks "is a loader installed"
-	// rather than "is it the one I expect", which is the check the legacy
-	// firmware got wrong in the other direction by testing one specific byte.
+	// The top 512 bytes: `hfuse d4` puts the boot section at 0x7c00, with
+	// pureboot's staging slot below its own slot.
 	static constexpr std::uint16_t base = 0x7e00;
 
-	static bool present()
-	{
-		return avr::flash_load(reinterpret_cast<const std::uint16_t *>(base)) != 0xffff;
-	}
-
 	// A function pointer holds a word address on AVR, so the byte address
 	// halves. [[gnu::noipa]] keeps the call indirect: a constant target folds
 	// into a relative call that cannot reach across flash.
@@ -73,20 +54,12 @@ class bootloader {
 
 		// Release the USART. While TXEN0 is set the peripheral owns PD1, not the
 		// port register, so a loader that bit-bangs the same pin receives
-		// perfectly and answers into nothing - mute, not deaf, and unverifiable
-		// from the host. pureboot clears this itself; TinySafeBoot, which is what
-		// this board still carries, does not. Four bytes make the hand-over work
-		// for either one, which is the only reason this route can be tested
-		// before the loader is replaced.
+		// perfectly and answers into nothing. pureboot releases it itself; this
+		// keeps the hand-over correct for a loader that does not.
 		avr::hw::ucsr0b::write(0);
 
 		call(reinterpret_cast<jump_fn>(base / 2));
 	}
-
-	static bool available()
-	{
-		return present();
-	}
 };
 
 } // namespace app

src/controller.hpp

@@ -38,11 +38,11 @@ class controller {
 		fan::set_duty(avr::percent_t::of(m_percent));
 	}
 
-	static void set_manual(std::uint8_t p)
+	static void set_manual(std::uint8_t percent)
 	{
 		m_auto_mode = false;
-		m_percent = p;
-		fan::set_duty(avr::percent_t::of(p));
+		m_percent = percent;
+		fan::set_duty(avr::percent_t::of(percent));
 	}
 
 	static void set_automatic()

src/curve.hpp

@@ -6,7 +6,7 @@
 #include <libavr/flash.hpp>
 
 // The auto-mode fan curve, tabulated at compile time as a flash_table of duty
-// percent per C: a cubic that is held at zero below the temperature the fan
+// percent per degree C: a cubic that is held at zero below the temperature the fan
 // starts at.
 namespace app::curve {
 

src/statistics.hpp

@@ -7,7 +7,7 @@
 
 #include "board.hpp"
 
-// Temperature histogram: one uint32 bucket per C 0..99, sampled once a
+// Temperature histogram: one uint32 bucket per degree C 0..99, sampled once a
 // second, written back to EEPROM every 30 minutes (update() only touches
 // changed bytes).
 namespace app {

src/terminal.hpp

@@ -3,6 +3,7 @@
 #include <array>
 #include <cstdint>
 #include <string_view>
+#include <utility>
 
 #include <libavr/libavr.hpp>
 
@@ -18,6 +19,11 @@
 // until Ctrl+C, and Ctrl+C abandons a half-typed line anywhere else.
 namespace app {
 
+struct command {
+	std::string_view name;
+	std::string_view summary;
+};
+
 // Commands, in the order they are matched - which is the order the original
 // firmware matched them in, and that order is load-bearing. An abbreviation
 // resolves to the *first* entry it prefixes, so `s` is show (not statistics,
@@ -25,57 +31,71 @@ namespace app {
 // steal an abbreviation that already meant something else.
 //
 // A function rather than a variable, so the list exists only while the flash
-// blob below is being built and never as storage of its own.
-consteval auto command_names()
+// blobs below are being built and never as storage of its own.
+consteval auto commands()
 {
-	return std::to_array<std::string_view>({
-	    "help",
-	    "show",
-	    "curve",
-	    "monitor",
-	    "bootloader",
-	    "uptime",
-	    "statistics",
-	    "histogram",
-	    "reset",
-	    "set",
-	    "auto",
-	    "version",
-	    "save",
+	return std::to_array<command>({
+	    {"help", "prints this help message"},
+	    {"show", "shows current temperature and fan speed"},
+	    {"curve", "shows mapping from temperature to fan speed"},
+	    {"monitor", "loops the show command until Ctrl+C is pressed"},
+	    {"bootloader", "enters the bootloader"},
+	    {"uptime", "shows system uptime"},
+	    {"statistics", "prints overall statistics like min and max temp"},
+	    {"histogram", "prints a histogram of the temperature"},
+	    {"reset", "resets statistics to 0 in EEPROM and RAM (no abbreviation)"},
+	    {"set", "sets the fan speed to the provided value, 0-100"},
+	    {"auto", "turns on automatic fan control"},
+	    {"version", "displays firmware version"},
+	    {"save", "writes the statistics to EEPROM now"},
 	});
 }
 
+// Found by name rather than written as an index, so reordering the list cannot
+// point a command at another one's code. A name not in the list does not
+// compile.
+consteval std::uint8_t command_index(std::string_view name)
+{
+	const auto list = commands();
+	for (std::uint8_t i = 0; i < list.size(); ++i) {
+		if (list[i].name == name) {
+			return i;
+		}
+	}
+	std::unreachable();
+}
+
 class terminal {
 	static constexpr char ctrl_c = 0x03;
 	static constexpr char backspace = 0x08;
 	static constexpr char del = 0x7f;
+	static constexpr std::uint16_t monitor_interval_ms = 1'000;
 	static inline std::array<char, 24> m_line{};
 	static inline std::uint8_t m_at = 0;
 	static inline bool m_overflowed = false;
 	static inline bool m_monitoring = false;
 	static inline std::uint64_t m_last_monitor = 0;
 
-	static constexpr std::uint8_t command_count = command_names().size();
+	static constexpr std::uint8_t command_count = commands().size();
 
-	static constexpr std::size_t name_bytes = [] {
+	// One column of the command table as a NUL-separated blob in flash. Matched
+	// and printed at run time, which is not a reason to sit in RAM on a machine
+	// with two address spaces: a table of `string_view` puts the characters
+	// *and* its own pointers there. Separators rather than an offset table,
+	// because an offset table is the RAM this exists to give back.
+	template <std::string_view command::*Column>
+	static constexpr auto joined = [] {
+		constexpr auto bytes = [] {
 			std::size_t total = 0;
-		for (auto one : command_names()) {
-			total += one.size() + 1;
+			for (const auto &one : commands()) {
+				total += (one.*Column).size() + 1;
 			}
 			return total;
 		}();
-
-	// The names as one NUL-separated blob in flash. Matched at run time, which
-	// is not a reason to sit in RAM on a machine with two address spaces: a
-	// table of `string_view` puts the characters *and* its own pointers there,
-	// and on this part that was 216 B of 2 KB spent on data that never changes.
-	// Separators rather than an offset table, because an offset table is the
-	// RAM this exists to give back.
-	static constexpr auto names_data = [] {
-		std::array<char, name_bytes> chars{};
+		std::array<char, bytes> chars{};
 		std::size_t at = 0;
-		for (auto one : command_names()) {
-			for (char c : one) {
+		for (const auto &one : commands()) {
+			for (char c : one.*Column) {
 				chars[at++] = c;
 			}
 			chars[at++] = '\0';
@@ -83,103 +103,86 @@ class terminal {
 		return chars;
 	}();
 
-	static constexpr avr::flash_table<names_data> names{};
+	static constexpr avr::flash_table<joined<&command::name>> names{};
+	static constexpr avr::flash_table<joined<&command::summary>> summaries{};
 
 	// `reset` must be typed in full - an abbreviation must not be able to wipe
-	// data. Found in the list rather than written as an index, so reordering the
-	// commands cannot move the protection onto a different one.
-	static constexpr std::uint8_t exact_command = [] {
-		const auto list = command_names();
-		for (std::uint8_t i = 0; i < list.size(); ++i) {
-			if (list[i] == "reset") {
-				return i;
-			}
-		}
-		return static_cast<std::uint8_t>(list.size());
-	}();
+	// data.
+	static constexpr std::uint8_t exact_command = command_index("reset");
 
-	// Column the descriptions' colons line up in, counted from the start of the
-	// name. The longest name is `bootloader` at 10, so 12 leaves it a space and
-	// one dot - the original's layout exactly.
-	static constexpr std::uint8_t help_column = 12;
+	// One space and at least one dot after the longest name, the original's
+	// layout exactly.
+	static constexpr std::uint8_t help_column = [] {
+		std::size_t longest = 0;
+		for (const auto &one : commands()) {
+			longest = one.name.size() > longest ? one.name.size() : longest;
+		}
+		return static_cast<std::uint8_t>(longest + 2);
+	}();
 
 	static void prompt()
 	{
 		serial << "> "_P;
 	}
 
-	// `name ....: ` - the dotted label the original used everywhere it printed a
-	// list of things, which is what makes a column of values readable without
-	// counting spaces. Its width comes from the type, so the padding needs no
-	// hand-counted constant.
+	// The tail of `name ....: ` - the dotted label the original used everywhere
+	// it printed a list of things, which is what makes a column of values
+	// readable without counting spaces.
+	static void pad(std::uint8_t width, std::uint8_t column)
+	{
+		serial << ' ';
+		for (std::uint8_t i = width + 1; i < column; ++i) {
+			serial << '.';
+		}
+		serial << ": "_P;
+	}
+
 	template <typename Flash>
 	static void label(Flash text, std::uint8_t column)
 	{
-		serial << text << ' ';
-		for (auto i = Flash::size + 1; i < column; ++i) {
-			serial << '.';
-		}
-		serial << ": "_P;
+		serial << text;
+		pad(Flash::size, column);
 	}
 
-	// The same shape for a command name, whose width is not a type but the walk
-	// to its separator - so it is counted as it is printed rather than measured
-	// first, which would read the blob twice.
-	static void help_row(std::uint8_t which)
+	// Prints one entry of a blob, from `at` to its separator, and answers its
+	// width - counted as it is printed rather than measured first, which would
+	// read the blob twice.
+	template <typename Blob>
+	static std::uint8_t print_entry(Blob blob, std::uint16_t at)
 	{
-		auto at = name_start(which);
 		std::uint8_t width = 0;
-		for (char c; (c = names.at(at + width)) != '\0'; ++width) {
+		for (char c; (c = blob.at(at + width)) != '\0'; ++width) {
 			serial << c;
 		}
-		serial << ' ';
-		for (std::uint8_t i = width + 1; i < help_column; ++i) {
-			serial << '.';
-		}
-		serial << ": "_P;
+		return width;
 	}
 
-	// Quarter- C as a signed decimal with a two-digit fraction. The sign is taken
-	// off first: C++ gives a negative remainder for a negative dividend, so
-	// `(q % 4) * 25` on -40.25 C yields -25 and prints "-40.-25".
+	// Quarter-degrees as a signed decimal with a two-digit fraction. The sign is
+	// taken off first: C++ gives a negative remainder for a negative dividend, so
+	// the fraction of -40.25 C would print as "-40.-25".
 	static void temperature(std::int16_t quarters)
 	{
+		constexpr auto per_degree = thermistor::quarters_per_degree;
 		if (quarters < 0) {
 			serial << '-';
 		}
 		auto magnitude = static_cast<std::uint16_t>(quarters < 0 ? -quarters : quarters);
-		serial << magnitude / 4 << '.' << avr::dec<{.width = 2, .fill = '0'}>((magnitude % 4) * 25);
+		serial << magnitude / per_degree << '.'
+		       << avr::dec<{.width = 2, .fill = '0'}>((magnitude % per_degree) * (100 / per_degree));
 	}
 
 	static void help()
 	{
 		serial << "\r\nFanTemp "_P << version << " command overview\r\n"_P;
-		help_row(0);
-		serial << "prints this help message\r\n"_P;
-		help_row(1);
-		serial << "shows current temperature and fan speed\r\n"_P;
-		help_row(2);
-		serial << "shows mapping from temperature to fan speed\r\n"_P;
-		help_row(3);
-		serial << "loops the show command until Ctrl+C is pressed\r\n"_P;
-		help_row(4);
-		serial << "enters the bootloader\r\n"_P;
-		help_row(5);
-		serial << "shows system uptime\r\n"_P;
-		help_row(6);
-		serial << "prints overall statistics like min and max temp\r\n"_P;
-		help_row(7);
-		serial << "prints a histogram of the temperature\r\n"_P;
-		help_row(8);
-		serial << "resets statistics to 0 in EEPROM and RAM (no abbreviation)\r\n"_P;
-		help_row(9);
-		serial << "sets the fan speed to the provided value, 0-100\r\n"_P;
-		help_row(10);
-		serial << "turns on automatic fan control\r\n"_P;
-		help_row(11);
-		serial << "displays firmware version\r\n"_P;
-		help_row(12);
-		serial << "writes the statistics to EEPROM now\r\n"_P;
+		std::uint16_t name = 0;
+		std::uint16_t summary = 0;
+		for (std::uint8_t i = 0; i < command_count; ++i) {
+			const auto width = print_entry(names, name);
+			name += width + 1;
+			pad(width, help_column);
+			summary += print_entry(summaries, summary) + 1;
+			serial << "\r\n"_P;
+		}
 		serial << "commands may be abbreviated: 'up' is uptime\r\n"_P;
 	}
 
@@ -206,7 +209,7 @@ class terminal {
 			return;
 		}
 		label("ADC value"_P, reading_column);
-		serial << controller::last_adc() << " / 1023\r\n"_P;
+		serial << controller::last_adc() << " / "_P << thermistor::adc_full_scale << "\r\n"_P;
 
 		label("Resistance"_P, reading_column);
 		if (auto ohms = resistance(); ohms == open_circuit) {
@@ -247,9 +250,12 @@ class terminal {
 
 	static void print_uptime()
 	{
+		constexpr std::uint32_t minute = 60;
+		constexpr std::uint32_t hour = 60 * minute;
+		constexpr std::uint32_t day = 24 * hour;
 		auto seconds = static_cast<std::uint32_t>(uptime::millis() / 1000);
-		serial << seconds / 86400 << "d "_P << (seconds / 3600) % 24 << "h "_P << (seconds / 60) % 60 << "m "_P
-		       << seconds % 60 << "s\r\n"_P;
+		serial << seconds / day << "d "_P << seconds % day / hour << "h "_P << seconds % hour / minute << "m "_P
+		       << seconds % minute << "s\r\n"_P;
 	}
 
 	static void print_statistics()
@@ -303,24 +309,10 @@ class terminal {
 		}
 	}
 
-	// Where the given command's name begins in the blob. A walk over the
-	// separators, for the two callers that need to start at one name.
-	static constexpr std::uint8_t name_start(std::uint8_t which)
-	{
-		std::uint8_t at = 0;
-		for (std::uint8_t i = 0; i < which; ++i) {
-			while (names.at(at) != '\0') {
-				++at;
-			}
-			++at;
-		}
-		return at;
-	}
-
   public:
-	// Which command a word names, or `command_count` for none. The input
-	// matches when it is a non-empty prefix of a name; `reset` is the exception
-	// and must be typed in full.
+	// Which command a word names, or `no_command` for none. The input matches
+	// when it is a non-empty prefix of a name; `reset` is the exception and must
+	// be typed in full.
 	//
 	// One pass over the blob answers both, because the names sit in it in match
 	// order: each name is compared as it is walked, and the walk to its
@@ -365,8 +357,8 @@ class terminal {
 			return;
 		}
 
-		// Split on the first space with the (pointer, length) constructor rather
-		// than substr, which throws - see matches().
+		// Split on the first space with the (pointer, length) constructor:
+		// substr checks its bounds by throwing, which this build cannot.
 		const auto space = input.find(' ');
 		const auto word = space == std::string_view::npos ? input : std::string_view{input.data(), space};
 		const auto rest = space == std::string_view::npos
@@ -376,40 +368,38 @@ class terminal {
 			return;
 		}
 
-		const auto which = lookup(word);
-
-		switch (which) {
-		case 0:
+		switch (lookup(word)) {
+		case command_index("help"):
 			help();
 			return;
-		case 1:
+		case command_index("show"):
 			show();
 			return;
-		case 2:
+		case command_index("curve"):
 			print_curve();
 			return;
-		case 3:
+		case command_index("monitor"):
 			m_monitoring = true;
 			return;
-		case 4:
+		case command_index("bootloader"):
 			serial << "entering bootloader\r\n"_P;
 			statistics::save();
 			serial.drain();
 			bootloader::enter();
-		case 5:
+		case command_index("uptime"):
 			print_uptime();
 			return;
-		case 6:
+		case command_index("statistics"):
 			print_statistics();
 			return;
-		case 7:
+		case command_index("histogram"):
 			print_histogram();
 			return;
-		case 8:
+		case command_index("reset"):
 			statistics::reset();
 			serial << "statistics cleared in EEPROM and RAM\r\n"_P;
 			return;
-		case 9: {
+		case command_index("set"): {
 			std::uint16_t percent = 0;
 			bool valid = !rest.empty();
 			for (char c : rest) {
@@ -430,14 +420,14 @@ class terminal {
 			}
 			return;
 		}
-		case 10:
+		case command_index("auto"):
 			controller::set_automatic();
 			serial << "automatic fan control\r\n"_P;
 			return;
-		case 11:
+		case command_index("version"):
 			serial << "FanTemp "_P << version << " on libavr\r\n"_P;
 			return;
-		case 12:
+		case command_index("save"):
 			statistics::save();
 			serial << "statistics written to EEPROM\r\n"_P;
 			return;
@@ -473,7 +463,7 @@ class terminal {
 	static void poll()
 	{
 		if (m_monitoring) {
-			if (uptime::millis() >= m_last_monitor + 1000) {
+			if (uptime::millis() >= m_last_monitor + monitor_interval_ms) {
 				show();
 				m_last_monitor = uptime::millis();
 			}

src/thermistor.hpp

@@ -8,7 +8,7 @@
 // NTC thermistor on a series divider, solved entirely at compile time:
 // the Beta equation (logarithm and all) runs consteval into a libavr
 // flash_table - the firmware never does floating point. Raw 10-bit ADC
-// counts map to quarter- C with linear interpolation between table steps.
+// counts map to quarter-degrees C with linear interpolation between table steps.
 namespace app::thermistor {
 
 // The converter's top count: this board reads the divider at the ADC's
@@ -20,43 +20,58 @@ inline constexpr double nominal_resistance = 9270;
 inline constexpr double beta = 3212;
 inline constexpr double nominal_temperature = 25;
 
+inline constexpr std::uint8_t quarter_bits = 2;
+inline constexpr std::uint8_t quarters_per_degree = 1 << quarter_bits;
+
 namespace detail {
 
+inline constexpr double kelvin_offset = 273.15;
+
+// Signed-promoting, so the interpolation below divides a negative slope as one.
+inline constexpr std::uint8_t counts_per_step = 4;
+inline constexpr std::int32_t steps = (adc_full_scale + 1) / counts_per_step;
+
+// A sane sensor window, in whole degrees.
+inline constexpr std::int32_t coldest = -40;
+inline constexpr std::int32_t hottest = 125;
+
 consteval double temperature_of(double adc)
 {
 	double resistance = series_resistor * adc / (adc_full_scale - adc);
 	// __builtin_log constant-folds on the AVR backend, so the table is
 	// built at compile time with no runtime libm.
-	double steinhart = __builtin_log(resistance / nominal_resistance) / beta + 1.0 / (nominal_temperature + 273.15);
-	return 1.0 / steinhart - 273.15;
+	double steinhart =
+	    __builtin_log(resistance / nominal_resistance) / beta + 1.0 / (nominal_temperature + kelvin_offset);
+	return 1.0 / steinhart - kelvin_offset;
 }
 
-// 256 entries over the 10-bit range (steps of 4 counts), quarter- C,
-// clamped to a sane sensor window; entry 256 mirrors 255 so interpolation
-// at full scale has a right neighbour.
+// One entry per step, in quarter-degrees, with the reading kept a step clear of
+// both rails, where the divider has no solution.
 consteval std::int16_t quarters_entry(std::int32_t index)
 {
-	double adc = index * 4.0;
-	if (adc < 4) {
-		adc = 4;
+	double adc = index * double{counts_per_step};
+	if (adc < counts_per_step) {
+		adc = counts_per_step;
 	}
-	if (adc > 1019) {
-		adc = 1019;
+	if (adc > adc_full_scale - counts_per_step) {
+		adc = adc_full_scale - counts_per_step;
 	}
-	double t = temperature_of(adc) * 4.0;
-	if (t < -40 * 4) {
-		t = -40 * 4;
+	double t = temperature_of(adc) * quarters_per_degree;
+	if (t < coldest * quarters_per_degree) {
+		t = coldest * quarters_per_degree;
 	}
-	if (t > 125 * 4) {
-		t = 125 * 4;
+	if (t > hottest * quarters_per_degree) {
+		t = hottest * quarters_per_degree;
 	}
 	return static_cast<std::int16_t>(t < 0 ? t - 0.5 : t + 0.5);
 }
 
+// One entry past the last step, mirroring it, so interpolation at full scale
+// has a right neighbour.
 inline constexpr avr::flash_table<[] {
-	std::array<std::int16_t, 257> out{};
+	std::array<std::int16_t, steps + 1> out{};
 	for (std::int32_t i = 0; i < static_cast<std::int32_t>(out.size()); ++i) {
-		out[static_cast<std::size_t>(i)] = quarters_entry(i < 256 ? i : 255);
+		out[static_cast<std::size_t>(i)] = quarters_entry(i < steps ? i : steps - 1);
 	}
 	return out;
 }()>
@@ -64,14 +79,14 @@ inline constexpr avr::flash_table<[] {
 
 } // namespace detail
 
-// Temperature in quarter- C from a raw (or averaged) 10-bit sample.
+// Temperature in quarter-degrees C from a raw (or averaged) 10-bit sample.
 inline std::int16_t quarters(std::uint16_t adc)
 {
-	std::uint16_t index = adc >> 2; // the 257th entry backs index+1 at full scale
-	std::uint8_t frac = adc & 3;
+	std::uint16_t index = adc / detail::counts_per_step;
+	std::uint8_t frac = adc % detail::counts_per_step;
 	auto a = detail::table[index];
 	auto b = detail::table[static_cast<std::uint16_t>(index + 1)];
-	return static_cast<std::int16_t>(a + ((b - a) * frac) / 4);
+	return static_cast<std::int16_t>(a + ((b - a) * frac) / detail::counts_per_step);
 }
 
 // Whole degrees from quarter-degrees, rounded to nearest. The shift is the
@@ -79,7 +94,7 @@ inline std::int16_t quarters(std::uint16_t adc)
 // -3.00 C as -2, where an arithmetic shift floors and reads it as -3.
 inline constexpr std::int8_t whole_degrees(std::int16_t quarters)
 {
-	return static_cast<std::int8_t>((quarters + 2) >> 2);
+	return static_cast<std::int8_t>((quarters + quarters_per_degree / 2) >> quarter_bits);
 }
 
 } // namespace app::thermistor

test/CMakeLists.txt

@@ -2,7 +2,7 @@ libavr_format_test()
 
 # The image must stay below the boot section at 0x7e00, and the README states
 # what it measures; a library advance that moves it says nothing on its own.
-libavr_size_claim_test(fantemp 8004)
+libavr_size_claim_test(fantemp 7836)
 
 # The README says the two modes emit the same image, and only a tree with
 # both built can say whether they do.

test/check_reachability.py

@@ -6,25 +6,20 @@ bootloader is the running firmware's `bootloader` command, so a firmware that
 gets that route wrong is a board that cannot be reflashed - and the failure is
 silent, because everything else still works.
 
-It has been wrong before. The firmware this one replaces probed and jumped to
-`0x7800`, the base of a 2 KB boot section, while the board's loader sits at
-`0x7e00`; `check()` therefore read an erased byte, was false, and the command
-never arrived anywhere. Nothing about that is visible short of trying it on the
-hardware, which is what this replaces.
-
-Three properties, all read out of the disassembly rather than the source:
+Four properties, all read out of the disassembly rather than the source:
 
 1. The image ends below the boot section. `hfuse d4` puts that at 0x7c00, so an
    application reaching into it would be overwritten by the loader - or worse,
    executed at reset, since BOOTRST points there.
 2. The hand-over targets the loader base. A word address of 0x3f00 is byte
-   0x7e00; anything else is the 0x7800 bug again.
-3. The hand-over does not arm the watchdog. pureboot hands straight back on
-   WDRF by design, so a reset-based route reaches it and opens no window. The
-   legacy firmware's route was exactly that, and it is the one change that
-   cannot be walked back from the host.
+   0x7e00; 0x7800, a 2 KB boot section's base, reads erased on this board.
+3. The hand-over releases the USART first. While TXEN0 is set the peripheral
+   owns PD1, so a loader that bit-bangs the pin answers into nothing.
+4. The hand-over does not arm the watchdog. pureboot hands straight back on
+   WDRF by design, so a reset-based route reaches it and opens no window.
 
-    check_reachability.py --objdump avr-objdump --elf fantemp --image fantemp.bin
+    check_reachability.py --objdump avr-objdump --elf fantemp --image fantemp.bin \
+        --loader-base 0x7e00
 """
 
 from __future__ import annotations
@@ -36,6 +31,7 @@ import subprocess
 import sys
 
 BOOT_SECTION = 0x7C00  # hfuse d4: BOOTSZ 512 words - a fuse fact, not a loader one
+UCSR0B = 0xC1
 WDTCSR = 0x60
 
 
@@ -73,17 +69,21 @@ def main() -> int:
     # loads the *word* address into a register pair immediately before it.
     lines = text.splitlines()
     helper = re.compile(r"\b(?:r?call)\b.*<_ZN3app10bootloader4call")
-    sites = []
+    release = re.compile(rf"\bsts\s+0x00{UCSR0B:02X},\s*r1\b", re.I)
+    sites, unreleased = [], 0
     for index, line in enumerate(lines):
         if not helper.search(line):
             continue
         held: dict[str, int] = {}
-        for back in lines[max(0, index - 8):index]:
+        window = lines[max(0, index - 8):index]
+        for back in window:
             if m := re.search(r"\bldi\s+(r\d+),\s*0x([0-9A-Fa-f]{2})", back):
                 held[m.group(1)] = int(m.group(2), 16)
         # The AVR ABI passes the pointer in r25:r24.
         if "r24" in held and "r25" in held:
             sites.append(held["r25"] << 8 | held["r24"])
+        if not any(release.search(back) for back in window):
+            unreleased += 1
 
     want = loader_base // 2
     if not sites:
@@ -97,6 +97,12 @@ def main() -> int:
         print(f"  ok    all {len(sites)} hand-over site(s) target word 0x{want:04x} "
               f"(byte 0x{loader_base:04x})")
 
+    if unreleased:
+        failures.append(f"{unreleased} hand-over site(s) do not clear UCSR0B first - "
+                        f"the USART keeps PD1 and the loader answers into nothing")
+    elif sites:
+        print("  ok    every hand-over site clears UCSR0B before the jump")
+
     # An icall/ijmp has to exist for that address to be jumped to indirectly.
     if not re.search(r"\b(icall|ijmp)\b", text):
         failures.append("no icall/ijmp - the hand-over cannot reach across flash")

test/consteval.cpp

@@ -43,7 +43,7 @@ static_assert(duty_entry(99) == 100);
 // Monotone across the whole table: a warmer reading never asks for less air.
 consteval bool curve_rises()
 {
-	for (std::int32_t t = 1; t < 100; ++t) {
+	for (std::int32_t t = 1; t < static_cast<std::int32_t>(app::curve::detail::table.size()); ++t) {
 		if (duty_entry(t) < duty_entry(t - 1)) {
 			return false;
 		}
@@ -57,23 +57,25 @@ static_assert(curve_rises());
 // thermistor's nominal resistance at
 // adc = full_scale * nominal / (series + nominal), and the equation's own
 // definition puts that count at the nominal temperature.
+using app::thermistor::detail::counts_per_step;
 using app::thermistor::detail::quarters_entry;
+using app::thermistor::detail::steps;
 
 inline constexpr std::int32_t nominal_count =
     static_cast<std::int32_t>(app::thermistor::adc_full_scale * app::thermistor::nominal_resistance /
                               (app::thermistor::series_resistor + app::thermistor::nominal_resistance));
-static_assert(quarters_entry(nominal_count / 4) >= 100);    // 25.00 C, in quarters
-static_assert(quarters_entry(nominal_count / 4 + 1) < 100); // and the step below it
+static_assert(quarters_entry(nominal_count / counts_per_step) >= 100);    // 25.00 C, in quarters
+static_assert(quarters_entry(nominal_count / counts_per_step + 1) < 100); // and the step below it
 
 // Both clamps, at the ends the divider cannot leave.
 static_assert(quarters_entry(0) == 125 * 4);
-static_assert(quarters_entry(255) == -40 * 4);
+static_assert(quarters_entry(steps - 1) == -40 * 4);
 
 // An NTC on this divider falls with the count: more counts is more resistance
 // is a colder sensor, over every step of the table.
 consteval bool thermistor_falls()
 {
-	for (std::int32_t i = 1; i < 256; ++i) {
+	for (std::int32_t i = 1; i < steps; ++i) {
 		if (quarters_entry(i) > quarters_entry(i - 1)) {
 			return false;
 		}

tools/check.sh

@@ -1,11 +1,10 @@
 #!/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
-# is subtle and was got wrong by hand more than once - the mode-identity checks
-# read the sibling mode's tree, so a build-then-test-per-preset run compares a
+# The loop is libavr's (tools/check-presets.sh): the mode-identity checks read
+# the sibling mode's tree, so a build-then-test-per-preset run would compare a
 # fresh image against a stale sibling. Anything this repository needs beyond
-# the presets is written after the call, where every tree is built and tested.
+# the presets goes after the call, where every tree is built and tested.
 set -euo pipefail
 cd "$(dirname "${BASH_SOURCE[0]}")/.."
 "${LIBAVR_ROOT:-libavr}/tools/check-presets.sh" "$@"