diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 3d65e7542..6bd6de21d 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -804,6 +804,7 @@ jobs: python3 -B test/test_notification_settings.py python3 -B test/test_notification_led_ownership.py -v python3 -B test/test_mono_led_rollover.py -v + python3 -B test/test_backlight_poll_rollover.py -v python3 -B test/test_display_power.py python3 -B test/test_legacy_ui_version_bounds.py -v python3 -B test/test_ssd1306_power.py -v diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index 2d05211a6..2e1d3d1dd 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -2444,13 +2444,17 @@ void UITask::loop() { } #endif #if defined(BACKLIGHT_BTN) - if (millis() > next_backlight_btn_check) { - static bool was_pressed = false; + // First-poll state must not collide with a completed poll at timestamp zero. + enum class BacklightButtonState : uint8_t { Unseen, Released, Pressed }; + static BacklightButtonState button_state = BacklightButtonState::Unseen; + const uint32_t backlight_now = millis(); + if (button_state == BacklightButtonState::Unseen ? backlight_now > 0 + : static_cast(backlight_now - last_backlight_btn_check) > 300) { const bool pressed = digitalRead(PIN_BUTTON2) == LOW; - if (pressed && !was_pressed && _display != NULL) + if (pressed && button_state != BacklightButtonState::Pressed && _display != NULL) _display->wake(mesh::ui::DisplayWake::Button); - was_pressed = pressed; - next_backlight_btn_check = millis() + 300; + button_state = pressed ? BacklightButtonState::Pressed : BacklightButtonState::Released; + last_backlight_btn_check = millis(); } #endif diff --git a/examples/companion_radio/ui-new/UITask.h b/examples/companion_radio/ui-new/UITask.h index e9186fca8..ad0bab61c 100644 --- a/examples/companion_radio/ui-new/UITask.h +++ b/examples/companion_radio/ui-new/UITask.h @@ -48,7 +48,7 @@ class UITask : public AbstractUITask { int _msgcount; unsigned long ui_started_at; uint32_t next_batt_chck; - int next_backlight_btn_check = 0; + uint32_t last_backlight_btn_check = 0; #ifdef HAS_TOUCH #ifndef TOUCH_CENTER_ZONE_PERCENT #define TOUCH_CENTER_ZONE_PERCENT 34 diff --git a/examples/companion_radio/ui-tiny/UITask.cpp b/examples/companion_radio/ui-tiny/UITask.cpp index 515db50e6..5ef36b6fa 100644 --- a/examples/companion_radio/ui-tiny/UITask.cpp +++ b/examples/companion_radio/ui-tiny/UITask.cpp @@ -940,13 +940,17 @@ void UITask::loop() { } #endif #if defined(BACKLIGHT_BTN) - if (millis() > next_backlight_btn_check) { - static bool was_pressed = false; + // First-poll state must not collide with a completed poll at timestamp zero. + enum class BacklightButtonState : uint8_t { Unseen, Released, Pressed }; + static BacklightButtonState button_state = BacklightButtonState::Unseen; + const uint32_t backlight_now = millis(); + if (button_state == BacklightButtonState::Unseen ? backlight_now > 0 + : static_cast(backlight_now - last_backlight_btn_check) > 300) { const bool pressed = digitalRead(PIN_BUTTON2) == LOW; - if (pressed && !was_pressed && _display != NULL) + if (pressed && button_state != BacklightButtonState::Pressed && _display != NULL) _display->wake(mesh::ui::DisplayWake::Button); - was_pressed = pressed; - next_backlight_btn_check = millis() + 300; + button_state = pressed ? BacklightButtonState::Pressed : BacklightButtonState::Released; + last_backlight_btn_check = millis(); } #endif #if defined(HAS_TORCH) diff --git a/examples/companion_radio/ui-tiny/UITask.h b/examples/companion_radio/ui-tiny/UITask.h index 25d0fd08e..e92705413 100644 --- a/examples/companion_radio/ui-tiny/UITask.h +++ b/examples/companion_radio/ui-tiny/UITask.h @@ -47,7 +47,7 @@ class UITask : public AbstractUITask { int _msgcount; unsigned long ui_started_at; uint32_t next_batt_chck; - int next_backlight_btn_check = 0; + uint32_t last_backlight_btn_check = 0; uint16_t _cached_batt_mv; #ifdef PIN_STATUS_LED int led_state = 0; diff --git a/test/fixtures/backlight_poll_rollover.cpp b/test/fixtures/backlight_poll_rollover.cpp new file mode 100644 index 000000000..30cb143b7 --- /dev/null +++ b/test/fixtures/backlight_poll_rollover.cpp @@ -0,0 +1,181 @@ +// Actual guarded loop block and timer field are injected without rewriting. +// Only Arduino clock/GPIO and display wake are host boundary adapters. +#include +#include +#include +#include + +static uint32_t clock_ms = 0; +uint32_t millis() { return clock_ms; } +static unsigned reads = 0, wakes = 0; +static uint32_t read_duration = 0, wake_duration = 0; +static bool pin_pressed = false; +static const char* active_case = "startup"; +static unsigned checks = 0; +#define CHECK(value) do { \ + ++checks; \ + if (!(value)) { \ + std::fprintf(stderr, "FAIL %s: %s (line %d)\n", active_case, #value, __LINE__); \ + std::exit(1); \ + } \ +} while (0) + +#if defined(BACKLIGHT_BTN) +constexpr unsigned LOW = 0, HIGH = 1; +int digitalRead(unsigned pin) { + CHECK(pin == PIN_BUTTON2); + ++reads; + clock_ms += read_duration; + return pin_pressed ? LOW : HIGH; +} +#endif + +namespace mesh { namespace ui { enum class DisplayWake { Button }; } } +struct DisplayDriver { + void wake(mesh::ui::DisplayWake reason) { + CHECK(reason == mesh::ui::DisplayWake::Button); + ++wakes; + clock_ms += wake_duration; + } +} display; + +struct UITask { + @FIELD@ + DisplayDriver* _display = &display; + void poll() { +@BLOCK@ + } +}; + +static void at(UITask& ui, uint32_t now, bool pressed, + unsigned expected_reads, unsigned expected_wakes) { + clock_ms = now; + pin_pressed = pressed; + ui.poll(); + CHECK(reads == expected_reads); + CHECK(wakes == expected_wakes); +} + +#if defined(BACKLIGHT_BTN) +static void cadence(UITask& ui, uint32_t start, bool initially_pressed) { + const unsigned first_wake = initially_pressed ? 1 : 0; + at(ui, start, initially_pressed, 1, first_wake); + at(ui, start, initially_pressed, 1, first_wake); + at(ui, start + 1, !initially_pressed, 1, first_wake); + at(ui, start + 300, !initially_pressed, 1, first_wake); + at(ui, start + 301, !initially_pressed, 2, 1); + at(ui, start + 301, !initially_pressed, 2, 1); +} +#endif + +// Every case is a fresh process: real static sampled-button state is retained. +int main(int argc, char** argv) { + CHECK(argc == 2); + active_case = argv[1]; + UITask ui; + const char* name = argv[1]; +#if defined(BACKLIGHT_BTN) + if (!std::strcmp(name, "zero")) { + at(ui, 0, true, 0, 0); + cadence(ui, 1, true); + } else if (!std::strcmp(name, "upper")) { + cadence(ui, 0x80000000u, false); + } else if (!std::strcmp(name, "upper-pressed")) { + cadence(ui, 0x80000000u, true); + } else if (!std::strcmp(name, "max")) { + cadence(ui, UINT32_MAX, false); + } else if (!std::strcmp(name, "normal")) { + cadence(ui, 100, false); + } else if (!std::strcmp(name, "signed-boundary")) { + cadence(ui, 0x7fffffffu - 150, false); + } else if (!std::strcmp(name, "wrap-visible")) { + cadence(ui, UINT32_MAX - 15, false); + } else if (!std::strcmp(name, "wrap-wake")) { + at(ui, UINT32_MAX - 310, false, 1, 0); + at(ui, 20, true, 2, 1); + at(ui, 20, true, 2, 1); + } else if (!std::strcmp(name, "deadline-zero")) { + cadence(ui, uint32_t(0u - 300u), false); + } else if (!std::strcmp(name, "held")) { + at(ui, 1, true, 1, 1); + at(ui, 302, true, 2, 1); + at(ui, 603, true, 3, 1); + at(ui, 904, false, 4, 1); + at(ui, 1205, true, 5, 2); + } else if (!std::strcmp(name, "unobserved-release")) { + at(ui, 1, true, 1, 1); + at(ui, 100, false, 1, 1); + at(ui, 101, true, 1, 1); + at(ui, 302, true, 2, 1); + } else if (!std::strcmp(name, "null-pressed")) { + ui._display = nullptr; + at(ui, 1, true, 1, 0); + ui._display = &display; + at(ui, 302, true, 2, 0); + at(ui, 603, false, 3, 0); + at(ui, 904, true, 4, 1); + } else if (!std::strcmp(name, "null-released")) { + ui._display = nullptr; + at(ui, 1, false, 1, 0); + ui._display = &display; + at(ui, 302, true, 2, 1); + } else if (!std::strcmp(name, "completion")) { + read_duration = 7; + wake_duration = 37; + at(ui, 100, true, 1, 1); + CHECK(clock_ms == 144); + read_duration = wake_duration = 0; + at(ui, 444, false, 1, 1); + at(ui, 445, false, 2, 1); + at(ui, 745, true, 2, 1); + at(ui, 746, true, 3, 2); + } else if (!std::strcmp(name, "completion-zero-wake")) { + wake_duration = 11; + at(ui, UINT32_MAX - 10, true, 1, 1); + CHECK(clock_ms == 0); + wake_duration = 0; + at(ui, 0, true, 1, 1); + at(ui, 1, true, 1, 1); + at(ui, 300, false, 1, 1); + at(ui, 301, false, 2, 1); + } else if (!std::strcmp(name, "completion-zero-read")) { + read_duration = 11; + at(ui, UINT32_MAX - 10, false, 1, 0); + CHECK(clock_ms == 0); + read_duration = 0; + at(ui, 0, true, 1, 0); + at(ui, 300, true, 1, 0); + at(ui, 301, true, 2, 1); + } else if (!std::strcmp(name, "long-released")) { + at(ui, 1, false, 1, 0); + at(ui, 1u + 26u * 24u * 60u * 60u * 1000u, true, 2, 1); + } else if (!std::strcmp(name, "long-pressed")) { + at(ui, 1, true, 1, 1); + const uint32_t later = 1u + 26u * 24u * 60u * 60u * 1000u; + at(ui, later, true, 2, 1); + at(ui, later + 301, false, 3, 1); + at(ui, later + 602, true, 4, 2); + } else if (!std::strcmp(name, "near-full-cycle")) { + at(ui, 100, false, 1, 0); + at(ui, uint32_t(100u + UINT32_MAX - 1000u), true, 2, 1); + } else if (!std::strcmp(name, "late-one-poll")) { + at(ui, 1, false, 1, 0); + at(ui, 100000, true, 2, 1); + at(ui, 100000, false, 2, 1); + at(ui, 100300, false, 2, 1); + at(ui, 100301, false, 3, 1); + } else { + std::fprintf(stderr, "unknown case: %s\n", name); + return 2; + } +#else + CHECK(std::strcmp(name, "no-feature") == 0); + const uint32_t stamps[] = {0u, 1u, 0x7fffffffu, 0x80000000u, UINT32_MAX}; + for (uint32_t stamp : stamps) { + at(ui, stamp, true, 0, 0); + at(ui, stamp, false, 0, 0); + } + CHECK(read_duration == 0 && wake_duration == 0); +#endif + std::printf("PASS %s: %u checks\n", name, checks); +} diff --git a/test/test_backlight_poll_rollover.py b/test/test_backlight_poll_rollover.py new file mode 100644 index 000000000..a370e3f5d --- /dev/null +++ b/test/test_backlight_poll_rollover.py @@ -0,0 +1,114 @@ +"""Compile actual guarded backlight polling with original timer initialization. + +Both UI paths are fixture-enabled, plus feature-disabled controls. Current +normal BACKLIGHT_BTN recipes select ui-new; ui-tiny is consistency coverage. +Arduino clock/GPIO and display wake are host fakes, not hardware sleep tests. +""" +from pathlib import Path +import os +import re +import shutil +import subprocess +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / 'test/fixtures/backlight_poll_rollover.cpp' +UIS = ('ui-new', 'ui-tiny') +CASES = ('zero', 'upper', 'upper-pressed', 'max', 'normal', 'signed-boundary', + 'wrap-visible', 'wrap-wake', 'deadline-zero', 'held', 'unobserved-release', + 'null-pressed', 'null-released', 'completion', 'completion-zero-wake', + 'completion-zero-read', 'long-released', 'long-pressed', 'near-full-cycle', + 'late-one-poll') + + +def production(ui): + prefix = ROOT / 'examples/companion_radio' / ui + source = (prefix / 'UITask.cpp').read_text(encoding='ascii') + header = (prefix / 'UITask.h').read_text(encoding='ascii') + first = source.index('#if defined(BACKLIGHT_BTN)') + block = source[first:source.index('#endif', first) + len('#endif')] + field = re.search(r'^ (?:int|uint32_t) (?:next|last)_backlight_btn_check = 0;', + header, re.MULTILINE).group() + return source, field, block + + +def harness(ui, mutation=None): + _, field, block = production(ui) + if mutation == 'level-wake': + block, count = re.subn(r'pressed\s*&&\s*button_state\s*!=\s*BacklightButtonState::Pressed\s*&&', + 'pressed &&', block, count=1) + if count != 1: + raise AssertionError('backlight wake must recognize a newly sampled press') + elif mutation == 'entry-anchor': + old = 'last_backlight_btn_check = millis();' + if block.count(old) != 1: + raise AssertionError('backlight timestamp must be anchored after I/O and wake') + block = block.replace(old, 'last_backlight_btn_check = backlight_now;', 1) + return FIXTURE.read_text(encoding='ascii').replace('@FIELD@', field).replace('@BLOCK@', block) + + +class BacklightPollRolloverTest(unittest.TestCase): + def test_actual_polling_wrap_cadence_and_button_state(self): + self.run_variants() + + def test_block_remains_in_live_loop_with_32bit_timer(self): + for ui in UIS: + with self.subTest(ui=ui): + source, field, block = production(ui) + self.assertEqual(field.strip(), 'uint32_t last_backlight_btn_check = 0;') + loop = extract_braced(source, 'void UITask::loop()') + self.assertEqual(loop.count(block), 1) + self.assertLess(loop.index(block), loop.index('userLedHandler();')) + self.assertIn('digitalRead(PIN_BUTTON2) == LOW', block) + self.assertIn('_display->wake(mesh::ui::DisplayWake::Button)', block) + + def test_held_level_wake_regression_is_rejected(self): + self.run_variants('level-wake', ('held',), 'FAIL held: wakes == expected_wakes') + + def test_entry_anchoring_regression_is_rejected(self): + self.run_variants('entry-anchor', ('completion',), 'FAIL completion: reads == expected_reads') + + def run_variants(self, mutation=None, cases=CASES, expected_failure=None): + compiler = shutil.which('g++') or shutil.which('clang++') + if compiler is None: + self.skipTest('a host C++17 compiler is required') + sanitize = [] if os.name == 'nt' else [ + '-fsanitize=address,undefined', '-fno-sanitize-recover=all', '-fno-pie', '-no-pie'] + with tempfile.TemporaryDirectory(prefix='backlight-poll-rollover-') as directory: + work = Path(directory) + for ui in UIS: + source = work / (ui + '.cpp') + source.write_text(harness(ui, mutation), encoding='ascii') + for enabled in (True,) if mutation else (True, False): + with self.subTest(ui=ui, enabled=enabled): + binary = work / (ui + '-' + str(int(enabled)) + + ('.exe' if os.name == 'nt' else '')) + # A disabled build deliberately has no PIN_BUTTON2/LOW + # definitions: accidental unguarded I/O cannot compile. + macros = ['-DBACKLIGHT_BTN=PIN_BUTTON2', '-DPIN_BUTTON2=14'] if enabled else [] + built = subprocess.run([ + compiler, '-std=c++17', '-O1', '-g', '-Wall', '-Wextra', '-Werror', + *sanitize, *macros, str(source), '-o', str(binary), + ], capture_output=True, text=True, timeout=60) + self.assertEqual(built.returncode, 0, built.stdout + built.stderr) + # Preserve the real function-static sampled state by + # launching a fresh process, not resetting fake statics. + for case in cases if enabled else ('no-feature',): + with self.subTest(case=case): + tested = subprocess.run([str(binary), case], capture_output=True, + text=True, timeout=10) + if expected_failure: + self.assertEqual(tested.returncode, 1, tested.stdout + tested.stderr) + self.assertIn(expected_failure, tested.stderr) + self.assertNotIn('AddressSanitizer:', tested.stderr) + self.assertNotIn('runtime error:', tested.stderr) + else: + self.assertEqual(tested.returncode, 0, tested.stdout + tested.stderr) + self.assertRegex(tested.stdout, r'PASS ' + case + r': \d+ checks') + + +if __name__ == '__main__': + unittest.main()