Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 156 additions & 0 deletions docs/DISPLAY_BATTERY_AUTOSLEEP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
# Display: no-controller auto-sleep + battery indicator

**Scope: display firmware only (LilyGo T-RGB, `env:display`).**
No changes to the controller firmware, coffee-control logic (heater/pump/valve/pressure/
temperature), profile schema/engine, BLE control commands, bootloader, partition table,
eFuse, Secure Boot, or Flash Encryption.

Base: forked from `jniebuhr/gaggimate` upstream `master` @ `591abe74`.

---

## Goals

1. **Auto-sleep when no controller** — if the display has no BLE connection to the
GaggiMate Controller (PCB) for **120 s**, it enters deep sleep to save battery. It
wakes on **touch** (the chip reboots and re-scans for the controller).
2. **Battery indicator** — show the single-cell 3.7 V Li-ion state (approx. percentage
**and** voltage) on the display.
3. **Official firmware stays recoverable** — no bootloader/partition/flash-security
changes; re-flash the official build to revert.

---

## What changed (display-only)

| File | Change |
|------|--------|
| `src/display/core/constants.h` | Auto-sleep + battery default constants |
| `src/display/drivers/Driver.h` | Add `sleep()`, `hasBattery()`, `getBatteryMilliVolts()` (default no-op / 0) |
| `src/display/drivers/LilyGoDriver.h` | Implement them via `panel.enableTouchWakeup()/sleep()` and `panel.getBattVoltage()` |
| `src/display/core/AutoSleepManager.h` | New pure-logic module: no-controller sleep policy |
| `src/display/core/BatteryMonitor.h` | New pure-logic module: voltage → approx. % (piecewise LUT) |
| `src/display/core/Settings.{h,cpp}` | `autoSleepNoController` (default on) + `noControllerSleepTimeout` (default 120 s) |
| `src/display/plugins/WebUIPlugin.cpp` | Read/serialize the two new settings (checkbox uses the `hasArg` presence convention, like `clock24hFormat`) |
| `web/src/pages/Settings/index.jsx` | Display Settings → **Auto Sleep**: enable toggle + timeout (s) input |
| `src/display/ui/default/DefaultUI.{h,cpp}` | Drive both managers; battery overlay on the LVGL top layer |
| `sim/driver/SdlDriver.h` | Mock battery (`GM_SIM_BATTERY_MV`, default 3900) + mock sleep (logs, never exits) |

No new BLE commands. The display only **reads** BLE connection state and **never**
sends standby/brew/stop/valve/pump/heater commands before sleeping.

---

## Behaviour

### Auto-sleep (`AutoSleepManager`)
- Countdown starts at boot and whenever the controller disconnects.
- Reconnect within the timeout cancels it.
- Touch resets the UI idle timer but does **not** block the no-controller countdown.
- Suppressed during OTA / firmware update / Wi-Fi AP setup / autotune.
- If the battery is **critical** (< 3400 mV) and no controller, it sleeps after ¼ of the timeout.
- Configurable in the **Web UI**: *Settings → Display Settings → Auto Sleep* (enable toggle +
timeout in seconds), backed by `Settings.autoSleepNoController` / `noControllerSleepTimeout`.
Compile-time defaults live in `constants.h`.

### Wake behaviour
- The LilyGo T-RGB display enters ESP32-S3 deep sleep with touch IRQ configured as
the wake source (`panel.enableTouchWakeup(); panel.sleep();`).
- Touch wake does **not** mean the main CPU keeps polling the touch panel. In deep
sleep, the CPU/BLE/Wi-Fi/LVGL are stopped; only the low-power wake circuitry and
touch interrupt path remain active.
- A wake from deep sleep is a reboot. The display firmware starts fresh and scans
for the controller again.
- The controller cannot wake a sleeping display over BLE because BLE is off during
deep sleep. If the display is already asleep, the user must touch it to wake it.
- A future timer-wake mode could periodically wake, scan for the controller, and go
back to sleep, but that is intentionally not part of this display-only touch-wake
implementation because it increases battery use.

### Battery (`BatteryMonitor`)
- Sampled every 30 s via `Driver::getBatteryMilliVolts()` → on the T-RGB this is
`panel.getBattVoltage()`, which averages ~20 ADC reads on `BOARD_ADC_DET` (GPIO4) and
already accounts for the on-board 1/2 divider.
- Percentage is an **approximation** (coarse Li-ion LUT, 3400 mV = 0 %, 4200 mV = 100 %).
Only the **percentage** is shown (no voltage), on a small overlay on the LVGL top layer:
`<icon> NN%`, white normally, amber < 3500 mV, red < 3400 mV.
- **Charging / USB:** the T-RGB has no charge-status pin, so charging is *inferred* — while
USB is plugged the reading tracks the charging voltage (≥ `BATTERY_CHARGING_MV` = 4200 mV
once it tops off). In that case the overlay shows a **charging bolt + percentage**
(`⚡ NN%`, green) — the bolt distinguishes charging from on-battery, while the percentage
(charging-voltage based, so it trends toward ~100 %) is still visible.

> **USB caveat:** because there is no charge-status pin, "charging" is a voltage heuristic, not
> a true charge signal: a battery genuinely at 100 % reads the same as one being charged. The
> reading is meaningful as a *percentage* only on battery power.

### Screenshots (from the simulator)

Top-centre overlay, inside the round panel — percentage only on battery, charging
indicator on USB:

| Normal (`GM_SIM_BATTERY_MV=3950`) | Charging / USB (`=4250`) | Critical (`=3380`) |
|---|---|---|
| ![normal](img/standby-battery.png) | ![charging](img/standby-battery-charging.png) | ![low](img/standby-battery-low.png) |

Captured headlessly with `./.pio/build/display-sim/program --screenshot shot.bmp 4500`.
The "starting / waiting-for-controller" view uses the same standby screen, so the overlay
appears there too.

---

## Build / test

> Note: PlatformIO's `display-sim` env breaks if the project path contains a **space**
> (unquoted `-I ${PROJECT_DIR}/sim/...` flags). Build the sim from a space-free path.
> The device `display` env builds fine regardless. The build scripts need **Python 3.11+**
> (`datetime.UTC`).

```bash
# Device firmware
pio run -e display

# Static analysis
pio check -e display

# Simulator (from a path without spaces)
pio run -e display-sim -t run
# simulate battery levels:
GM_SIM_BATTERY_MV=3400 pio run -e display-sim -t run
# format
scripts/format.sh
```

### Manual test checklist (real T-RGB)
1. Display on battery, controller off → "waiting", battery visible, **sleeps after 120 s**, wakes on touch.
2. Controller turned on within 120 s → connects, does **not** sleep.
3. Connected, then controller off → "waiting", sleeps after 120 s.
4. Controller back within 120 s → does **not** sleep.
5. USB to a PC → flashes, serial logs OK; auto-sleep can be disabled via the setting while debugging.
6. Simulator → mock battery visible; `[sim] AutoSleep: enterDisplaySleep() (mock, not sleeping)` logged instead of exiting.

### Hardware test record (fill in on a real T-RGB)

| # | Scenario | Expected | Result (PASS/FAIL + notes) |
|---|----------|----------|----------------------------|
| 1 | Battery power, controller off | Sleeps ~120 s after boot | **PASS** — verified via a 15 s test build (USB-CDC port dropped = deep sleep, screen black) |
| 2 | Battery power, touch the sleeping screen | Wakes (reboots), re-scans BLE | **PASS** — touch woke the CST820 panel and the board rebooted |
| 3 | Controller turned on within 120 s | Connects, does **not** sleep | _todo_ |
| 4 | Connected, then controller off | Sleeps ~120 s after disconnect | _todo_ |
| 5 | Controller back within 120 s of disconnect | Does **not** sleep | _todo_ |
| 6 | Display asleep, then controller powered on | Display stays asleep until touched; no BLE wake | _todo_ |
| 7 | Battery in, then plug USB-C charger | Overlay shows **⚡ NN%** (charging bolt + percentage) | _todo_ |
| 8 | Battery overlay on battery power | Approx **% only** (no voltage); amber < 3.5 V, red < 3.4 V | _todo_ |

> Status: core path **verified on a real LilyGo T-RGB** (touch IC: **CST820**) — no-controller
> deep sleep and **touch wake** both work (items 1–2). Builds (display + display-sim), static
> check (space-free path) and web UI all pass. Items 3–8 remain to confirm with a controller
> and on battery power.

---

## Recovery

Auto-sleep is a setting (default on) — turn it off in the web config to disable. To fully
revert, re-flash the official display firmware from <https://docs.gaggimate.eu/docs/flashing/>.
Nothing in bootloader / partitions / flash security is touched.
Binary file added docs/img/standby-battery-charging.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/img/standby-battery-low.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/img/standby-battery.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions sim/driver/SdlDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
// Driver interface the hardware panels use.
#pragma once

#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <display/drivers/Driver.h>

class SdlDriver : public Driver {
Expand All @@ -13,6 +16,18 @@ class SdlDriver : public Driver {
bool supportsSDCard() override { return false; }
bool installSDCard() override { return false; }

// [display-auto-sleep] The simulator must never really sleep/exit; just record
// the event so auto-sleep behaviour can be exercised on the desktop.
void sleep() override { printf("[sim] AutoSleep: enterDisplaySleep() (mock, not sleeping)\n"); }

// [display-battery] Mock battery for the simulator. Defaults to 3900 mV; override
// with the GM_SIM_BATTERY_MV environment variable (e.g. 4200 / 3700 / 3400).
bool hasBattery() override { return true; }
uint16_t getBatteryMilliVolts() override {
const char *mv = getenv("GM_SIM_BATTERY_MV");
return static_cast<uint16_t>(mv ? atoi(mv) : 3900);
}
Comment on lines +26 to +29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate and clamp GM_SIM_BATTERY_MV before casting.

atoi() + direct uint16_t cast can wrap invalid inputs (e.g., -1 becomes 65535), which distorts simulated battery percentage and auto-sleep behavior.

Suggested fix
 uint16_t getBatteryMilliVolts() override {
-    const char *mv = getenv("GM_SIM_BATTERY_MV");
-    return static_cast<uint16_t>(mv ? atoi(mv) : 3900);
+    const char *mv = std::getenv("GM_SIM_BATTERY_MV");
+    if (mv == nullptr) {
+        return 3900;
+    }
+    char *end = nullptr;
+    const long parsed = std::strtol(mv, &end, 10);
+    if (end == mv || *end != '\0' || parsed < 0 || parsed > 5000) {
+        return 3900;
+    }
+    return static_cast<uint16_t>(parsed);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uint16_t getBatteryMilliVolts() override {
const char *mv = getenv("GM_SIM_BATTERY_MV");
return static_cast<uint16_t>(mv ? atoi(mv) : 3900);
}
uint16_t getBatteryMilliVolts() override {
const char *mv = std::getenv("GM_SIM_BATTERY_MV");
if (mv == nullptr) {
return 3900;
}
char *end = nullptr;
const long parsed = std::strtol(mv, &end, 10);
if (end == mv || *end != '\0' || parsed < 0 || parsed > 5000) {
return 3900;
}
return static_cast<uint16_t>(parsed);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sim/driver/SdlDriver.h` around lines 26 - 29, The getBatteryMilliVolts()
method directly casts the result of atoi() to uint16_t without validating the
input, which causes negative values to wrap around (e.g., -1 becomes 65535).
Modify the method to validate and clamp the integer value returned by atoi() to
ensure it falls within a valid range for uint16_t before performing the cast,
treating invalid or out-of-range values as the default 3900 millivolts instead.


static SdlDriver *getInstance() {
if (instance == nullptr)
instance = new SdlDriver();
Expand Down
95 changes: 95 additions & 0 deletions src/display/core/AutoSleepManager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
#ifndef AUTOSLEEPMANAGER_H
#define AUTOSLEEPMANAGER_H

#include <cstdint>

#include "constants.h"

// [display-auto-sleep] Display-only policy deciding when the screen should enter
// deep sleep because the GaggiMate Controller (PCB) has been unreachable over BLE
// for too long. This is pure logic with NO hardware access and NO BLE/controller
// writes, so it also compiles and runs in the simulator and is easy to reason about.
//
// The only sleep reason is "no controller for N ms". Touch resets the UI idle timer
// but never blocks the no-controller countdown (matches the spec). Sleep is also
// suppressed while OTA / firmware update / Wi-Fi setup / web flows are in progress.
class AutoSleepManager {
public:
enum class SleepReason { None, NoController };

void setEnabled(bool enabled) { enabled_ = enabled; }
bool isEnabled() const { return enabled_; }

void setTimeoutMs(uint32_t timeoutMs) { timeoutMs_ = timeoutMs; }
uint32_t timeoutMs() const { return timeoutMs_; }

// Temporarily block sleeping (OTA / update / Wi-Fi AP setup / web management).
void setSuppressed(bool suppressed) { suppressed_ = suppressed; }
bool isSuppressed() const { return suppressed_; }

// When true and the controller is absent, sleep after a quarter of the timeout
// (used when the battery is critically low). Display-only behaviour.
void setForceShortTimeout(bool force) { forceShortTimeout_ = force; }

bool isControllerConnected() const { return controllerConnected_; }
uint32_t controllerDisconnectedSince() const { return controllerDisconnectedSince_; }

// Begin with no connection yet (start the countdown at boot).
void begin(uint32_t now) {
controllerConnected_ = false;
controllerDisconnectedSince_ = now ? now : 1; // 0 means "not counting"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix immediate-sleep underflow when timestamp is 0.

Line 40 and Line 54 coerce now == 0 to 1, but Line 69 computes now - controllerDisconnectedSince_ as unsigned math. At now == 0, that underflows and can instantly satisfy timeout, causing an unintended deep sleep right at boot/disconnect edge.

💡 Suggested fix (separate “counting active” state from timestamp value)
 class AutoSleepManager {
   public:
@@
     void begin(uint32_t now) {
         controllerConnected_ = false;
-        controllerDisconnectedSince_ = now ? now : 1; // 0 means "not counting"
+        disconnectCountdownActive_ = true;
+        controllerDisconnectedSince_ = now;
         lastUserInteractionAt_ = now;
     }
@@
     void onControllerConnected(uint32_t now) {
         controllerConnected_ = true;
         lastControllerConnectedAt_ = now;
-        controllerDisconnectedSince_ = 0; // cancel the countdown
+        disconnectCountdownActive_ = false; // cancel the countdown
+        controllerDisconnectedSince_ = 0;
     }
@@
     void onControllerDisconnected(uint32_t now) {
@@
-        if (controllerConnected_ || controllerDisconnectedSince_ == 0) {
-            controllerDisconnectedSince_ = now ? now : 1;
+        if (controllerConnected_ || !disconnectCountdownActive_) {
+            controllerDisconnectedSince_ = now;
+            disconnectCountdownActive_ = true;
         }
         controllerConnected_ = false;
     }
@@
     SleepReason evaluate(uint32_t now) const {
-        if (!enabled_ || suppressed_ || controllerConnected_ || controllerDisconnectedSince_ == 0) {
+        if (!enabled_ || suppressed_ || controllerConnected_ || !disconnectCountdownActive_) {
             return SleepReason::None;
         }
         const uint32_t timeout = forceShortTimeout_ ? (timeoutMs_ / 4) : timeoutMs_;
         if (now - controllerDisconnectedSince_ >= timeout) {
             return SleepReason::NoController;
         }
         return SleepReason::None;
     }
@@
   private:
@@
+    bool disconnectCountdownActive_ = false;
     uint32_t controllerDisconnectedSince_ = 0;

Also applies to: 54-54, 69-69

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/display/core/AutoSleepManager.h` at line 40, The code uses 0 as a
sentinel value to indicate "not counting" for the controllerDisconnectedSince_
variable, but coerces it to 1 on lines 40 and 54 to avoid sentinel issues.
However, line 69 performs unsigned subtraction (now -
controllerDisconnectedSince_), which causes an underflow when now equals 0,
resulting in a very large value that immediately satisfies the timeout check and
triggers unintended deep sleep at boot/disconnect. Fix this by separating the
"counting active" state from the timestamp value itself—use a distinct boolean
flag to track whether the controller disconnection timer is actively counting,
rather than relying on sentinel values in the timestamp field. Update lines 40,
54, and 69 to check this flag instead of relying on the 0-to-1 coercion and
unsigned math behavior.

lastUserInteractionAt_ = now;
}

void onControllerConnected(uint32_t now) {
controllerConnected_ = true;
lastControllerConnectedAt_ = now;
controllerDisconnectedSince_ = 0; // cancel the countdown
}

void onControllerDisconnected(uint32_t now) {
// Only (re)start the countdown on a real transition, so repeated
// "waiting" events don't keep pushing the deadline forward.
if (controllerConnected_ || controllerDisconnectedSince_ == 0) {
controllerDisconnectedSince_ = now ? now : 1;
}
controllerConnected_ = false;
}

// Resets the UI idle timer only; does NOT block no-controller sleep.
void onUserInteraction(uint32_t now) { lastUserInteractionAt_ = now; }
uint32_t lastUserInteractionAt() const { return lastUserInteractionAt_; }

// Returns why the display should sleep right now, or None.
SleepReason evaluate(uint32_t now) const {
if (!enabled_ || suppressed_ || controllerConnected_ || controllerDisconnectedSince_ == 0) {
return SleepReason::None;
}
const uint32_t timeout = forceShortTimeout_ ? (timeoutMs_ / 4) : timeoutMs_;
if (now - controllerDisconnectedSince_ >= timeout) {
return SleepReason::NoController;
}
return SleepReason::None;
}

static const char *reasonName(SleepReason r) {
switch (r) {

Check warning on line 76 in src/display/core/AutoSleepManager.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this "switch" statement by "if" statements to increase readability.

See more on https://sonarcloud.io/project/issues?id=jniebuhr_gaggimate&issues=AZ7JTm-Hz0W_bWZePj_w&open=AZ7JTm-Hz0W_bWZePj_w&pullRequest=773
case SleepReason::NoController:
return "NO_CONTROLLER";
default:
return "NONE";
}
}

private:
bool enabled_ = DEFAULT_AUTO_SLEEP_NO_CONTROLLER;
bool suppressed_ = false;
bool forceShortTimeout_ = false;
bool controllerConnected_ = false;
uint32_t timeoutMs_ = DEFAULT_NO_CONTROLLER_SLEEP_TIMEOUT_MS;
uint32_t controllerDisconnectedSince_ = 0;
uint32_t lastControllerConnectedAt_ = 0;
uint32_t lastUserInteractionAt_ = 0;
};

#endif // AUTOSLEEPMANAGER_H
67 changes: 67 additions & 0 deletions src/display/core/BatteryMonitor.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#ifndef BATTERYMONITOR_H
#define BATTERYMONITOR_H

#include <cstdint>

#include "constants.h"

// [display-battery] Display-only single-cell (3.7V) Li-ion telemetry. Pure logic
// (no hardware) so it also builds in the simulator. Feed it sampled millivolts; it
// stores the latest reading and maps it to an APPROXIMATE percentage via a coarse
// piecewise lookup table. The percentage is intentionally approximate — the UI also
// shows the raw voltage. This value is NEVER used for any controller/coffee decision.
//
// Caveat (documented in the UI/docs): when USB is plugged in, the measured voltage
// can reflect the charging voltage rather than the true remaining battery charge.
class BatteryMonitor {
public:
void update(uint16_t voltageMv, uint32_t now) {
voltageMv_ = voltageMv;
valid_ = voltageMv_ > 0;
percent_ = valid_ ? percentFromMv(voltageMv_) : 0;
lastSampleAt_ = now;
}

bool isValid() const { return valid_; }
uint16_t voltageMv() const { return voltageMv_; }
uint8_t percent() const { return percent_; }
uint32_t lastSampleAt() const { return lastSampleAt_; }
bool isLow() const { return valid_ && voltageMv_ < BATTERY_LOW_MV; }
bool isCritical() const { return valid_ && voltageMv_ < BATTERY_CRITICAL_MV; }
// Heuristic "USB plugged / charging": with no charge-status pin, a reading pinned
// at/above a full cell means external power is present (see BATTERY_CHARGING_MV).
bool isCharging() const { return valid_ && voltageMv_ >= BATTERY_CHARGING_MV; }

// Coarse Li-ion approximation with linear interpolation between table points.
static uint8_t percentFromMv(uint16_t mv) {
struct Point {
uint16_t mv;
uint8_t pct;
};
// Descending by voltage.
static const Point lut[] = {

Check warning on line 42 in src/display/core/BatteryMonitor.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use "std::array" or "std::vector" instead of a C-style array.

See more on https://sonarcloud.io/project/issues?id=jniebuhr_gaggimate&issues=AZ7JTm18z0W_bWZePj_t&open=AZ7JTm18z0W_bWZePj_t&pullRequest=773
{4200, 100}, {4100, 90}, {4000, 80}, {3900, 65}, {3800, 50}, {3700, 30}, {3600, 15}, {3500, 5}, {3400, 0},
};
const int n = sizeof(lut) / sizeof(lut[0]);
if (mv >= lut[0].mv)
return 100;
if (mv <= lut[n - 1].mv)
return 0;
for (int i = 0; i < n - 1; i++) {
if (mv <= lut[i].mv && mv > lut[i + 1].mv) {
const uint16_t hiMv = lut[i].mv, loMv = lut[i + 1].mv;

Check warning on line 52 in src/display/core/BatteryMonitor.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define each identifier in a dedicated statement.

See more on https://sonarcloud.io/project/issues?id=jniebuhr_gaggimate&issues=AZ7JTm18z0W_bWZePj_u&open=AZ7JTm18z0W_bWZePj_u&pullRequest=773
const uint8_t hiPct = lut[i].pct, loPct = lut[i + 1].pct;

Check warning on line 53 in src/display/core/BatteryMonitor.h

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define each identifier in a dedicated statement.

See more on https://sonarcloud.io/project/issues?id=jniebuhr_gaggimate&issues=AZ7JTm18z0W_bWZePj_v&open=AZ7JTm18z0W_bWZePj_v&pullRequest=773
return loPct + (uint32_t)(mv - loMv) * (hiPct - loPct) / (hiMv - loMv);
}
}
return 0;
}

private:
bool valid_ = false;
uint16_t voltageMv_ = 0;
uint8_t percent_ = 0;
uint32_t lastSampleAt_ = 0;
};

#endif // BATTERYMONITOR_H
16 changes: 16 additions & 0 deletions src/display/core/Settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ Settings::Settings() {
mainBrightness = preferences.getInt("main_b", 16);
standbyBrightness = preferences.getInt("standby_b", 8);
standbyBrightnessTimeout = preferences.getInt("standby_bt", 60000);
// [display-auto-sleep] display-only
autoSleepNoController = preferences.getBool("as_noctl", DEFAULT_AUTO_SLEEP_NO_CONTROLLER);
noControllerSleepTimeout = preferences.getInt("as_noctl_t", DEFAULT_NO_CONTROLLER_SLEEP_TIMEOUT_MS);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clamp persisted and incoming noControllerSleepTimeout values.

Line [99] and Line [381] accept raw int timeout values; those flow into AutoSleepManager::setTimeoutMs(uint32_t) (see src/display/ui/default/DefaultUI.cpp Line [362]). Negative/zero/corrupt values can wrap or trigger unintended immediate sleep behavior.

🔧 Proposed fix
-    noControllerSleepTimeout = preferences.getInt("as_noctl_t", DEFAULT_NO_CONTROLLER_SLEEP_TIMEOUT_MS);
+    noControllerSleepTimeout =
+        std::clamp(preferences.getInt("as_noctl_t", DEFAULT_NO_CONTROLLER_SLEEP_TIMEOUT_MS),
+                   NO_CONTROLLER_SLEEP_CHECK_INTERVAL_MS, 24 * 60 * 60 * 1000);

 void Settings::setNoControllerSleepTimeout(int timeout_ms) {
-    noControllerSleepTimeout = timeout_ms;
+    noControllerSleepTimeout =
+        std::clamp(timeout_ms, NO_CONTROLLER_SLEEP_CHECK_INTERVAL_MS, 24 * 60 * 60 * 1000);
     save();
 }

Also applies to: 380-383

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/display/core/Settings.cpp` at line 99, The noControllerSleepTimeout
values read via preferences.getInt() are not being validated or clamped to
acceptable ranges before flowing into AutoSleepManager::setTimeoutMs(uint32_t).
Negative, zero, or corrupt values can cause wrap-around or trigger unintended
immediate sleep behavior. Clamp the noControllerSleepTimeout values at both the
point where they are read from preferences and where they are set from incoming
values to ensure they fall within a valid timeout range before being passed to
the AutoSleepManager method.

wifiApTimeout = preferences.getInt("wifi_apt", DEFAULT_WIFI_AP_TIMEOUT_MS);
themeMode = preferences.getInt("theme", 0);

Expand Down Expand Up @@ -369,6 +372,16 @@ void Settings::setStandbyBrightnessTimeout(int standby_brightness_timeout) {
save();
}

void Settings::setAutoSleepNoController(bool enabled) {
autoSleepNoController = enabled;
save();
}

void Settings::setNoControllerSleepTimeout(int timeout_ms) {
noControllerSleepTimeout = timeout_ms;
save();
}

void Settings::setWifiApTimeout(int timeout) {
wifiApTimeout = timeout;
save();
Expand Down Expand Up @@ -564,6 +577,9 @@ void Settings::doSave() {
preferences.putInt("main_b", mainBrightness);
preferences.putInt("standby_b", standbyBrightness);
preferences.putInt("standby_bt", standbyBrightnessTimeout);
// [display-auto-sleep] display-only
preferences.putBool("as_noctl", autoSleepNoController);
preferences.putInt("as_noctl_t", noControllerSleepTimeout);
preferences.putInt("wifi_apt", wifiApTimeout);
preferences.putInt("theme", themeMode);

Expand Down
Loading