From eda70c832bdc3e0100f48701f6b7ce7e8fd9ba94 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 16 Oct 2026 18:27:08 -0700 Subject: [PATCH] Format infrastructure startup version labels without allocating --- .github/workflows/run-unit-tests.yml | 1 + examples/simple_repeater/UITask.cpp | 14 +-- examples/simple_room_server/UITask.cpp | 14 +-- examples/simple_sensor/UITask.cpp | 14 +-- test/fixtures/infrastructure_ui_version.cpp | 122 ++++++++++++++++++ test/test_infrastructure_ui_version.py | 133 ++++++++++++++++++++ 6 files changed, 277 insertions(+), 21 deletions(-) create mode 100644 test/fixtures/infrastructure_ui_version.cpp create mode 100644 test/test_infrastructure_ui_version.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index eaf4c69f8..e503b2218 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -809,6 +809,7 @@ jobs: python3 -B test/test_rgb_led_rollover.py -v python3 -B test/test_display_power.py python3 -B test/test_legacy_ui_version_bounds.py -v + python3 -B test/test_infrastructure_ui_version.py -v python3 -B test/test_ssd1306_power.py -v python3 -B test/test_companion_inbox.py python3 -B test/test_heltec_r8_external_power.py diff --git a/examples/simple_repeater/UITask.cpp b/examples/simple_repeater/UITask.cpp index e3cf865f4..ccfbc3bdd 100644 --- a/examples/simple_repeater/UITask.cpp +++ b/examples/simple_repeater/UITask.cpp @@ -350,17 +350,17 @@ void UITask::begin(NodePrefs* node_prefs, const char* build_date, const char* fi user_btn.begin(); #endif - // strip off dash and commit hash by changing dash to null terminator + // strip off dash and commit hash without allocating a mutable version copy // e.g: v1.2.3-abcdef -> v1.2.3 - char *version = strdup(firmware_version); - char *dash = strchr(version, '-'); - if(dash){ - *dash = 0; + size_t version_len = 0; + while (version_len < sizeof(_version_info) - 1 + && firmware_version[version_len] && firmware_version[version_len] != '-') { + ++version_len; } // v1.2.3 (1 Jan 2025) - snprintf(_version_info, sizeof(_version_info), "%s (%s)", version, build_date); - free(version); + snprintf(_version_info, sizeof(_version_info), "%.*s (%s)", + static_cast(version_len), firmware_version, build_date); } void UITask::resetRadioProfileDisplayPage() { diff --git a/examples/simple_room_server/UITask.cpp b/examples/simple_room_server/UITask.cpp index 9b7a96077..69c34f5f8 100644 --- a/examples/simple_room_server/UITask.cpp +++ b/examples/simple_room_server/UITask.cpp @@ -151,17 +151,17 @@ void UITask::begin(NodePrefs* node_prefs, const char* build_date, const char* fi _frame_valid = false; #endif - // strip off dash and commit hash by changing dash to null terminator + // strip off dash and commit hash without allocating a mutable version copy // e.g: v1.2.3-abcdef -> v1.2.3 - char *version = strdup(firmware_version); - char *dash = strchr(version, '-'); - if(dash){ - *dash = 0; + size_t version_len = 0; + while (version_len < sizeof(_version_info) - 1 + && firmware_version[version_len] && firmware_version[version_len] != '-') { + ++version_len; } // v1.2.3 (1 Jan 2025) - snprintf(_version_info, sizeof(_version_info), "%s (%s)", version, build_date); - free(version); + snprintf(_version_info, sizeof(_version_info), "%.*s (%s)", + static_cast(version_len), firmware_version, build_date); } void UITask::resetRadioProfileDisplayPage() { diff --git a/examples/simple_sensor/UITask.cpp b/examples/simple_sensor/UITask.cpp index 2e633f5ea..03fc01c67 100644 --- a/examples/simple_sensor/UITask.cpp +++ b/examples/simple_sensor/UITask.cpp @@ -89,17 +89,17 @@ void UITask::begin(NodePrefs* node_prefs, const char* build_date, const char* fi user_btn.begin(); #endif - // strip off dash and commit hash by changing dash to null terminator + // strip off dash and commit hash without allocating a mutable version copy // e.g: v1.2.3-abcdef -> v1.2.3 - char *version = strdup(firmware_version); - char *dash = strchr(version, '-'); - if(dash){ - *dash = 0; + size_t version_len = 0; + while (version_len < sizeof(_version_info) - 1 + && firmware_version[version_len] && firmware_version[version_len] != '-') { + ++version_len; } // v1.2.3 (1 Jan 2025) - snprintf(_version_info, sizeof(_version_info), "%s (%s)", version, build_date); - free(version); + snprintf(_version_info, sizeof(_version_info), "%.*s (%s)", + static_cast(version_len), firmware_version, build_date); // Make the first read/frame immediately due, including begin() after wrap. const uint32_t now = millis(); diff --git a/test/fixtures/infrastructure_ui_version.cpp b/test/fixtures/infrastructure_ui_version.cpp new file mode 100644 index 000000000..c036168ba --- /dev/null +++ b/test/fixtures/infrastructure_ui_version.cpp @@ -0,0 +1,122 @@ +#include +#include +#include +#include +#include + +static uint32_t now_ms; +static uint32_t millis() { return now_ms; } +static constexpr int HIGH = 1; +static bool deny_copy; +static unsigned copies, resets, flips, palettes; +[[maybe_unused]] static char* copyVersion(const char* text) { + ++copies; + return deny_copy ? nullptr : ::strdup(text); +} +#define strdup copyVersion +namespace mesh { +struct MainBoard { + bool external = false, usb = false; + bool isExternalPowered() const { return external; } + bool isUsbHostConnected() const { return usb; } +}; +} +static mesh::MainBoard board; +struct DisplayDriver { + unsigned power_calls = 0; + bool servicePower(bool powered) { + if (powered != (board.external || board.usb)) std::abort(); + ++power_calls; + return false; + } +}; +struct NodePrefs {}; +struct Button { unsigned begins = 0; void begin() { ++begins; } } user_btn; +struct CHSC6XTouch { unsigned begins = 0; void begin() { ++begins; } }; +struct RadioActivityWindow {}; +struct ObserverDashboard { + enum { ROW_COUNT = 4 }; + struct Context {}; + static void applyDarkPalette() { ++palettes; } +}; +#include "UITask.h" +void UITask::resetRadioProfileDisplayPage() { ++resets; } +#if !TEST_SENSOR +void UITask::applyDisplayFlip() { ++flips; } +#endif +@BEGIN@ + +static const char* scenario; +static unsigned checks; +#define CHECK(expr) do { ++checks; if (!(expr)) { \ + std::fprintf(stderr, "FAIL %s: %s (line %d)\n", scenario, #expr, __LINE__); \ + return false; } } while (0) + +static bool exercise(const std::string& version, const std::string& date, + bool denied, bool require_no_copy, uint32_t now, unsigned power) { + const std::string saved_version = version, saved_date = date; + copies = resets = flips = palettes = user_btn.begins = 0; + deny_copy = denied; + now_ms = now; + board.external = (power & 1) != 0; + board.usb = (power & 2) != 0; + DisplayDriver display; + NodePrefs prefs; +#if TEST_REPEATER || (!TEST_SENSOR && defined(DISPLAY_TOUCH_TOGGLE)) + UITask ui(board, display); +#else + UITask ui(display); +#endif +#if !TEST_SENSOR && defined(DISPLAY_REDRAW_ON_CHANGE) + ui._frame_valid = true; +#endif + std::memset(ui._version_info, 0x55, sizeof(ui._version_info)); + ui.begin(&prefs, date.c_str(), version.c_str()); + const std::string full = version.substr(0, version.find('-')) + " (" + date + ")"; + const std::string expected = full.substr(0, sizeof(ui._version_info) - 1); + CHECK(std::memchr(ui._version_info, 0, sizeof(ui._version_info)) != nullptr); + CHECK(expected == ui._version_info); + CHECK(version == saved_version && date == saved_date); + CHECK(ui._node_prefs == &prefs && ui._display == &display && ui._prevBtnState == HIGH); + CHECK(display.power_calls == 1 && resets == 1); + CHECK(user_btn.begins == unsigned(TEST_FEATURES != 0)); + if (require_no_copy) CHECK(copies == 0); +#if TEST_SENSOR + CHECK(ui._last_read == uint32_t(now - 200u)); + CHECK(ui._last_refresh == uint32_t(now - 1000u)); + CHECK(flips == 0 && palettes == 0); +#else + CHECK(ui._started_at == now && flips == 1); + CHECK(palettes == unsigned(TEST_FEATURES == 2)); +#ifdef DISPLAY_REDRAW_ON_CHANGE + CHECK(!ui._frame_valid); +#endif +#ifdef DISPLAY_TOUCH_TOGGLE + CHECK(ui._touch.begins == 1); +#endif +#endif + return true; +} + +int main(int argc, char** argv) { + if (argc != 2) return 2; + scenario = argv[1]; + if (!std::strcmp(scenario, "denied")) { + if (!exercise("v1.17.1.10-halo-keymind-cascade-dev-7acd76d0", "17-Oct-2026", + true, true, 1234, 2)) return 1; + } else if (!std::strcmp(scenario, "format") || !std::strcmp(scenario, "no-copy")) { + const bool no_copy = !std::strcmp(scenario, "no-copy"); + for (const std::string& version : {std::string(), std::string("v1.18.0"), + std::string("v1.18.0-alpha-beta"), std::string("-hash"), std::string("v1-"), + std::string(30, 'v'), std::string(31, 'v'), std::string(32, 'v'), + std::string(33, 'v'), std::string(31, 'v') + "-hash", std::string(32768, 'v')}) { + for (const std::string& date : {std::string("17-Oct-2026"), std::string(), + std::string(4096, 'd')}) { + for (uint32_t now : {0u, 1234u, UINT32_MAX}) for (unsigned power = 0; power < 4; ++power) { + if (!exercise(version, date, false, no_copy, now, power)) return 1; + } + } + } + } else return 2; + std::printf("PASS %s: %u checks\n", scenario, checks); +} diff --git a/test/test_infrastructure_ui_version.py b/test/test_infrastructure_ui_version.py new file mode 100644 index 000000000..62781e05a --- /dev/null +++ b/test/test_infrastructure_ui_version.py @@ -0,0 +1,133 @@ +"""Run complete infrastructure UI begin methods and headers with denied strdup. + +Only board, display, button and radio-page boundaries are adapters. This does +not simulate total-heap exhaustion or qualify hardware/display initialization. +""" +from pathlib import Path +import os +import re +import shutil +import subprocess +import sys +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +ROLES = ('simple_sensor', 'simple_repeater', 'simple_room_server') +FIXTURE = ROOT / 'test/fixtures/infrastructure_ui_version.cpp' + +# Original identical formatter from all three roles at 7acd76d0950e89d2324191c9359053b364e03db5. +COPY_FORMAT = ''' // strip off dash and commit hash by changing dash to null terminator + // e.g: v1.2.3-abcdef -> v1.2.3 + char *version = strdup(firmware_version); + char *dash = strchr(version, '-'); + if(dash){ + *dash = 0; + } + + // v1.2.3 (1 Jan 2025) + snprintf(_version_info, sizeof(_version_info), "%s (%s)", version, build_date); + free(version); +''' + + +class InfrastructureUiVersionTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + compiler = os.environ.get('CXX') or shutil.which('g++') or shutil.which('clang++') + if not compiler: + raise unittest.SkipTest('a host C++17 compiler is required') + cls.temp = tempfile.TemporaryDirectory(prefix='infrastructure-ui-version-') + cls.addClassCleanup(cls.temp.cleanup) + work = Path(cls.temp.name) + (work / 'helpers/ui').mkdir(parents=True) + for stub in ('helpers/ui/DisplayDriver.h', 'helpers/CommonCLI.h', + 'helpers/RadioActivityWindow.h', 'helpers/ui/ObserverDashboard.h', + 'helpers/ui/CHSC6XTouch.h'): + (work / stub).write_text('#pragma once\n', encoding='ascii') + sanitize = [] if not sys.platform.startswith('linux') else [ + '-fsanitize=address,undefined', '-fno-sanitize-recover=all', + '-fno-omit-frame-pointer', '-fno-pie', '-no-pie'] + cls.binaries = {} + for role in ROLES: + directory = work / role + directory.mkdir() + source = (ROOT / 'examples' / role / 'UITask.cpp').read_text(encoding='ascii') + header = (ROOT / 'examples' / role / 'UITask.h').read_text(encoding='ascii') + # Access changes only; preserve the actual declarations and array. + if header.count('class UITask {') != 1: + raise AssertionError('expected one actual UI class declaration') + (directory / 'UITask.h').write_text( + header.replace('class UITask {', 'struct UITask {'), encoding='ascii') + begin = extract_braced(source, 'void UITask::begin(') + start = begin.index(' // strip off dash') + end = (begin.index(' // Make the first read/frame', start) + if role == 'simple_sensor' else begin.rfind('}')) + legacy = begin[:start] + COPY_FORMAT + '\n' + begin[end:] + for kind, method in (('production', begin), ('copy', legacy)): + cpp = directory / (kind + '.cpp') + cpp.write_text(FIXTURE.read_text(encoding='ascii').replace('@BEGIN@', method), + encoding='ascii') + for features in (0, 1, 2): + macros = ['-DTEST_SENSOR=' + str(int(role == 'simple_sensor')), + '-DTEST_REPEATER=' + str(int(role == 'simple_repeater')), + '-DTEST_FEATURES=' + str(features)] + if features: + macros += ['-DPIN_USER_BTN=4', '-DDISPLAY_CLASS=DisplayDriver', + '-DMOMENTARY_BUTTON_WAKE_FROM_SLEEP=1'] + if features == 2: + macros += ['-DDISPLAY_TOUCH_TOGGLE=1', '-DDISPLAY_ACTIVITY_DASHBOARD=1', + '-DDISPLAY_REDRAW_ON_CHANGE=1'] + binary = directory / (kind + '-' + str(features) + + ('.exe' if os.name == 'nt' else '')) + built = subprocess.run([ + compiler, '-std=c++17', '-O1', '-g', '-Wall', '-Wextra', '-Werror', + *sanitize, *macros, '-I' + str(work), str(cpp), '-o', str(binary)], + capture_output=True, text=True, timeout=60) + if built.returncode: + raise AssertionError(built.stdout + built.stderr) + cls.binaries[role, kind, features] = binary + + def check_case(self, case, kind='production', expected_failure=False): + for role in ROLES: + for features in (0, 1, 2): + with self.subTest(role=role, features=features, case=case): + checked = subprocess.run([str(self.binaries[role, kind, features]), case], + capture_output=True, text=True, timeout=10) + if expected_failure: + self.assertNotEqual(checked.returncode, 0) + self.assertRegex(checked.stderr, + r'runtime error: null pointer|AddressSanitizer:') + else: + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn('PASS ' + case, checked.stdout) + + def test_displayed_labels_bounds_and_startup_side_effects(self): + self.check_case('format') + + def test_formatting_needs_no_mutable_version_copy(self): + self.check_case('no-copy') + + def test_denied_version_allocation_cannot_crash_startup(self): + self.check_case('denied') + + @unittest.skipUnless(sys.platform.startswith('linux'), 'sanitizer diagnostic control') + def test_original_copy_failure_is_reproduced_not_silently_skipped(self): + self.check_case('denied', 'copy', expected_failure=True) + + def test_actual_capacity_live_startup_and_ci_wiring(self): + for role in ROLES: + with self.subTest(role=role): + folder = ROOT / 'examples' / role + header = (folder / 'UITask.h').read_text(encoding='ascii') + self.assertRegex(header, r'char _version_info\[32\];') + main = (folder / 'main.cpp').read_text(encoding='ascii') + self.assertIn('ui_task.begin(', extract_braced(main, 'void setup()')) + workflow = (ROOT / '.github/workflows/run-unit-tests.yml').read_text(encoding='ascii') + self.assertIn('python3 -B test/test_infrastructure_ui_version.py -v', workflow) + + +if __name__ == '__main__': + unittest.main()