Skip to content

fix(skins): restore brightness when skin view closes - #643

Merged
tadelv merged 3 commits into
mainfrom
odev/issue-621-restore-brightness
Aug 24, 2026
Merged

fix(skins): restore brightness when skin view closes#643
tadelv merged 3 commits into
mainfrom
odev/issue-621-restore-brightness

Conversation

@ODevStudio

@ODevStudio ODevStudio commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

What changed, and why?

  • Give the embedded SkinView the production DisplayController and restore
    OS-managed brightness through setBrightness(100) when the view is removed.
  • Define native UI as the brightness authority after skin exit, overriding any
    active REST or WebSocket fixed-brightness request.
  • Document that clients must request fixed brightness again after skin exit.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore / infra
  • Plugin (DYE2 or bundled skin)

Scope (select all touched areas)

  • BLE transport / device comms
  • REST API / handlers
  • WebSocket API
  • Machine state / shot logic
  • Scale / weight / flow
  • Profiles / beans / grinders / workflows
  • WebUI skins
  • Plugins / JS runtime
  • UI / Flutter widgets
  • Storage / Drift database
  • CI / build / infra
  • Docs / specs

Linked Issues

Fixes #621

Android tablet brightness restoration is validated. No additional hardware test is required for this display-lifecycle change.

Root Cause (if bug fix)

  • Root cause: Skin-requested brightness outlived the embedded SkinView; the
    view did not own the production controller needed to restore OS-managed
    brightness when it closed.
  • Missing detection or guardrail: There was no widget lifecycle test for
    brightness cleanup on SkinView removal.
  • Contributing context: Display WebSocket cleanup cannot own this because one
    client disconnect must not reset another client's brightness.

Regression Test Plan (if bug fix or refactor)

  • Coverage level that should have caught this:
    • Unit test
    • Integration test (mock transport edge)
    • End-to-end test (simulate=1 + curl/websocat)
    • Existing coverage already sufficient
  • Target test or file: test/unit/skin_feature/skin_view_brightness_test.dart.
  • Scenario the test should lock in: Removing SkinView while another client
    requests brightness 40 replaces that request with 100 and resets platform
    brightness exactly once.
  • If no new test added, why not: N/A; a lifecycle regression test was added.

Documentation Obligations (required)

  • API spec updated: assets/api/rest_v1.yml or assets/api/websocket_v1.yml (if REST/WebSocket changed)
  • API docs updated: doc/Api.md (if user-facing endpoint changed)
  • Plugin docs updated: doc/Plugins.md (if events/API changed)
  • Skin docs updated: doc/Skins.md (if skin behavior changed)
  • Profile docs updated: doc/Profiles.md (if profile handling changed)
  • Device docs updated: doc/DeviceManagement.md (if device flows changed)
  • N/A - no docs affected

Security Impact (required)

  • New or changed REST endpoints? No.
  • New or changed WebSocket topics? No.
  • New or changed network calls? No.
  • BLE/USB surface changed? No.
  • File system access changed? No.
  • Plugin sandbox boundary changed? No.
  • If any Yes, explain risk and mitigation: N/A.

User-Visible Changes

  • Leaving the embedded skin now discards any skin-requested fixed brightness
    and returns the display to OS-managed brightness.
  • An unrelated display WebSocket disconnect still cannot reset another
    client's brightness.

Verification

Local gates (run before pushing)

  • dart format lib test - no remaining candidate changes
  • flutter analyze - clean
  • flutter test - 3,179 passed, 1 skipped
  • ./scripts/fetch_dye2_plugin.sh - not rerun for this local draft

Manual verification (if applicable)

  • OS / platform tested: Windows test host and physical Samsung Galaxy Tab A9+ (Android 15, 1920 x 1200).
  • Simulated devices? (simulate=1): Yes.
  • Real hardware? (DE1/Bengle/scale): No; physical Android tablet only.
  • What you personally verified and how: Computer Use confirmed the skin ran on the Samsung tablet and Android Back removed the embedded SkinView. A REST request fixed brightness at 10%; Android then reported a 0.1 window brightness override. Leaving the skin cleared Decaid's physical override and returned brightness control to Android; adaptive brightness remained low in the current lighting.
  • Edge cases checked: View removal while a competing client requests fixed
    brightness, and separation from WebSocket client cleanup.
  • What you did not verify: Real DE1, Bengle, or scale hardware; not applicable to display brightness cleanup.

Evidence

  • Test output (failing before + passing after)
  • Log snippets
  • Screenshot / recording (UI changes)
  • curl / websocat output (API changes)

Detailed evidence:

  • Regression-first proof: the new lifecycle test failed before implementation
    because SkinView had no host brightness restore hook.
  • flutter test --no-pub test/unit/skin_feature/skin_view_brightness_test.dart test/controllers/display_controller_test.dart
    (41 passed).
  • dart format --output=none --set-exit-if-changed lib/main.dart lib/src/app.dart lib/src/skin_feature/skin_view.dart test/unit/skin_feature/skin_view_brightness_test.dart
  • flutter analyze --no-pub (no issues).
  • flutter test --no-pub with the package-matched QuickJS DLL on PATH
    (3,179 passed, 1 skipped).
  • Samsung Galaxy Tab A9+ physical smoke test: PUT /api/v1/display/brightness with 10 returned requested and applied brightness 10. Android dumpsys display reported screenBrightnessOverride=0.1, reason=override, and OverrideBrightnessStrategy.
  • After Android Back removed SkinView, Decaid reported requested and applied brightness 100. Android reported screenBrightnessOverride=NaN, reason=automatic, and InvalidBrightnessStrategy; automatic-brightness mode remained enabled. The panel did not visibly brighten because Android selected approximately 4% for the current lighting.
  • Computer Use confirmed the supported skin-to-Dashboard bridge removes the
    live SkinView and renders the native launcher on Windows.

Compatibility & Migration

  • Backward compatible? Yes.
  • Config / env changes needed? No.
  • Database migration needed? No.
  • Exact steps: None.

Risks & Mitigations

  • Risk: A REST or WebSocket client may expect fixed brightness to survive after
    the skin UI is gone.
    • Mitigation: The documented authority boundary makes native UI authoritative
      on skin exit; clients can request fixed brightness again afterward.

Contributor Responsibility

AI-assisted development is allowed. The submitter remains responsible for the submitted work.

  • I have reviewed and understand all changes in this PR and take responsibility for their correctness, security, behavior, licensing, and provenance, including any AI-assisted or AI-generated work.

Reset display brightness to OS-managed mode when SkinView is removed so a sleeping skin cannot leave the native UI unreadably dim.
@ODevStudio
ODevStudio marked this pull request as ready for review August 22, 2026 14:07
@ODevStudio
ODevStudio requested a review from tadelv August 22, 2026 15:15

@tadelv tadelv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. The host-side lifecycle boundary is the right place to restore brightness when leaving SkinView, and the regression coverage is appropriate.

Optional follow-up: the docs currently say closing the embedded skin resets brightness to 100 / OS-managed. With lowBatteryBrightnessLimit active below 30%, setBrightness(100) is still capped by native policy, so effective brightness remains 20 rather than resetting the platform override. I think that behavior is desirable; it would just be worth tightening the wording (and optionally adding a small regression case) to say that the skin's fixed-brightness request is discarded and native display policy becomes authoritative, including the low-battery cap.

@tadelv
tadelv merged commit 203aec7 into main Aug 24, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Skin view does not restore display brightness when pushed off the navigation stack while machine is sleeping

2 participants