diff --git a/README.md b/README.md index d143819..bfca542 100644 --- a/README.md +++ b/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 diff --git a/dev/tasks.md b/dev/tasks.md index 8142c16..71c1e20 100644 --- a/dev/tasks.md +++ b/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 diff --git a/src/controller.hpp b/src/controller.hpp index 62547f3..83e4dff 100644 --- a/src/controller.hpp +++ b/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() diff --git a/src/terminal.hpp b/src/terminal.hpp index b0d80ad..deebb1c 100644 --- a/src/terminal.hpp +++ b/src/terminal.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -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({ - "help", - "show", - "curve", - "monitor", - "bootloader", - "uptime", - "statistics", - "histogram", - "reset", - "set", - "auto", - "version", - "save", + return std::to_array({ + {"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 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 = [] { - 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 chars{}; + // 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 + static constexpr auto joined = [] { + constexpr auto bytes = [] { + std::size_t total = 0; + for (const auto &one : commands()) { + total += (one.*Column).size() + 1; + } + return total; + }(); + std::array 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{}; + static constexpr avr::flash_table> names{}; + static constexpr avr::flash_table> 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(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(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 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 + 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(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(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(); } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5f38153..916c81b 100644 --- a/test/CMakeLists.txt +++ b/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.