deps: the pins advance to the identity check that reads its own sibling

libavr 93d8b0e, pureboot 26a8621. The check is handed the sibling build tree now and asks its build system whether the image is current, so a comparison against a tree the run did not build skips rather than passing. Gated at the pin. Its own gate builds both modes, so the comparison still happens.

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

.clang-format

@@ -1,13 +1,15 @@
 ---
 BasedOnStyle: LLVM
+Standard: Latest
 ColumnLimit: 120
 IndentWidth: 4
 TabWidth: 4
 UseTab: ForIndentation
 AlignEscapedNewlines: DontAlign
 AllowShortFunctionsOnASingleLine: Empty
-AlwaysBreakTemplateDeclarations: true
+BreakTemplateDeclarations: Yes
 BreakBeforeBraces: Custom
 BraceWrapping:
   AfterFunction: true
+InsertBraces: true
 ...

.clangd

@@ -0,0 +1,30 @@
+CompileFlags:
+  # Named here rather than in the editor's settings because nothing vendors
+  # this repo: the one build tree is the one an editor should read.
+  CompilationDatabase: build/atmega328p-generated
+  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.
+    - -D__uint24=unsigned long
+    - -D__int24=long
+    # clangd forwards the driver's system includes but not its own header
+    # directory, so <stdint.h> resolves to avr-libc's, which still gates the
+    # limit and constant macros on the C++98 opt-in.
+    - -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
+    # default limit of 19 inside the headers and truncate the parse.
+    - -ferror-limit=0
+  Remove:
+    # The build promotes warnings for the compiler that has to be right about
+    # them; in the editor the flag paints a second frontend's opinions in the
+    # colour reserved for things that do not compile.
+    - -Werror
+Diagnostics:
+  Suppress:
+    # clang's AVR `signal` attribute takes no arguments and it knows none of
+    # progmem, naked or OS_main. A misspelling is what the build is for.
+    - attribute_wrong_number_arguments
+    - unknown-attributes

.gitattributes

@@ -1,8 +1,11 @@
-*.h         eol=lf
+# Line endings are the repository's, not the editing machine's: this checkout
-*.hpp       eol=lf
+# is reached from two hosts, and a file rewritten by a Windows tool comes back
-*.c         eol=lf
+# with every line changed unless something says otherwise. Naming the source
-*.cpp       eol=lf
+# extensions left Markdown, Python, shell and CMake to whatever the writing
-.git*       eol=lf
+# 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

.gitignore

@@ -1,4 +1,5 @@
 build/
+local/
 .cache/
 
 # Atmel Studio: generated per machine, and its build outputs

.gitmodules

@@ -1,3 +1,6 @@
 [submodule "libavr"]
 	path = libavr
 	url = ../libavr.git
+[submodule "pureboot"]
+	path = pureboot
+	url = ../pureboot.git

.vscode/extensions.json

@@ -0,0 +1,6 @@
+{
+	"recommendations": [
+		"llvm-vs-code-extensions.vscode-clangd",
+		"ms-vscode.cmake-tools"
+	]
+}

.vscode/settings.json

@@ -0,0 +1,33 @@
+{
+	// clangd is the language server; the cpptools engine would parse every file
+	// a second time and disagree, since nothing tells it about a cross
+	// compiler. Which database to read is in .clangd.
+	"C_Cpp.intelliSenseEngine": "disabled",
+
+	// --query-driver lets clangd ask the cross compiler for its own system
+	// includes and target; without it every standard header is missing.
+	"clangd.arguments": [
+		"--query-driver=**avr-g++*",
+		"--header-insertion=never"
+	],
+
+	// 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
+	// named here**: a committed file may not name a path that is true of one
+	// machine (libavr guidance rule 50), so the gitignored local/machine.cmake
+	// 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,
+		"**/libavr/**": true
+	},
+
+	"files.associations": {
+		".clangd": "yaml",
+		".clang-format": "yaml"
+	}
+}

CMakeLists.txt

@@ -12,10 +12,40 @@ if(NOT LIBAVR_ROOT)
 	set(LIBAVR_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/libavr)
 endif()
 if(NOT EXISTS ${LIBAVR_ROOT}/CMakeLists.txt)
-	message(FATAL_ERROR "libavr not found at ${LIBAVR_ROOT} — run: git submodule update --init libavr")
+	message(FATAL_ERROR "libavr not found at ${LIBAVR_ROOT} - run: git submodule update --init libavr")
+endif()
+# pureboot rides as a pinned submodule too: this board's only way in is its
+# resident loader, so the commit that names this firmware names the loader it
+# has to hand over to. Consumed for the geometry it exports - the loader links
+# the libavr target above, so pureboot's own libavr submodule stays
+# uninitialised.
+if(NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/pureboot/CMakeLists.txt)
+	message(FATAL_ERROR "pureboot not found - run: git submodule update --init pureboot")
 endif()
 add_subdirectory(${LIBAVR_ROOT} libavr-build)
+add_subdirectory(pureboot pureboot-build)
+include(${LIBAVR_ROOT}/cmake/checks.cmake)
 
 add_executable(fantemp src/main.cpp)
 target_link_libraries(fantemp PRIVATE libavr)
-add_custom_command(TARGET fantemp POST_BUILD COMMAND ${CMAKE_SIZE} $<TARGET_FILE:fantemp>)
+# The raw image is what the loader takes, and what the reachability check
+# measures the boot-section clearance against.
+add_custom_command(TARGET fantemp POST_BUILD
+                   COMMAND ${CMAKE_SIZE} $<TARGET_FILE:fantemp>
+                   COMMAND ${CMAKE_OBJCOPY} -O binary -R .eeprom
+                           $<TARGET_FILE:fantemp> $<TARGET_FILE_DIR:fantemp>/fantemp.bin
+                   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.
+pureboot_add_loader(pureboot)
+
+enable_testing()
+add_subdirectory(test)

CMakePresets.json

@@ -1,43 +1,59 @@
-{
+{
-	"version": 8,
+	"version": 8,
-	"configurePresets": [
+	"configurePresets": [
-		{
+		{
-			"name": "base",
+			"name": "base",
-			"hidden": true,
+			"hidden": true,
-			"generator": "Ninja",
+			"generator": "Ninja",
-			"binaryDir": "${sourceDir}/build/${presetName}",
+			"binaryDir": "${sourceDir}/build/${presetName}",
-			"toolchainFile": "${sourceDir}/libavr/cmake/avr-toolchain.cmake",
+			"toolchainFile": "${sourceDir}/libavr/cmake/avr-toolchain.cmake",
-			"cacheVariables": {
+			"cacheVariables": {
-				"CMAKE_BUILD_TYPE": "Release",
+				"CMAKE_BUILD_TYPE": "Release",
-				"CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
+				"CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
-				"CMAKE_COLOR_DIAGNOSTICS": "ON"
+				"CMAKE_COLOR_DIAGNOSTICS": "ON"
-			}
+			}
-		},
+		},
-		{
+		{
-			"name": "atmega328p-generated",
+			"name": "atmega328p-generated",
-			"inherits": "base",
+			"inherits": "base",
-			"cacheVariables": {
+			"cacheVariables": {
-				"LIBAVR_MCU": "atmega328p",
+				"LIBAVR_MCU": "atmega328p",
-				"LIBAVR_REFLECT": "OFF"
+				"LIBAVR_REFLECT": "OFF"
-			}
+			}
-		},
+		},
-		{
+		{
-			"name": "atmega328p-reflect",
+			"name": "atmega328p-reflect",
-			"inherits": "base",
+			"inherits": "base",
-			"cacheVariables": {
+			"cacheVariables": {
-				"LIBAVR_MCU": "atmega328p",
+				"LIBAVR_MCU": "atmega328p",
-				"LIBAVR_REFLECT": "ON"
+				"LIBAVR_REFLECT": "ON"
-			}
+			}
-		}
+		}
-	],
+	],
-	"buildPresets": [
+	"buildPresets": [
-		{
+		{
-			"name": "atmega328p-generated",
+			"name": "atmega328p-generated",
-			"configurePreset": "atmega328p-generated"
+			"configurePreset": "atmega328p-generated"
-		},
+		},
-		{
+		{
-			"name": "atmega328p-reflect",
+			"name": "atmega328p-reflect",
-			"configurePreset": "atmega328p-reflect"
+			"configurePreset": "atmega328p-reflect"
-		}
+		}
-	]
+	],
-}
+	"testPresets": [
+		{
+			"name": "atmega328p-generated",
+			"configurePreset": "atmega328p-generated",
+			"output": {
+				"outputOnFailure": true
+			}
+		},
+		{
+			"name": "atmega328p-reflect",
+			"configurePreset": "atmega328p-reflect",
+			"output": {
+				"outputOnFailure": true
+			}
+		}
+	]
+}

README.md

@@ -1,11 +1,55 @@
 # fantemp
 
-Temperature-controlled fan firmware (ATmega328P, 16 MHz), rewritten on
+**v2.2.** Temperature-controlled fan firmware (ATmega328P, 16 MHz), rewritten on
 [libavr](https://git.blackmark.me/avr/libavr): thermistor on ADC0 sampled
 free-running and averaged over 1000 conversions, fan on OC0B at 50 kHz,
 115200 Bd serial console (`help` lists the commands), temperature
-histogram persisted to EEPROM, watchdog-reset path into a boot-section
+histogram persisted to EEPROM, and a direct jump into a boot-section
-bootloader.
+bootloader at `0x7e00`.
+
+The EEPROM format is the legacy firmware's, unchanged: 100 little-endian
+`uint32` buckets at address 0, one per °C. A board carrying years of history
+from FanTemp 1.8b keeps every count — verified on hardware, all 67 non-empty
+buckets byte-identical across the conversion.
+
+## The console
+
+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
+minutes and on the way into the bootloader.
+
+`show`, `statistics` and the histogram print one value per line behind a dotted
+label, the way the original did — a run-on line is fine for one reading and
+unreadable when `monitor` emits one a second. `curve` walks every whole degree
+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.
+
+## Reaching the bootloader
+
+`bootloader` **jumps**; it does not reset. That is not a style choice:
+
+- **pureboot hands straight back on WDRF**, by design — an unattended board that
+  watchdog-resets in a loop must not sit in a loader. So the legacy
+  watchdog-reset hand-over arrives and opens no window at all, and on a board
+  with no reset line that is a board that cannot be reflashed.
+- The address is `0x7e00`, the top 512 bytes. The legacy firmware used `0x7800`,
+  a 2 KB boot section's base, which on a board with a 512-byte boot section reads
+  erased — so its `bootloader` command silently never arrived anywhere.
+- `UCSR0B` is cleared first. While `TXEN0` is set the USART owns PD1, so a loader
+  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.
 
 The Steinhart–Hart math of the legacy firmware (runtime doubles + libm
 log) is gone: the Beta equation and the cubic fan curve are evaluated
@@ -22,6 +66,9 @@ cmake --preset atmega328p-generated
 cmake --build --preset atmega328p-generated
 ```
 
+The firmware is **8004 B** of flash, byte-identical between the generated and
+reflect modes, and `ctest` holds it to that number.
+
 ## Atmel Studio
 
 `master` carries a Studio solution, so this branch does too: `ide/fantemp.atsln`

dev/tasks.md

@@ -0,0 +1,40 @@
+# Tasks
+
+This file is current work. The repo is the sole task tracker (libavr guidance
+rule 20).
+
+## Open
+
+Nothing.
+
+## Decided against
+
+Both of these are measured, and both are recorded here rather than deleted so
+that the next person to measure them does not read a number as an opportunity
+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.
+
+**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
+millisecond counter wraps at, and the display is not the only cost - the
+interval tests would have to become subtractions, since `now - last >=
+interval` survives a wrap where `now >= last + interval` does not, and
+`terminal.hpp`'s monitor tick is written the second way. The 64-bit counter is
+what puts the wrap out of reach, which is the property being bought.
+
+### The 1 kHz tick keeps its 64-bit increment
+
+The compare handler increments 64 bits, so it calls libgcc's `__adddi3_s8`, and
+a call inside a signal handler decides the prologue - twelve push/pop pairs for
+what the helper might clobber. Splitting the counter into two 32-bit halves
+removes the call and keeps the full range, taking the handler from ~47
+instructions to ~26 with the carry running once every 49.7 days.
+
+**Refused: it costs 66 B of flash to buy about 0.4 % of the CPU**, and nothing
+here is timing-critical. It was only ever worth considering alongside the
+narrowing above, which is refused outright.

src/board.hpp

@@ -3,7 +3,7 @@
 #include <libavr/libavr.hpp>
 
 // The board composition: every peripheral of the fan controller in one
-// place. ATmega328P at 16 MHz — thermistor divider on ADC0 (PC0), fan on
+// place. ATmega328P at 16 MHz - thermistor divider on ADC0 (PC0), fan on
 // OC0B (PD5) at 50 kHz, console on the hardware UART.
 namespace app {
 
@@ -13,56 +13,67 @@ using dev = avr::device<{.clock = 16_MHz}>;
 
 // Millisecond uptime from timer2 CTC (the fan owns timer0).
 class uptime {
-	static inline volatile std::uint64_t ms = 0;
+	static inline volatile std::uint64_t m_ms = 0;
 
   public:
-	using ticker = dev::timer2<{.frequency = 1_kHz, .on_compare = [] { ms = ms + 1; }}>;
+	using ticker = dev::timer2<{.frequency = 1_kHz, .on_compare = [] { m_ms = m_ms + 1; }}>;
 
 	static std::uint64_t millis()
 	{
-		avr::irq::atomic_guard lock;
+		avr::irq::interrupt_guard lock;
-		return ms;
+		return m_ms;
 	}
 };
 
 // 1000-sample averaging window fed by the conversion interrupt.
 class sampler {
-	static inline volatile std::uint32_t sum = 0;
+	static inline volatile std::uint32_t m_sum = 0;
-	static inline volatile std::uint16_t count = 0;
+	static inline volatile std::uint16_t m_count = 0;
-	static inline volatile std::uint16_t window = 0;
+	static inline volatile std::uint16_t m_window = 0;
-	static inline volatile bool ready = false;
+	static inline volatile bool m_ready = false;
 
 	static constexpr std::uint16_t samples = 1000;
 
   public:
-	using input = dev::adc<{.input = avr::adc::input_pin(0),
+	using input = dev::adc<{.trigger = avr::adc::trigger::free_running,
-	                        .trigger = avr::adc::trigger::free_running,
+	                        .on_conversion =
-	                        .on_conversion = [](std::uint16_t value) {
+	                            [](std::uint16_t value) {
-		                        sum = sum + value;
+		                            m_sum = m_sum + value;
-		                        count = count + 1;
+		                            m_count = m_count + 1;
-		                        if (count >= samples) {
+		                            if (m_count >= samples) {
-			                        window = static_cast<std::uint16_t>(sum / samples);
+			                            m_window = static_cast<std::uint16_t>(m_sum / samples);
-			                        sum = 0;
+			                            m_sum = 0;
-			                        count = 0;
+			                            m_count = 0;
-			                        ready = true;
+			                            m_ready = true;
-		                        }
+		                            }
-	                        }}>;
+	                            }},
+	                       avr::adc::input<avr::adc::input_pin(0)>>;
+	// The bound input, whose start() is free-running's one kick.
+	using thermistor = input::in<avr::adc::input_pin(0)>;
 
 	// The finished average (raw 10-bit), once per window.
 	static bool take(std::uint16_t &value)
 	{
-		avr::irq::atomic_guard lock;
+		avr::irq::interrupt_guard lock;
-		if (!ready)
+		if (!m_ready) {
 			return false;
-		value = window;
+		}
-		ready = false;
+		value = m_window;
+		m_ready = false;
 		return true;
 	}
 };
 
 using fan = dev::pwm<avr::pd5, {.frequency = 50_kHz}>;
 
-using serial_t = dev::uart0<{.baud = 115200_Bd, .rx_buffer = 32, .max_baud_error = 2.5_pct}>;
+// 115200 at 16 MHz lands +2.1 % off, past the receiver-tolerance table the
+// solver holds rates to. It is the rate the console on the other end of the
+// cable expects, so the override states that the miss is meant.
+using serial_t = dev::uart0<{
+    .baud = 115200_Bd,
+    .rx_buffer = 32,
+    .allow_baud_error = true,
+}>;
 inline constexpr serial_t serial{};
 
 } // namespace app

src/bootloader.hpp

@@ -1,15 +1,27 @@
 #pragma once
 
-#include <avr/pgmspace.h>
-
 #include <libavr/libavr.hpp>
 
 #include "board.hpp"
 
-// Reset-into-bootloader: `bootloader` on the console arms the watchdog
+// Reaching the resident bootloader from the console.
-// and hangs; the next boot sees WDRF and jumps to the boot section at
+//
-// 0x7800 (byte address) — if one is flashed there — before anything else
+// The legacy firmware did this with a watchdog reset: `bootloader` armed the
-// runs.
+// 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.
 namespace app {
 
 class bootloader {
@@ -17,28 +29,63 @@ 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.
+	static constexpr std::uint16_t base = 0x7e00;
+
 	static bool present()
 	{
-		return pgm_read_byte(0x7800) != 0xff;
+		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.
+	[[gnu::noipa, noreturn]] static void call(jump_fn target)
+	{
+		target();
+		__builtin_unreachable();
 	}
 
   public:
-	// Call first thing in main: reset_cause() clears MCUSR (a lingering
+	// Call first thing in main. Clearing MCUSR is the whole job: a lingering
-	// WDRF would re-arm the watchdog), then a watchdog reset diverts into
+	// WDRF forces the watchdog back on at its shortest timeout, which is a
-	// the bootloader when one is flashed.
+	// reset loop rather than a boot. The cause is not routed on - pureboot
+	// peeks WDRF without clearing it and hands back on purpose, so a watchdog
+	// reset arrives here with the flag still set, and diverting into the
+	// loader on it would reopen the very window that policy closes.
 	static void handle_reset()
 	{
-		auto cause = avr::power::reset_cause();
+		avr::power::consume_reset_cause();
 		guard::disable();
-		if (cause.watchdog && present())
-			reinterpret_cast<jump_fn>(0x7800 / 2)();
 	}
 
+	// Hand over for real: no reset, so no WDRF for the loader to refuse.
 	[[noreturn]] static void enter()
 	{
-		guard::init();
+		// Interrupts first - the receive vector and the timer live in this
-		while (true) {
+		// application's vector table, and once the loader is running there is no
-		}
+		// application to vector into.
+		avr::irq::disable();
+		guard::disable();
+
+		// 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.
+		avr::hw::ucsr0b::write(0);
+
+		call(reinterpret_cast<jump_fn>(base / 2));
+	}
+
+	static bool available()
+	{
+		return present();
 	}
 };
 

src/controller.hpp

@@ -6,71 +6,73 @@
 #include "curve.hpp"
 #include "thermistor.hpp"
 
-// Control loop: averaged thermistor samples → temperature → fan duty
+// Control loop: averaged thermistor samples -> temperature -> fan duty
 // through the curve table (auto) or a console-set value (manual).
 namespace app {
 
 class controller {
-	static inline std::uint16_t adc_average = 0;
+	static inline std::uint16_t m_adc_average = 0;
-	static inline std::int16_t temp_quarters = 0;
+	static inline std::int16_t m_temp_quarters = 0;
-	static inline std::uint8_t percent = 100;
+	static inline std::uint8_t m_percent = 100;
-	static inline bool auto_mode = true;
+	static inline bool m_auto_mode = true;
-	static inline bool have_data = false;
+	static inline bool m_have_data = false;
 
   public:
 	static void init()
 	{
-		fan::duty(avr::percent_t{10000}); // full blast until the first reading
+		fan::set_duty(100_pct); // full blast until the first reading
 	}
 
 	static void poll()
 	{
 		std::uint16_t sample;
-		if (!sampler::take(sample))
+		if (!sampler::take(sample)) {
 			return;
-		adc_average = sample;
+		}
-		temp_quarters = thermistor::quarters(sample);
+		m_adc_average = sample;
-		have_data = true;
+		m_temp_quarters = thermistor::quarters(sample);
-		if (auto_mode)
+		m_have_data = true;
-			percent = curve::duty(static_cast<std::int8_t>((temp_quarters + 2) / 4));
+		if (m_auto_mode) {
-		fan::duty(avr::percent_t{static_cast<std::uint16_t>(percent * 100)});
+			m_percent = curve::duty(thermistor::whole_degrees(m_temp_quarters));
+		}
+		fan::set_duty(avr::percent_t::of(m_percent));
 	}
 
 	static void set_manual(std::uint8_t p)
 	{
-		auto_mode = false;
+		m_auto_mode = false;
-		percent = p;
+		m_percent = p;
-		fan::duty(avr::percent_t{static_cast<std::uint16_t>(p * 100)});
+		fan::set_duty(avr::percent_t::of(p));
 	}
 
 	static void set_automatic()
 	{
-		auto_mode = true;
+		m_auto_mode = true;
 	}
 
 	static bool automatic()
 	{
-		return auto_mode;
+		return m_auto_mode;
 	}
 
 	static bool data_available()
 	{
-		return have_data;
+		return m_have_data;
 	}
 
 	static std::int16_t temperature_quarters()
 	{
-		return temp_quarters;
+		return m_temp_quarters;
 	}
 
 	static std::uint16_t last_adc()
 	{
-		return adc_average;
+		return m_adc_average;
 	}
 
 	static std::uint8_t fan_percent()
 	{
-		return percent;
+		return m_percent;
 	}
 };
 

src/curve.hpp

@@ -5,43 +5,55 @@
 
 #include <libavr/flash.hpp>
 
-// The auto-mode fan curve, tabulated at compile time: a cubic in °C
+// The auto-mode fan curve, tabulated at compile time as a flash_table of duty
-// (0.002246·x³ − 0.09·x² + 0.91·x, zero below 20 °C) as a flash_table of
+// percent per C: a cubic that is held at zero below the temperature the fan
-// duty percent per °C.
+// starts at.
 namespace app::curve {
 
+inline constexpr double cubic_term = 0.002246;
+inline constexpr double square_term = -0.09;
+inline constexpr double linear_term = 0.91;
+inline constexpr std::int8_t start_celsius = 20;
+
 namespace detail {
 
-consteval std::uint8_t duty_entry(int celsius)
+consteval std::uint8_t duty_entry(std::int32_t celsius)
 {
 	double x = celsius;
-	if (x < 20)
+	if (x < start_celsius) {
 		return 0;
-	double duty = 0.002246 * x * x * x - 0.09 * x * x + 0.91 * x;
+	}
-	if (duty < 0)
+	double duty = cubic_term * x * x * x + square_term * x * x + linear_term * x;
+	if (duty < 0) {
 		duty = 0;
-	if (duty > 100)
+	}
+	if (duty > 100) {
 		duty = 100;
+	}
 	return static_cast<std::uint8_t>(duty + 0.5);
 }
 
 inline constexpr avr::flash_table<[] {
 	std::array<std::uint8_t, 100> out{};
-	for (int t = 0; t < 100; ++t)
+	for (std::int32_t t = 0; t < static_cast<std::int32_t>(out.size()); ++t) {
 		out[static_cast<std::size_t>(t)] = duty_entry(t);
+	}
 	return out;
 }()>
     table;
 
 } // namespace detail
 
-// Duty percent for a temperature (clamped to the 0..99 °C table window).
+// Duty percent for a temperature, clamped to the table's own window.
 inline std::uint8_t duty(std::int8_t celsius)
 {
-	if (celsius < 0)
+	constexpr auto highest = static_cast<std::int8_t>(detail::table.size() - 1);
+	if (celsius < 0) {
 		celsius = 0;
-	if (celsius > 99)
+	}
-		celsius = 99;
+	if (celsius > highest) {
+		celsius = highest;
+	}
 	return detail::table[static_cast<std::uint8_t>(celsius)];
 }
 

src/main.cpp

@@ -5,6 +5,7 @@
 #include "controller.hpp"
 #include "statistics.hpp"
 #include "terminal.hpp"
+#include "thermistor.hpp"
 
 using namespace app;
 
@@ -16,14 +17,15 @@ int main()
 
 	avr::init<uptime::ticker, sampler::input, fan, serial_t, statistics>();
 	avr::irq::enable();
-	sampler::input::start();
+	sampler::thermistor::start();
 	controller::init();
 	terminal::init();
 
 	while (true) {
 		controller::poll();
-		if (controller::data_available())
+		if (controller::data_available()) {
-			statistics::record(static_cast<std::int8_t>((controller::temperature_quarters() + 2) / 4));
+			statistics::record(thermistor::whole_degrees(controller::temperature_quarters()));
+		}
 		terminal::poll();
 	}
 }

src/statistics.hpp

@@ -7,9 +7,9 @@
 
 #include "board.hpp"
 
-// Temperature histogram: one uint32 bucket per °C 0..99, sampled once a
+// Temperature histogram: one uint32 bucket per C 0..99, sampled once a
 // second, written back to EEPROM every 30 minutes (update() only touches
-// changed bytes). Erased EEPROM reads back as 0xffffffff — treated as 0.
+// changed bytes).
 namespace app {
 
 class statistics {
@@ -17,11 +17,15 @@ class statistics {
 	static constexpr std::uint32_t sample_delay_ms = 1'000;
 	static constexpr std::uint32_t writeback_delay_ms = 1'800'000;
 
+	// What an erased cell reads back as, so a bucket nobody has written yet
+	// counts as no samples rather than four billion.
+	static constexpr std::uint32_t never_written = ~std::uint32_t{0};
+
 	using stored = avr::eeprom::var<std::array<std::uint32_t, range>, 0>;
 
-	static inline std::array<std::uint32_t, range> histogram{};
+	static inline std::array<std::uint32_t, range> m_histogram{};
-	static inline std::uint64_t last_sample = 0;
+	static inline std::uint64_t m_last_sample = 0;
-	static inline std::uint64_t last_writeback = 0;
+	static inline std::uint64_t m_last_writeback = 0;
 
 	static constexpr std::uint8_t clamp(std::int8_t t)
 	{
@@ -33,72 +37,81 @@ class statistics {
 
 	static void init()
 	{
-		histogram = stored::read();
+		m_histogram = stored::read();
-		for (auto &bucket : histogram)
+		for (auto &bucket : m_histogram) {
-			if (bucket == 0xffffffff)
+			if (bucket == never_written) {
 				bucket = 0;
+			}
+		}
 	}
 
 	static void record(std::int8_t celsius)
 	{
 		auto now = uptime::millis();
-		if (now >= last_sample + sample_delay_ms) {
+		if (now >= m_last_sample + sample_delay_ms) {
-			++histogram[clamp(celsius)];
+			++m_histogram[clamp(celsius)];
-			last_sample = now;
+			m_last_sample = now;
 		}
-		if (now >= last_writeback + writeback_delay_ms) {
+		if (now >= m_last_writeback + writeback_delay_ms) {
 			save();
-			last_writeback = now;
+			m_last_writeback = now;
 		}
 	}
 
 	static void save()
 	{
-		stored::update(histogram);
+		stored::update(m_histogram);
 	}
 
 	static void reset()
 	{
-		histogram = {};
+		m_histogram = {};
-		stored::update(histogram);
+		stored::update(m_histogram);
 	}
 
 	static std::uint8_t min_temperature()
 	{
-		for (std::uint8_t i = 0; i < range; ++i)
+		for (std::uint8_t i = 0; i < range; ++i) {
-			if (histogram[i])
+			if (m_histogram[i]) {
 				return i;
+			}
+		}
 		return range;
 	}
 
 	static std::uint8_t max_temperature()
 	{
-		for (std::uint8_t i = range; i > 0; --i)
+		for (std::uint8_t i = range; i > 0; --i) {
-			if (histogram[i - 1])
+			if (m_histogram[i - 1]) {
 				return i - 1;
+			}
+		}
 		return 0;
 	}
 
 	static std::uint64_t total_samples()
 	{
 		std::uint64_t total = 0;
-		for (auto bucket : histogram)
+		for (auto bucket : m_histogram) {
 			total += bucket;
+		}
 		return total;
 	}
 
 	static std::uint32_t highest_bucket()
 	{
 		std::uint32_t highest = 0;
-		for (auto bucket : histogram)
+		for (auto bucket : m_histogram) {
-			if (bucket > highest)
+			if (bucket > highest) {
 				highest = bucket;
+			}
+		}
 		return highest;
 	}
 
 	static std::uint32_t bucket(std::uint8_t celsius)
 	{
-		return histogram[clamp(static_cast<std::int8_t>(celsius))];
+		return m_histogram[clamp(static_cast<std::int8_t>(celsius))];
 	}
 };
 

src/terminal.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <array>
 #include <cstdint>
 #include <string_view>
 
@@ -10,39 +11,238 @@
 #include "controller.hpp"
 #include "curve.hpp"
 #include "statistics.hpp"
+#include "thermistor.hpp"
 
 // The serial console: line-buffered commands over the hardware UART.
-// `help` lists everything; `monitor` streams until any key.
+// `help` lists everything, command names may be abbreviated, `monitor` streams
+// until Ctrl+C, and Ctrl+C abandons a half-typed line anywhere else.
 namespace app {
 
+// 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,
+// not set) exactly as it always was, and anything appended to this list cannot
+// 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()
+{
+	return std::to_array<std::string_view>({
+	    "help",
+	    "show",
+	    "curve",
+	    "monitor",
+	    "bootloader",
+	    "uptime",
+	    "statistics",
+	    "histogram",
+	    "reset",
+	    "set",
+	    "auto",
+	    "version",
+	    "save",
+	});
+}
+
 class terminal {
-	static constexpr std::uint8_t line_max = 24;
+	static constexpr char ctrl_c = 0x03;
-	static inline char line[line_max]{};
+	static constexpr char backspace = 0x08;
-	static inline std::uint8_t at = 0;
+	static constexpr char del = 0x7f;
-	static inline bool monitoring = false;
+	static inline std::array<char, 24> m_line{};
-	static inline std::uint64_t last_monitor = 0;
+	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::size_t name_bytes = [] {
+		std::size_t total = 0;
+		for (auto one : command_names()) {
+			total += one.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::size_t at = 0;
+		for (auto one : command_names()) {
+			for (char c : one) {
+				chars[at++] = c;
+			}
+			chars[at++] = '\0';
+		}
+		return chars;
+	}();
+
+	static constexpr avr::flash_table<names_data> names{};
+
+	// `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());
+	}();
+
+	// 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;
 
 	static void prompt()
 	{
 		serial << "> "_P;
 	}
 
-	static void show()
+	// `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.
+	template <typename Flash>
+	static void label(Flash text, std::uint8_t column)
 	{
-		serial << "temperature "_P << controller::temperature_quarters() / 4 << '.'
+		serial << text << ' ';
-		       << (controller::temperature_quarters() % 4) * 25 << " C, adc "_P << controller::last_adc() << ", fan "_P
+		for (auto i = Flash::size + 1; i < column; ++i) {
-		       << controller::fan_percent() << " %, "_P;
+			serial << '.';
-		if (controller::automatic())
+		}
-			serial << "auto"_P;
+		serial << ": "_P;
-		else
-			serial << "manual"_P;
-		serial << "\r\n"_P;
 	}
 
+	// 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)
+	{
+		auto at = name_start(which);
+		std::uint8_t width = 0;
+		for (char c; (c = names.at(at + width)) != '\0'; ++width) {
+			serial << c;
+		}
+		serial << ' ';
+		for (std::uint8_t i = width + 1; i < help_column; ++i) {
+			serial << '.';
+		}
+		serial << ": "_P;
+	}
+
+	// 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".
+	static void temperature(std::int16_t quarters)
+	{
+		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);
+	}
+
+	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;
+		serial << "commands may be abbreviated: 'up' is uptime\r\n"_P;
+	}
+
+	// The thermistor's resistance from the divider, in whole ohms. The original
+	// printed this beside the reading and it is the one number that says *why* a
+	// temperature is wrong: an open sensor rails the ADC and the resistance goes
+	// to the tens of megohms, a shorted one to zero.
+	static std::uint32_t resistance()
+	{
+		auto adc = controller::last_adc();
+		if (adc >= thermistor::adc_full_scale) {
+			return open_circuit;
+		}
+		return static_cast<std::uint32_t>(thermistor::series_resistor) * adc / (thermistor::adc_full_scale - adc);
+	}
+
+	// One value per line behind a dotted label, as the original had it. A single
+	// run-on line is fine for one reading and unreadable when `monitor` prints
+	// one a second forever.
+	static void show()
+	{
+		if (!controller::data_available()) {
+			serial << "no data yet\r\n"_P;
+			return;
+		}
+		label("ADC value"_P, reading_column);
+		serial << controller::last_adc() << " / 1023\r\n"_P;
+
+		label("Resistance"_P, reading_column);
+		if (auto ohms = resistance(); ohms == open_circuit) {
+			serial << "open circuit\r\n"_P;
+		} else {
+			serial << ohms << " Ohm\r\n"_P;
+		}
+
+		label("Temperature"_P, reading_column);
+		temperature(controller::temperature_quarters());
+		serial << " C\r\n"_P;
+
+		label("Fan speed"_P, reading_column);
+		serial << controller::fan_percent() << "% "_P;
+		if (controller::automatic()) {
+			serial << "auto\r\n"_P;
+		} else {
+			serial << "manual\r\n"_P;
+		}
+	}
+
+	// Every whole degree from 10 to 60 with a bar, which is the original's and is
+	// the point of the command: the curve is a cubic, and five-degree samples
+	// without a graph show none of its shape. The bar is the duty itself, so it
+	// reads as a percentage without needing a scale.
 	static void print_curve()
 	{
-		for (std::int8_t t = 15; t <= 60; t += 5)
+		for (std::uint8_t t = curve_low; t <= curve_high; ++t) {
-			serial << t << " C -> "_P << curve::duty(t) << " %\r\n"_P;
+			auto duty = curve::duty(static_cast<std::int8_t>(t));
+			serial << avr::dec<{.width = 2, .fill = '0'}>(t) << " C = "_P << avr::dec<{.width = 3, .fill = ' '}>(duty)
+			       << "% |"_P;
+			for (std::uint8_t i = 0; i < duty; ++i) {
+				serial << '#';
+			}
+			serial << "\r\n"_P;
+		}
 	}
 
 	static void print_uptime()
@@ -54,117 +254,275 @@ class terminal {
 
 	static void print_statistics()
 	{
-		serial << "min "_P << statistics::min_temperature() << " C, max "_P << statistics::max_temperature()
+		auto empty = statistics::total_samples() == 0;
-		       << " C, samples "_P << static_cast<std::uint32_t>(statistics::total_samples()) << "\r\n"_P;
+		label("Minimum temperature"_P, stat_column);
+		if (empty) {
+			serial << "not available\r\n"_P;
+		} else {
+			serial << statistics::min_temperature() << " C\r\n"_P;
+		}
+
+		label("Maximum temperature"_P, stat_column);
+		if (empty) {
+			serial << "not available\r\n"_P;
+		} else {
+			serial << statistics::max_temperature() << " C\r\n"_P;
+		}
+
+		label("Total samples"_P, stat_column);
+		serial << static_cast<std::uint32_t>(statistics::total_samples()) << "\r\n"_P;
 	}
 
 	static void print_histogram()
 	{
 		auto highest = statistics::highest_bucket();
 		if (highest == 0) {
-			serial << "empty\r\n"_P;
+			serial << "no data yet\r\n"_P;
 			return;
 		}
+		// The original's normalisation, and its resolution: divide by whatever
+		// makes the tallest bucket fit in a hundred columns, not forty. A bar
+		// that tops out at 40 throws away most of the difference between
+		// neighbouring buckets, which on a distribution this narrow is the whole
+		// picture.
+		std::uint32_t factor = highest / bar_max > 1 ? highest / bar_max : 1;
+		while (highest / factor > bar_max) {
+			++factor;
+		}
+
 		for (std::uint8_t t = statistics::min_temperature(); t <= statistics::max_temperature(); ++t) {
-			serial << t << " C |"_P;
+			auto count = statistics::bucket(t);
-			auto width = static_cast<std::uint8_t>((statistics::bucket(t) * 40) / highest);
+			// Count first, in a fixed column, so the numbers read as a table
-			for (std::uint8_t i = 0; i < width; ++i)
+			// instead of trailing off the ragged right-hand end of the bars.
+			serial << avr::dec<{.width = 2, .fill = '0'}>(t) << " C : "_P << avr::dec<{.width = 10, .fill = ' '}>(count)
+			       << " |"_P;
+			for (std::uint32_t i = 0; i < count / factor; ++i) {
 				serial << '#';
-			serial << ' ' << statistics::bucket(t) << "\r\n"_P;
+			}
+			serial << "\r\n"_P;
 		}
 	}
 
-	static void help()
+	// 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)
 	{
-		serial << "help show curve monitor uptime statistics histogram reset set <0-100> auto version bootloader\r\n"_P;
+		std::uint8_t at = 0;
+		for (std::uint8_t i = 0; i < which; ++i) {
+			while (names.at(at) != '\0') {
+				++at;
+			}
+			++at;
+		}
+		return at;
 	}
 
-	static void dispatch(std::string_view cmd)
+  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.
+	//
+	// 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
+	// separator is also what measures it.
+	//
+	// Public and `constexpr` so the battery can pin it: the match order is
+	// load-bearing, an abbreviation must never reach `reset`, and reading the
+	// names from flash must not have changed either.
+	static constexpr std::uint8_t lookup(std::string_view input)
 	{
-		if (cmd.empty()) {
+		if (input.empty()) {
-		} else if (cmd == "help") {
+			return command_count;
+		}
+		std::uint8_t at = 0;
+		for (std::uint8_t i = 0; i < command_count; ++i) {
+			std::uint8_t length = 0;
+			bool prefix = true;
+			for (char c; (c = names.at(at + length)) != '\0'; ++length) {
+				if (length < input.size() && input[length] != c) {
+					prefix = false;
+				}
+			}
+			const bool whole = input.size() == length;
+			if (prefix && input.size() <= length && (i != exact_command || whole)) {
+				return i;
+			}
+			at += length + 1;
+		}
+		return command_count;
+	}
+
+	static constexpr std::uint8_t no_command = command_count;
+
+  private:
+	static void dispatch(std::string_view input)
+	{
+		// A line that overflowed the buffer is not a command - it is the tail of
+		// one. Acting on it is how a truncated `reset` becomes a surprise.
+		if (m_overflowed) {
+			serial << "input too long, ignored\r\n"_P;
+			m_overflowed = false;
+			return;
+		}
+
+		// Split on the first space with the (pointer, length) constructor rather
+		// than substr, which throws - see matches().
+		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
+		                      ? std::string_view{}
+		                      : std::string_view{input.data() + space + 1, input.size() - space - 1};
+		if (word.empty()) {
+			return;
+		}
+
+		const auto which = lookup(word);
+
+		switch (which) {
+		case 0:
 			help();
-		} else if (cmd == "show") {
+			return;
+		case 1:
 			show();
-		} else if (cmd == "curve") {
+			return;
+		case 2:
 			print_curve();
-		} else if (cmd == "monitor") {
+			return;
-			monitoring = true;
+		case 3:
-		} else if (cmd == "uptime") {
+			m_monitoring = true;
-			print_uptime();
+			return;
-		} else if (cmd == "statistics") {
+		case 4:
-			print_statistics();
-		} else if (cmd == "histogram") {
-			print_histogram();
-		} else if (cmd == "reset") {
-			statistics::reset();
-			serial << "statistics cleared\r\n"_P;
-		} else if (cmd == "auto") {
-			controller::set_automatic();
-			serial << "auto\r\n"_P;
-		} else if (cmd == "version") {
-			serial << "fantemp on libavr\r\n"_P;
-		} else if (cmd == "bootloader") {
 			serial << "entering bootloader\r\n"_P;
 			statistics::save();
+			serial.drain();
 			bootloader::enter();
-		} else if (cmd.starts_with("set ")) {
+		case 5:
-			std::uint8_t percent = 0;
+			print_uptime();
-			bool valid = cmd.size() > 4;
+			return;
-			for (std::size_t i = 4; i < cmd.size(); ++i) {
+		case 6:
-				if (cmd[i] < '0' || cmd[i] > '9') {
+			print_statistics();
+			return;
+		case 7:
+			print_histogram();
+			return;
+		case 8:
+			statistics::reset();
+			serial << "statistics cleared in EEPROM and RAM\r\n"_P;
+			return;
+		case 9: {
+			std::uint16_t percent = 0;
+			bool valid = !rest.empty();
+			for (char c : rest) {
+				if (c < '0' || c > '9') {
 					valid = false;
 					break;
 				}
-				percent = static_cast<std::uint8_t>(percent * 10 + (cmd[i] - '0'));
+				percent = static_cast<std::uint16_t>(percent * 10 + (c - '0'));
+				if (percent > 100) {
+					valid = false;
+				}
 			}
-			if (valid && percent <= 100) {
+			if (valid) {
-				controller::set_manual(percent);
+				controller::set_manual(static_cast<std::uint8_t>(percent));
-				serial << "fan "_P << percent << " %\r\n"_P;
+				serial << "fan "_P << percent << " %, manual\r\n"_P;
 			} else {
 				serial << "set 0..100\r\n"_P;
 			}
-		} else {
+			return;
-			serial << "? (help)\r\n"_P;
+		}
+		case 10:
+			controller::set_automatic();
+			serial << "automatic fan control\r\n"_P;
+			return;
+		case 11:
+			serial << "FanTemp "_P << version << " on libavr\r\n"_P;
+			return;
+		case 12:
+			statistics::save();
+			serial << "statistics written to EEPROM\r\n"_P;
+			return;
+		default:
+			serial << '\'' << word << "' is not a command; 'help' for the list\r\n"_P;
+			return;
 		}
 	}
 
   public:
+	// A flash string, like every other literal the console prints: as a
+	// `string_view` it was the last constant left in RAM, holding its own
+	// characters and a pointer to them.
+	static constexpr auto version = "v2.2"_P;
+	static constexpr std::uint8_t bar_max = 100;
+
+	// Columns the dotted labels' colons land in, and the curve's span. All four
+	// are the original's.
+	static constexpr std::uint8_t reading_column = 13;
+	static constexpr std::uint8_t stat_column = 21;
+	static constexpr std::uint8_t curve_low = 10;
+	static constexpr std::uint8_t curve_high = 60;
+
+	// A railed ADC leaves the divider with no solution rather than a huge one.
+	static constexpr std::uint32_t open_circuit = 0xffffffff;
+
 	static void init()
 	{
-		serial << "\r\nfantemp on libavr — help for commands\r\n"_P;
+		serial << "\r\nFanTemp "_P << version << " on libavr -- 'help' for commands\r\n"_P;
 		prompt();
 	}
 
 	static void poll()
 	{
-		if (monitoring) {
+		if (m_monitoring) {
-			if (uptime::millis() >= last_monitor + 1000) {
+			if (uptime::millis() >= m_last_monitor + 1000) {
 				show();
-				last_monitor = uptime::millis();
+				m_last_monitor = uptime::millis();
 			}
-			if (serial_t::read()) { // any key stops
+			// Ctrl+C only, as the original had it. Stopping on *any* byte reads
-				monitoring = false;
+			// well until a host sends a line ending: `monitor\r\n` then stops
+			// itself on the `\n` it arrived with, one reading in.
+			if (auto in = serial_t::read(); in && *in == ctrl_c) {
+				serial << "^C\r\n"_P;
+				m_monitoring = false;
 				prompt();
 			}
 			return;
 		}
 		while (auto in = serial_t::read()) {
 			char c = static_cast<char>(*in);
-			if (c == '\r' || c == '\n') {
+			if (c == ctrl_c) {
+				// Abandon whatever was typed and start a fresh line, which is
+				// what Ctrl+C means at every other prompt in the world.
+				serial << "^C\r\n"_P;
+				m_at = 0;
+				m_overflowed = false;
+				prompt();
+			} else if (c == '\r' || c == '\n') {
 				serial << "\r\n"_P;
-				dispatch(std::string_view{line, at});
+				if (m_at == 0 && !m_overflowed) {
-				at = 0;
+					prompt(); // a bare Enter just reprompts, no gap needed
-				if (!monitoring)
+					continue;
+				}
+				dispatch(std::string_view{m_line.data(), m_at});
+				m_at = 0;
+				if (!m_monitoring) {
+					// A blank line between a command's output and the next
+					// prompt: without it the answer and the thing you type
+					// next run together and a screen of them is unreadable.
+					serial << "\r\n"_P;
 					prompt();
-			} else if (c == 0x7f || c == 0x08) {
+				}
-				if (at) {
+			} else if (c == del || c == backspace) {
-					--at;
+				if (m_at) {
+					--m_at;
 					serial << "\b \b"_P;
 				}
-			} else if (at < line_max && c >= ' ') {
+			} else if (c >= ' ') {
-				line[at++] = c;
+				if (m_at < m_line.size()) {
-				serial << c; // echo
+					m_line[m_at++] = c;
+					serial << c; // echo
+				} else {
+					m_overflowed = true; // reported when the line is submitted
+				}
 			}
 		}
 	}

src/thermistor.hpp

@@ -7,10 +7,14 @@
 
 // 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
+// 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- C with linear interpolation between table steps.
 namespace app::thermistor {
 
+// The converter's top count: this board reads the divider at the ADC's
+// full 10 bits, so a reading and the resistance it implies both scale by it.
+inline constexpr std::uint16_t adc_full_scale = (1u << 10) - 1;
+
 inline constexpr double series_resistor = 9951;
 inline constexpr double nominal_resistance = 9270;
 inline constexpr double beta = 3212;
@@ -20,42 +24,47 @@ namespace detail {
 
 consteval double temperature_of(double adc)
 {
-	double resistance = series_resistor * adc / (1023.0 - 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;
 }
 
-// 256 entries over the 10-bit range (steps of 4 counts), quarter-°C,
+// 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.
-consteval std::int16_t quarters_entry(int index)
+consteval std::int16_t quarters_entry(std::int32_t index)
 {
 	double adc = index * 4.0;
-	if (adc < 4)
+	if (adc < 4) {
 		adc = 4;
-	if (adc > 1019)
+	}
+	if (adc > 1019) {
 		adc = 1019;
+	}
 	double t = temperature_of(adc) * 4.0;
-	if (t < -40 * 4)
+	if (t < -40 * 4) {
 		t = -40 * 4;
-	if (t > 125 * 4)
+	}
+	if (t > 125 * 4) {
 		t = 125 * 4;
+	}
 	return static_cast<std::int16_t>(t < 0 ? t - 0.5 : t + 0.5);
 }
 
 inline constexpr avr::flash_table<[] {
 	std::array<std::int16_t, 257> out{};
-	for (int i = 0; i < 257; ++i)
+	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);
+	}
 	return out;
 }()>
     table;
 
 } // namespace detail
 
-// Temperature in quarter-°C from a raw (or averaged) 10-bit sample.
+// Temperature in quarter- 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
@@ -65,9 +74,12 @@ inline std::int16_t quarters(std::uint16_t adc)
 	return static_cast<std::int16_t>(a + ((b - a) * frac) / 4);
 }
 
-inline std::int8_t celsius(std::uint16_t adc)
+// Whole degrees from quarter-degrees, rounded to nearest. The shift is the
+// point: C truncates a negative quotient toward zero, so `(q + 2) / 4` reads
+// -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(adc) + 2) / 4);
+	return static_cast<std::int8_t>((quarters + 2) >> 2);
 }
 
 } // namespace app::thermistor

test/CMakeLists.txt

@@ -0,0 +1,34 @@
+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)
+
+# The README says the two modes emit the same image, and only a tree with
+# both built can say whether they do.
+libavr_mode_identity_test(fantemp)
+
+# The battery is a compile: a static_assert that fails is the failure. It is a
+# target rather than only a ctest so a plain build catches a regression too.
+add_library(consteval_tests OBJECT consteval.cpp)
+target_include_directories(consteval_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../src)
+target_link_libraries(consteval_tests PRIVATE libavr)
+
+add_test(NAME fantemp.consteval
+         COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target consteval_tests)
+
+# The board has no reset line and no programming header, so the loader-entry
+# route in the emitted image is the only thing standing between a firmware change
+# and an unreflashable board.
+# A check whose interpreter this host lacks is registered and *skips*, rather
+# than leaving the suite a different size on a different machine - which is a
+# suite nothing can be compared against. The launcher and the marker are
+# libavr's (`cmake/checks.cmake`).
+libavr_launcher(_python "${LIBAVR_NO_PYTHON}" ${Python3_EXECUTABLE})
+add_test(NAME fantemp.reachability
+         COMMAND ${_python} ${CMAKE_CURRENT_SOURCE_DIR}/check_reachability.py
+                 --objdump ${CMAKE_OBJDUMP} --elf $<TARGET_FILE:fantemp>
+                 --image $<TARGET_FILE_DIR:fantemp>/fantemp.bin
+                 --loader-base ${PUREBOOT_BASE_HEX})
+
+libavr_skip_unverified()

test/check_reachability.py

@@ -0,0 +1,136 @@
+#!/usr/bin/env python3
+"""The board's only way in, checked in the emitted image.
+
+This board has no reset line and no programming header. The single route to the
+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:
+
+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.
+
+    check_reachability.py --objdump avr-objdump --elf fantemp --image fantemp.bin
+"""
+
+from __future__ import annotations
+
+import argparse
+import pathlib
+import re
+import subprocess
+import sys
+
+BOOT_SECTION = 0x7C00  # hfuse d4: BOOTSZ 512 words - a fuse fact, not a loader one
+WDTCSR = 0x60
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser()
+    parser.add_argument("--objdump", required=True)
+    parser.add_argument("--elf", type=pathlib.Path, required=True)
+    parser.add_argument("--image", type=pathlib.Path, required=True)
+    # pureboot's own geometry, from the pinned submodule that exports it, so the
+    # slot's base is stated once for the loader this firmware is deployed with.
+    parser.add_argument("--loader-base", required=True,
+                        type=lambda v: int(v, 0), metavar="ADDR")
+    args = parser.parse_args()
+    loader_base = args.loader_base
+
+    failures = []
+
+    size = args.image.stat().st_size
+    if size >= BOOT_SECTION:
+        failures.append(f"the image is {size} B and reaches 0x{size - 1:04x}, "
+                        f"into the boot section at 0x{BOOT_SECTION:04x}")
+    else:
+        print(f"  ok    image {size} B, ends 0x{size - 1:04x}, "
+              f"{BOOT_SECTION - size} B clear of the boot section")
+
+    text = subprocess.run([args.objdump, "-d", str(args.elf)],
+                          capture_output=True, text=True, check=True).stdout
+
+    # The address the hand-over actually targets, read at its call sites - not
+    # "does the image contain this byte somewhere", which proves nothing: 0x3f is
+    # an ordinary constant that appears in the curve tables, so a check like that
+    # passes just as happily on the 0x7800 bug it is supposed to catch.
+    #
+    # bootloader::call() takes the target as a function pointer, so each call site
+    # loads the *word* address into a register pair immediately before it.
+    lines = text.splitlines()
+    helper = re.compile(r"\b(?:r?call)\b.*<_ZN3app10bootloader4call")
+    sites = []
+    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]:
+            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"])
+
+    want = loader_base // 2
+    if not sites:
+        failures.append("no call to bootloader::call with a loaded target - the "
+                        "hand-over could not be read out of the image")
+    elif wrong := [a for a in sites if a != want]:
+        failures.append(f"the hand-over targets word {[hex(a) for a in wrong]} "
+                        f"(byte {[hex(a * 2) for a in wrong]}), not the loader at "
+                        f"0x{loader_base:04x}")
+    else:
+        print(f"  ok    all {len(sites)} hand-over site(s) target word 0x{want:04x} "
+              f"(byte 0x{loader_base:04x})")
+
+    # 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")
+    else:
+        print("  ok    an indirect call exists (a relative one cannot reach)")
+
+    # What actually reaches WDTCSR, not what the image happens to load somewhere.
+    # A timed disable writes WDCE|WDE (0x18) and then zero. Arming writes WDE
+    # *without* WDCE - including 0x08, a 16 ms timeout with every prescaler bit
+    # clear, which is precisely what the legacy route used and is why this cannot
+    # be a check for "a prescaler is present".
+    WDCE, WDE = 0x10, 0x08
+    values, held = [], {}
+    for line in text.splitlines():
+        if m := re.search(r"\bldi\s+(r\d+),\s*0x([0-9A-Fa-f]{2})", line):
+            held[m.group(1)] = int(m.group(2), 16)
+        elif m := re.search(rf"\bsts\s+0x00{WDTCSR:02X},\s*(r\d+)", line, re.I):
+            reg = m.group(1)
+            values.append(0 if reg == "r1" else held.get(reg))
+    armed = [v for v in values if v is not None and (v & WDE) and not (v & WDCE)]
+    if armed:
+        failures.append(f"WDTCSR is written {[hex(v) for v in armed]} - WDE without "
+                        f"WDCE is arming the watchdog, and a reset-based hand-over "
+                        f"opens no pureboot window")
+    elif not values:
+        print("  ok    the watchdog is never written")
+    else:
+        print(f"  ok    WDTCSR writes are {[hex(v) if v is not None else '?' for v in values]}"
+              f" - unlock and clear, never an arm")
+
+    for line in failures:
+        print(f"  FAIL  {line}")
+    return 1 if failures else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

test/consteval.cpp

@@ -0,0 +1,120 @@
+// Compile-only battery, built with the cross compiler so the target's 16-bit
+// int is exercised. Two of this firmware's three numeric surfaces are decided
+// entirely at compile time - the fan curve and the thermistor table - and the
+// third, quarter-degrees to whole ones, is the arithmetic between them.
+
+#include <cstdint>
+
+#include "curve.hpp"
+#include "terminal.hpp"
+#include "thermistor.hpp"
+
+namespace {
+
+using app::thermistor::whole_degrees;
+
+// Rounding to nearest, across zero. A negative quotient truncates toward zero
+// in C, so a `(q + 2) / 4` reads -3.00 C as -2 and every case below the tie
+// with it; the arithmetic shift floors, which is what these hold it to.
+static_assert(whole_degrees(0) == 0);
+static_assert(whole_degrees(1) == 0);  // +0.25
+static_assert(whole_degrees(2) == 1);  // +0.50, the tie
+static_assert(whole_degrees(3) == 1);  // +0.75
+static_assert(whole_degrees(4) == 1);  // +1.00
+static_assert(whole_degrees(-1) == 0); // -0.25
+static_assert(whole_degrees(-2) == 0); // -0.50, the tie, toward zero
+static_assert(whole_degrees(-3) == -1);
+static_assert(whole_degrees(-4) == -1);
+static_assert(whole_degrees(-12) == -3);
+static_assert(whole_degrees(-160) == -40); // the table's own floor
+static_assert(whole_degrees(500) == 125);  // and its ceiling
+
+// The curve is held at zero below the temperature the fan starts at, and
+// saturates inside the table's window rather than at its edge.
+using app::curve::detail::duty_entry;
+static_assert(duty_entry(app::curve::start_celsius - 1) == 0);
+static_assert(duty_entry(app::curve::start_celsius) == 0);
+static_assert(duty_entry(30) == 7);
+static_assert(duty_entry(40) == 36);
+static_assert(duty_entry(49) == 93);
+static_assert(duty_entry(50) == 100);
+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) {
+		if (duty_entry(t) < duty_entry(t - 1)) {
+			return false;
+		}
+	}
+	return true;
+}
+static_assert(curve_rises());
+
+// The thermistor table, anchored where the Beta equation fixes it rather than
+// against numbers this file computed the same way: the divider reads the
+// 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::quarters_entry;
+
+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
+
+// Both clamps, at the ends the divider cannot leave.
+static_assert(quarters_entry(0) == 125 * 4);
+static_assert(quarters_entry(255) == -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) {
+		if (quarters_entry(i) > quarters_entry(i - 1)) {
+			return false;
+		}
+	}
+	return true;
+}
+static_assert(thermistor_falls());
+
+// The console's one parsing rule, read out of the flash blob the names live
+// in. Every case here is a property of the *order* the names sit in, so this
+// is what an edit to that list has to answer to.
+using app::terminal;
+
+// Full names, and the ends of the list - a walk that miscounts a separator
+// lands on a neighbour rather than failing, so both ends are named.
+static_assert(terminal::lookup("help") == 0);
+static_assert(terminal::lookup("save") == 12);
+static_assert(terminal::lookup("bootloader") == 4);
+
+// Abbreviations resolve to the first entry they prefix. `s` prefixes show,
+// statistics, set and save, and show is first - which is the original's
+// behaviour and the reason the list is ordered rather than sorted.
+static_assert(terminal::lookup("s") == 1);
+static_assert(terminal::lookup("st") == 6);
+static_assert(terminal::lookup("se") == 9);
+static_assert(terminal::lookup("sa") == 12);
+static_assert(terminal::lookup("up") == 5);
+static_assert(terminal::lookup("b") == 4);
+
+// `reset` is the exception: it must be typed in full, so no abbreviation of it
+// resolves - and none of its prefixes names anything else either.
+static_assert(terminal::lookup("reset") == 8);
+static_assert(terminal::lookup("rese") == terminal::no_command);
+static_assert(terminal::lookup("res") == terminal::no_command);
+static_assert(terminal::lookup("r") == terminal::no_command);
+
+// Nothing, and nothing that matches.
+static_assert(terminal::lookup("") == terminal::no_command);
+static_assert(terminal::lookup("xyzzy") == terminal::no_command);
+
+// Longer than the name it prefixes is not a match: `helpful` is not `help`.
+static_assert(terminal::lookup("helpful") == terminal::no_command);
+
+} // namespace

tools/check.sh

@@ -0,0 +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
+# 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
+# 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.
+set -euo pipefail
+cd "$(dirname "${BASH_SOURCE[0]}")/.."
+"${LIBAVR_ROOT:-libavr}/tools/check-presets.sh" "$@"