From d43bc23f5aaf17535e2c0cfe8d9caccec0f9b987 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Wed, 14 Oct 2026 00:21:23 -0700 Subject: [PATCH] Release OLED power ownership when initialization fails --- .github/workflows/run-unit-tests.yml | 1 + src/helpers/ui/SSD1306Display.cpp | 19 ++- .../ssd1306_power/mocks/Adafruit_GFX.h | 1 + .../ssd1306_power/mocks/Adafruit_SSD1306.h | 56 +++++++ test/fixtures/ssd1306_power/mocks/Arduino.h | 27 +++ test/fixtures/ssd1306_power/mocks/Wire.h | 11 ++ test/fixtures/ssd1306_power/test.cpp | 155 ++++++++++++++++++ test/test_ssd1306_power.py | 66 ++++++++ 8 files changed, 333 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/ssd1306_power/mocks/Adafruit_GFX.h create mode 100644 test/fixtures/ssd1306_power/mocks/Adafruit_SSD1306.h create mode 100644 test/fixtures/ssd1306_power/mocks/Arduino.h create mode 100644 test/fixtures/ssd1306_power/mocks/Wire.h create mode 100644 test/fixtures/ssd1306_power/test.cpp create mode 100644 test/test_ssd1306_power.py diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 0d333b769..9ca560029 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -651,6 +651,7 @@ jobs: node test/test_notification_builder.js python3 -B test/test_notification_settings.py python3 -B test/test_display_power.py + python3 -B test/test_ssd1306_power.py -v python3 -B test/test_companion_inbox.py python3 -B test/test_heltec_r8_external_power.py python3 -B test/test_bluetooth_mac_contract.py diff --git a/src/helpers/ui/SSD1306Display.cpp b/src/helpers/ui/SSD1306Display.cpp index 3905dcb92..82dbec97d 100644 --- a/src/helpers/ui/SSD1306Display.cpp +++ b/src/helpers/ui/SSD1306Display.cpp @@ -39,6 +39,17 @@ bool SSD1306Display::begin() { if (started) { _panel_ready = true; applyRotation(); + } else { + _panel_ready = false; + if (_peripher_power) { +#if PIN_OLED_RESET >= 0 + digitalWrite(PIN_OLED_RESET, LOW); +#endif + // begin() owns one claim even when it was already on, or turnOn() + // reached it after a previous failure. Other peripheral owners remain. + _peripher_power->release(); + } + _isOn = false; } return started; } @@ -79,9 +90,11 @@ void SSD1306Display::setFlipped(bool flipped) { void SSD1306Display::turnOn() { if (!_isOn) { - if (_peripher_power) _peripher_power->claim(); - _isOn = true; // set before begin() to prevent double claim - if (_peripher_power) begin(); // re-init display after power was cut + if (_peripher_power || !_panel_ready) { + if (!begin()) return; // begin owns and rolls back its claim on failure + } else { + _isOn = true; // A continuously powered, initialized panel needs no reset. + } } display.ssd1306_command(SSD1306_DISPLAYON); } diff --git a/test/fixtures/ssd1306_power/mocks/Adafruit_GFX.h b/test/fixtures/ssd1306_power/mocks/Adafruit_GFX.h new file mode 100644 index 000000000..6f70f09be --- /dev/null +++ b/test/fixtures/ssd1306_power/mocks/Adafruit_GFX.h @@ -0,0 +1 @@ +#pragma once diff --git a/test/fixtures/ssd1306_power/mocks/Adafruit_SSD1306.h b/test/fixtures/ssd1306_power/mocks/Adafruit_SSD1306.h new file mode 100644 index 000000000..64ecc5a78 --- /dev/null +++ b/test/fixtures/ssd1306_power/mocks/Adafruit_SSD1306.h @@ -0,0 +1,56 @@ +#pragma once + +// Only the hardware/library boundary is doubled. The complete firmware driver, +// its public header, DisplayDriver and RefCountedDigitalPin compile unchanged. +#include +#include +#include + +#define SSD1306_BLACK 0 +#define SSD1306_WHITE 1 +#define SSD1306_SWITCHCAPVCC 2 +#define SSD1306_DISPLAYOFF 0xAE +#define SSD1306_DISPLAYON 0xAF + +struct PanelState { + bool allocation_ok = true; + unsigned begins = 0, rotations = 0, frames = 0; + unsigned on_commands = 0, off_commands = 0; +}; +inline PanelState panel_state; + +class Adafruit_SSD1306 { + int16_t _width, _height; + uint8_t _rotation = 0; +public: + Adafruit_SSD1306(int16_t width, int16_t height, TwoWire*, int) + : _width(width), _height(height) {} + bool begin(int, uint8_t, bool, bool) { + ++panel_state.begins; + return panel_state.allocation_ok; + } + void setRotation(uint8_t rotation) { _rotation = rotation; ++panel_state.rotations; } + int16_t width() const { return _rotation & 1 ? _height : _width; } + int16_t height() const { return _rotation & 1 ? _width : _height; } + void clearDisplay() {} + void display() { ++panel_state.frames; } + void ssd1306_command(uint8_t command) { + if (command == SSD1306_DISPLAYON) ++panel_state.on_commands; + if (command == SSD1306_DISPLAYOFF) ++panel_state.off_commands; + } + void setTextColor(uint16_t) {} + void setTextSize(int) {} + void setTextWrap(bool) {} + void cp437(bool) {} + void setCursor(int, int) {} + void print(const char*) {} + void fillRect(int, int, int, int, uint16_t) {} + void drawRect(int, int, int, int, uint16_t) {} + void drawBitmap(int, int, const uint8_t*, int, int, uint16_t) {} + void getTextBounds(const char* text, int, int, int16_t* x, int16_t* y, + uint16_t* width, uint16_t* height) { + *x = *y = 0; + *width = strlen(text) * 6; + *height = 8; + } +}; diff --git a/test/fixtures/ssd1306_power/mocks/Arduino.h b/test/fixtures/ssd1306_power/mocks/Arduino.h new file mode 100644 index 000000000..0333ff208 --- /dev/null +++ b/test/fixtures/ssd1306_power/mocks/Arduino.h @@ -0,0 +1,27 @@ +#pragma once + +#include +#include +#include + +#define HIGH 1 +#define LOW 0 +#define INPUT 0 +#define OUTPUT 1 + +inline uint8_t pin_levels[64] = {}; +inline uint8_t pin_modes[64] = {}; +inline uint32_t now_ms = 100; +inline uint32_t millis() { return now_ms; } +inline void pinMode(unsigned pin, uint8_t mode) { + assert(pin < 64); + pin_modes[pin] = mode; +} +inline void digitalWrite(unsigned pin, uint8_t level) { + assert(pin < 64); + pin_levels[pin] = level; +} +inline uint8_t digitalRead(unsigned pin) { + assert(pin < 64); + return pin_levels[pin]; +} diff --git a/test/fixtures/ssd1306_power/mocks/Wire.h b/test/fixtures/ssd1306_power/mocks/Wire.h new file mode 100644 index 000000000..299b28c72 --- /dev/null +++ b/test/fixtures/ssd1306_power/mocks/Wire.h @@ -0,0 +1,11 @@ +#pragma once + +#include + +struct TwoWire { + uint8_t result = 0, address = 0; + unsigned probes = 0; + void beginTransmission(uint8_t value) { address = value; } + uint8_t endTransmission() { ++probes; return result; } +}; +inline TwoWire Wire; diff --git a/test/fixtures/ssd1306_power/test.cpp b/test/fixtures/ssd1306_power/test.cpp new file mode 100644 index 000000000..cac66f58a --- /dev/null +++ b/test/fixtures/ssd1306_power/test.cpp @@ -0,0 +1,155 @@ +#include +#include +#include +#include + +static constexpr uint8_t RAIL_PIN = 7; + +static void resetBoundary() { + panel_state = PanelState(); + Wire = TwoWire(); + for (unsigned i = 0; i < 64; ++i) pin_levels[i] = pin_modes[i] = 0; +} + +static void fail(bool allocation) { + panel_state.allocation_ok = !allocation; + Wire.result = allocation ? 0 : 2; +} + +static void repair() { panel_state.allocation_ok = true; Wire.result = 0; } + +static void expectNotReady(SSD1306Display& panel) { + assert(!panel.isOn()); + const unsigned before = panel_state.rotations; + panel.setFlipped(true); + assert(panel.setRotationDegrees(90)); + panel.setFlipped(false); + assert(panel.setRotationDegrees(0)); + assert(panel_state.rotations == before); +} + +static void failedBoot(bool allocation, bool other_owner, bool shared) { + resetBoundary(); + RefCountedDigitalPin rail(RAIL_PIN); + rail.begin(); + if (other_owner) rail.claim(); + SSD1306Display panel(shared ? &rail : nullptr); + fail(allocation); + for (unsigned attempt = 0; attempt < 3; ++attempt) { + // This is the caller's real admission contract: failed panels never get a + // UI task which could later turn them off and release a phantom claim. + DisplayDriver* ui = panel.begin() ? &panel : nullptr; + assert(ui == nullptr); + expectNotReady(panel); + assert(digitalRead(RAIL_PIN) == (other_owner ? HIGH : LOW)); + if (shared && PIN_OLED_RESET >= 0) assert(digitalRead(PIN_OLED_RESET) == LOW); + } + assert(panel_state.begins == 3); + assert(Wire.probes == (allocation ? 0U : 3U)); + // A failed panel owns nothing: repeated off may not steal another owner's + // claim. Once that independent owner stops, the rail must really turn off. + for (unsigned i = 0; i < 3; ++i) panel.turnOff(); + assert(digitalRead(RAIL_PIN) == (other_owner ? HIGH : LOW)); + if (other_owner) rail.release(); + assert(digitalRead(RAIL_PIN) == LOW); + repair(); + assert(panel.begin() && panel.isOn()); + assert(digitalRead(RAIL_PIN) == (shared ? HIGH : LOW)); + assert(panel.begin()); // Already on: do not acquire a second display claim. + panel.turnOff(); + assert(!panel.isOn() && digitalRead(RAIL_PIN) == LOW); + // Claim/release after all failures proves the counter has not underflowed. + rail.claim(); assert(digitalRead(RAIL_PIN) == HIGH); + rail.release(); assert(digitalRead(RAIL_PIN) == LOW); +} + +static void failedReinitialization(bool allocation, bool other_owner, bool shared) { + resetBoundary(); + RefCountedDigitalPin rail(RAIL_PIN); + rail.begin(); + if (other_owner) rail.claim(); + SSD1306Display panel(shared ? &rail : nullptr); + assert(panel.begin()); + const unsigned rotations = panel_state.rotations; + fail(allocation); + assert(!panel.begin()); // Already-on begin has exactly one claim to roll back. + expectNotReady(panel); + assert(panel_state.rotations == rotations); + assert(digitalRead(RAIL_PIN) == (other_owner ? HIGH : LOW)); + if (other_owner) rail.release(); + assert(digitalRead(RAIL_PIN) == LOW); + repair(); + assert(panel.begin()); + panel.turnOff(); + assert(digitalRead(RAIL_PIN) == LOW); +} + +static void failedWake(bool allocation, bool other_owner, bool shared) { + resetBoundary(); + RefCountedDigitalPin rail(RAIL_PIN); + rail.begin(); + if (other_owner) rail.claim(); + SSD1306Display panel(shared ? &rail : nullptr); + assert(panel.begin()); + if (shared) panel.turnOff(); + else { + // A continuously powered panel normally needs no reinit after turnOff. + // Mark it unready through a real failed begin before testing failed wake. + fail(allocation); + assert(!panel.begin()); + } + fail(allocation); + const unsigned commands = panel_state.on_commands, begins = panel_state.begins; + for (unsigned attempt = 0; attempt < 3; ++attempt) { + panel.turnOn(); + expectNotReady(panel); + assert(panel_state.on_commands == commands); + assert(panel_state.begins == begins + attempt + 1); + assert(digitalRead(RAIL_PIN) == (other_owner ? HIGH : LOW)); + } + repair(); + panel.turnOn(); + assert(panel.isOn() && panel_state.on_commands == commands + 1); + assert(digitalRead(RAIL_PIN) == ((shared || other_owner) ? HIGH : LOW)); + panel.turnOff(); + if (other_owner) { + assert(digitalRead(RAIL_PIN) == HIGH); + rail.release(); + } + assert(digitalRead(RAIL_PIN) == LOW); +} + +static void ordinarySuccess(bool shared) { + resetBoundary(); + RefCountedDigitalPin rail(RAIL_PIN); + rail.begin(); + SSD1306Display panel(shared ? &rail : nullptr); + assert(panel.begin()); + assert(panel.isOn() && panel_state.begins == 1 && panel_state.rotations == 1); + panel.turnOn(); + assert(panel_state.begins == 1 && panel_state.on_commands == 1); + panel.turnOff(); + assert(!panel.isOn() && digitalRead(RAIL_PIN) == LOW); + panel.turnOn(); + assert(panel.isOn() && panel_state.begins == (shared ? 2U : 1U)); + assert(panel_state.on_commands == 2); + panel.turnOff(); + mesh::ui::displayPowerPrefs() = mesh::ui::DisplayPowerPrefs(); + mesh::ui::displayPowerPrefs().battery.mode = mesh::ui::DisplayMode::On; + assert(panel.servicePower(false) && panel.isOn()); + mesh::ui::displayPowerPrefs().battery.mode = mesh::ui::DisplayMode::Off; + assert(panel.servicePower(false) && !panel.isOn()); + assert(digitalRead(RAIL_PIN) == LOW); +} + +int main() { + unsigned checks = 0; + for (bool allocation : {false, true}) for (bool other_owner : {false, true}) + for (bool shared : {false, true}) { + failedBoot(allocation, other_owner, shared); ++checks; + failedReinitialization(allocation, other_owner, shared); ++checks; + failedWake(allocation, other_owner, shared); ++checks; + } + for (bool shared : {false, true}) { ordinarySuccess(shared); ++checks; } + printf("PASS: actual SSD1306 power ownership (%u scenarios)\n", checks); +} diff --git a/test/test_ssd1306_power.py b/test/test_ssd1306_power.py new file mode 100644 index 000000000..496ca6dba --- /dev/null +++ b/test/test_ssd1306_power.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +"""Execute the full OLED driver and actual shared-rail reference counting.""" + +from pathlib import Path +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + +from test_replay_reset_integration import extract_braced + +ROOT = Path(__file__).resolve().parents[1] +FIXTURE = ROOT / "test/fixtures/ssd1306_power" +DRIVER = ROOT / "src/helpers/ui/SSD1306Display.cpp" + + +class Ssd1306PowerTests(unittest.TestCase): + def compile_run(self, reset_pin, source=None, expect_failure=False): + compiler = os.environ.get("CXX") or shutil.which("g++") or shutil.which("clang++") + if compiler is None: + self.skipTest("a host C++17 compiler is required") + with tempfile.TemporaryDirectory(prefix="meshcore-ssd1306-power-") as directory: + work = Path(directory) + driver = DRIVER + if source is not None: + driver = work / "SSD1306Display.cpp" + driver.write_text(source, encoding="ascii") + binary = work / "power" + compiled = subprocess.run([ + compiler, "-std=c++17", "-Wall", "-Wextra", "-Werror", + "-Wno-unused-parameter", f"-DPIN_OLED_RESET={reset_pin}", + *(["-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-fno-pie", "-no-pie"] if sys.platform.startswith("linux") else []), + "-I", str(FIXTURE / "mocks"), "-I", str(ROOT / "src"), + "-I", str(ROOT / "src/helpers/ui"), + str(FIXTURE / "test.cpp"), str(driver), + str(ROOT / "src/helpers/ui/DisplayDriver.cpp"), "-o", str(binary), + ], text=True, capture_output=True, timeout=60) + self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr) + checked = subprocess.run([str(binary)], text=True, capture_output=True, timeout=10) + if expect_failure: + self.assertNotEqual(checked.returncode, 0, "negative control escaped the regression") + self.assertIn("Assertion", checked.stderr) + else: + self.assertEqual(checked.returncode, 0, checked.stdout + checked.stderr) + self.assertIn("PASS: actual SSD1306 power ownership (26 scenarios)", checked.stdout) + + def test_actual_failure_retry_wake_and_shared_owner_lifecycle(self): + for reset_pin in (21, -1): + with self.subTest(reset_pin=reset_pin): + self.compile_run(reset_pin) + + def test_missing_rollback_and_failed_wake_negative_controls(self): + source = DRIVER.read_text() + begin = extract_braced(source, "bool SSD1306Display::begin(") + old_begin = begin[:begin.index(" } else {\n _panel_ready = false;")] + " }\n return started;\n}" + self.compile_run(21, source.replace(begin, old_begin), expect_failure=True) + stopped_wake = "if (!begin()) return; // begin owns and rolls back its claim on failure" + self.assertEqual(source.count(stopped_wake), 1) + self.compile_run(21, source.replace(stopped_wake, "begin();"), expect_failure=True) + + +if __name__ == "__main__": + unittest.main()