console: each command carries its summary, and dispatch names it, -166 B

The help text was thirteen help_row(N) calls and the dispatch thirteen
case N labels, both counting positions in a list defined elsewhere. A
command is now one row of name and summary; the summaries become a second
NUL-separated flash blob printed in a loop, and every case label is
command_index("name"), which does not compile for a name the list lacks.
The help column derives from the longest name, the dotted padding is one
helper for labels and help rows alike, and the ADC scale, the monitor
interval and the uptime units are named. Output is unchanged; the image
goes from 8004 B to 7838 B, both modes byte-identical. The 32-bit
narrowing in dev/tasks.md is re-measured against the new image.

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

README.md

@@ -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 **7838 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
+`uint64_t` to `uint32_t` is **602 B of flash, 7.7 % of the image** (7838 B down
-to 7396), plus 16 B of RAM.
+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/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;
+		m_percent = percent;
-		fan::set_duty(avr::percent_t::of(p));
+		fan::set_duty(avr::percent_t::of(percent));
 	}
 
 	static void set_automatic()

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.
+// blobs below are being built and never as storage of its own.
-consteval auto command_names()
+consteval auto commands()
 {
-	return std::to_array<std::string_view>({
+	return std::to_array<command>({
-	    "help",
+	    {"help", "prints this help message"},
-	    "show",
+	    {"show", "shows current temperature and fan speed"},
-	    "curve",
+	    {"curve", "shows mapping from temperature to fan speed"},
-	    "monitor",
+	    {"monitor", "loops the show command until Ctrl+C is pressed"},
-	    "bootloader",
+	    {"bootloader", "enters the bootloader"},
-	    "uptime",
+	    {"uptime", "shows system uptime"},
-	    "statistics",
+	    {"statistics", "prints overall statistics like min and max temp"},
-	    "histogram",
+	    {"histogram", "prints a histogram of the temperature"},
-	    "reset",
+	    {"reset", "resets statistics to 0 in EEPROM and RAM (no abbreviation)"},
-	    "set",
+	    {"set", "sets the fan speed to the provided value, 0-100"},
-	    "auto",
+	    {"auto", "turns on automatic fan control"},
-	    "version",
+	    {"version", "displays firmware version"},
-	    "save",
+	    {"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
-		std::size_t total = 0;
+	// and printed at run time, which is not a reason to sit in RAM on a machine
-		for (auto one : command_names()) {
+	// with two address spaces: a table of `string_view` puts the characters
-			total += one.size() + 1;
+	// *and* its own pointers there. Separators rather than an offset table,
-		}
+	// because an offset table is the RAM this exists to give back.
-		return total;
+	template <std::string_view command::*Column>
-	}();
+	static constexpr auto joined = [] {
-
+		constexpr auto bytes = [] {
-	// The names as one NUL-separated blob in flash. Matched at run time, which
+			std::size_t total = 0;
-	// is not a reason to sit in RAM on a machine with two address spaces: a
+			for (const auto &one : commands()) {
-	// table of `string_view` puts the characters *and* its own pointers there,
+				total += (one.*Column).size() + 1;
-	// 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
+			return total;
-	// RAM this exists to give back.
+		}();
-	static constexpr auto names_data = [] {
+		std::array<char, bytes> chars{};
-		std::array<char, name_bytes> chars{};
 		std::size_t at = 0;
-		for (auto one : command_names()) {
+		for (const auto &one : commands()) {
-			for (char c : one) {
+			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
+	// data.
-	// commands cannot move the protection onto a different one.
+	static constexpr std::uint8_t exact_command = command_index("reset");
-	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
+	// One space and at least one dot after the longest name, the original's
-	// name. The longest name is `bootloader` at 10, so 12 leaves it a space and
+	// layout exactly.
-	// one dot - the original's layout exactly.
+	static constexpr std::uint8_t help_column = [] {
-	static constexpr std::uint8_t help_column = 12;
+		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
+	// The tail of `name ....: ` - the dotted label the original used everywhere
-	// list of things, which is what makes a column of values readable without
+	// it printed a list of things, which is what makes a column of values
-	// counting spaces. Its width comes from the type, so the padding needs no
+	// readable without counting spaces.
-	// hand-counted constant.
+	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 << ' ';
+		serial << text;
-		for (auto i = Flash::size + 1; i < column; ++i) {
+		pad(Flash::size, column);
-			serial << '.';
-		}
-		serial << ": "_P;
 	}
 
-	// The same shape for a command name, whose width is not a type but the walk
+	// Prints one entry of a blob, from `at` to its separator, and answers its
-	// to its separator - so it is counted as it is printed rather than measured
+	// width - counted as it is printed rather than measured first, which would
-	// first, which would read the blob twice.
+	// read the blob twice.
-	static void help_row(std::uint8_t which)
+	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 << ' ';
+		return width;
-		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
+	// Quarter-degrees as a signed decimal with a two-digit fraction. The sign is
-	// off first: C++ gives a negative remainder for a negative dividend, so
+	// 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".
+	// 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);
+		std::uint16_t name = 0;
-		serial << "prints this help message\r\n"_P;
+		std::uint16_t summary = 0;
-		help_row(1);
+		for (std::uint8_t i = 0; i < command_count; ++i) {
-		serial << "shows current temperature and fan speed\r\n"_P;
+			const auto width = print_entry(names, name);
-		help_row(2);
+			name += width + 1;
-		serial << "shows mapping from temperature to fan speed\r\n"_P;
+			pad(width, help_column);
-		help_row(3);
+			summary += print_entry(summaries, summary) + 1;
-		serial << "loops the show command until Ctrl+C is pressed\r\n"_P;
+			serial << "\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;
 	}
 
@@ -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
+		serial << seconds / day << "d "_P << seconds % day / hour << "h "_P << seconds % hour / minute << "m "_P
-		       << seconds % 60 << "s\r\n"_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
+	// Which command a word names, or `no_command` for none. The input matches
-	// matches when it is a non-empty prefix of a name; `reset` is the exception
+	// when it is a non-empty prefix of a name; `reset` is the exception and must
-	// and must be typed in full.
+	// 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
+		// Split on the first space with the (pointer, length) constructor:
-		// than substr, which throws - see matches().
+		// 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 (lookup(word)) {
-
+		case command_index("help"):
-		switch (which) {
-		case 0:
 			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();
 			}

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 7838)
 
 # The README says the two modes emit the same image, and only a tree with
 # both built can say whether they do.