Release OLED power ownership when initialization fails

.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

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);
 }

test/fixtures/ssd1306_power/mocks/Adafruit_GFX.h

@@ -0,0 +1 @@
+#pragma once

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 <Arduino.h>
+#include <Wire.h>
+#include <string.h>
+
+#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;
+  }
+};

test/fixtures/ssd1306_power/mocks/Arduino.h

@@ -0,0 +1,27 @@
+#pragma once
+
+#include <assert.h>
+#include <stddef.h>
+#include <stdint.h>
+
+#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];
+}

test/fixtures/ssd1306_power/mocks/Wire.h

@@ -0,0 +1,11 @@
+#pragma once
+
+#include <stdint.h>
+
+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;

test/fixtures/ssd1306_power/test.cpp

@@ -0,0 +1,155 @@
+#include <helpers/ui/SSD1306Display.h>
+#include <assert.h>
+#include <initializer_list>
+#include <stdio.h>
+
+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);
+}

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()