Push the E3 sensor feed at a fixed 1 Hz keep-alive to stop widget freezes - #20
Merged
Conversation
…ezes The panel firmware freezes its on-screen sensor widgets (the recurring "stuck at 25W with no movement", sometimes blank) when the E3 SensorFeed is pushed slower than ~1 Hz. PROTOCOL.md and README already document this requirement, and Gigabyte's own AorusLcdService pushes E3 with a fixed 1000 ms sleep regardless of widget count or rotation interval. SensorFeedTiming.PollIntervalMs conflated the push cadence with the widget rotation interval: for a multi-widget dashboard it returned the rotation interval clamped up to 5 s, so a 2-widget / 4 s setup pushed E3 only every 4 s (0.25 Hz) and starved the keep-alive. Single-widget setups pushed at 1 Hz and never froze, matching the observed behavior. Replace the adaptive cadence with a fixed KeepAlivePollMs = 1000. The rotation interval is unchanged and still delivered to the panel via E1 SetDisplay, which the firmware handles on its own. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 33af0397-3611-492d-aa25-ef398403aba9
There was a problem hiding this comment.
Pull request overview
This PR fixes LCD widget freezes by decoupling the E3 SensorFeed push cadence from the dashboard rotation interval and enforcing a fixed ~1 Hz keep-alive, matching the documented/proven firmware expectations.
Changes:
- Replace adaptive E3 polling with a fixed
SensorFeedTiming.KeepAlivePollMs = 1000. - Update the background service feed loop to use the fixed keep-alive cadence.
- Update protocol documentation and add tests to pin the cadence at 1000 ms.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/AorusLcd.Tests/SensorFeedTimingTests.cs | Adds tests to pin/guard the E3 keep-alive cadence at 1000 ms. |
| src/AorusLcd.Service/FeedWorker.cs | Uses the fixed keep-alive poll interval when running the E3 feed loop. |
| src/AorusLcd.Core/Sensors/SensorFeedTiming.cs | Removes adaptive polling and defines a single fixed keep-alive constant. |
| docs/PROTOCOL.md | Updates documentation to reflect the fixed ~1 Hz E3 keep-alive and its rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JustinMDotNet
added a commit
that referenced
this pull request
Aug 7, 2026
## Problem The LCD Edge View panel intermittently freezes with stale content. This has recurred three times: prior fixes added I2C write retries (#17) and a fixed 1 Hz sensor keep-alive (#20), but both only masked the symptom, the GPU I2C engine rejecting otherwise-valid panel writes with status `-1`. ## Root cause Decompiling Gigabyte Control Center's `AorusLcdService.exe` (the always-on LCD feed component) shows its `I2CApi.SendData` pins the GPU I2C bus to **400 kHz** (`nI2cSpeed = 400`). Everything else matches this project byte-for-byte: the `E3` sensor packet layout and the fixed 1000 ms feed pacing are identical. This project instead passed `NVAPI_I2C_SPEED_DEFAULT` (`i2cSpeedKhz = 0`). NVAPI defines `DEFAULT` as "the controller's current/default (unspecified) speed", and at that speed the I2C engine intermittently rejects valid writes and the panel content freezes. `NV_I2C_INFO_V3.i2cSpeedKhz` takes the discrete `NV_I2C_SPEED` enum (DEFAULT=0 ... 100KHZ=4, 200KHZ=5, 400KHZ=6), not a literal kHz number. ## Change - Add `NvApiI2cSpeed`, a typed enum mirroring NVAPI's `NV_I2C_SPEED`. - Thread a `speed` parameter through `NvApiI2cBus`; the `i2cSpeedKhz` field now uses it. - Pin all three LCD (`0x61`) construction sites to `Khz400`, matching GCC: `NvApiPanelLocator.Locate`, `NvApiPanelLocator.Survey`, and the `0x61` presence probe in `RgbLocator.AorusGpus`. - Leave the separately hardware-validated RGB path on the existing default speed to avoid regressing it. - Document the bus speed in `docs/PROTOCOL.md`. ## Testing - `dotnet build`: clean, 0 warnings. - `dotnet test`: 44/44 pass. - Not verified on physical hardware from this environment; needs a real-panel soak to confirm the freeze is gone. ## Follow-up The RGB controller shares this bus and still uses the default speed. If RGB shows similar glitches, aligning it to a defined speed (OpenRGB uses 100 kHz) is the next step.
JustinMDotNet
added a commit
that referenced
this pull request
Aug 11, 2026
## What Make it impossible to construct the GPU I2C bus at the wrong speed. This is the structural fix for the audit's top finding (#1), the root cause behind the recurring panel-freeze patches (#17/#20/#21/#22). > Stacked on `fix/bus-lock-hardening` (PR #27) -> `audit/integration`. Base retargets as those merge. ## Problem The GPU has one physical I2C engine shared by the LCD (`0x61`) and RGB (`0x71`/`0x75`) controllers. It only behaves at 400 kHz and wedges silently at the NVAPI default speed. `NvApiI2cBus`'s `speed` parameter defaulted to `NvApiI2cSpeed.Default`, so safety depended on every call site remembering to pass `Khz400`. Each past freeze was a call site that forgot. ## Changes - **Remove the unsafe default (compile-time guard).** `NvApiI2cBus`'s `speed` (and `address`/`port`) parameters are now required, so no caller can silently get the wedging default. The `NvApiI2cSpeed.Default` enum member is kept (it documents the NVAPI value); only its use as a parameter default is gone. Added a read-only `Speed` property for testability; `BuildInfo` and the wire encoding are unchanged. - **Single bus factory.** New `NvApiBusFactory` is the one production place that builds an Aorus bus: `Panel(gpu)` -> `0x61` / port 1 / 400 kHz, `Rgb(gpu, addr)` -> addr / port 1 / 400 kHz. Port and speed are enforced in one spot. - **Route all construction through it.** `NvApiPanelLocator` and `RgbLocator` build via the factory; the locators' probe/detection/ordering/dispose logic is byte-for-byte unchanged. A caller-supplied non-default `port` to `Locate` is still honored (direct construction, still 400 kHz), not ignored. - **Policy tests.** `NvApiBusFactoryTests` assert the invariant (address/port/400 kHz) with no hardware, since the `NvApiI2cBus` constructor is pure. ## Scope note This is the "Core session" step the audit recommended as sufficient for alpha; a fuller single-owner bus service (the other side of the "needs human judgment" design fork) remains a possible future step. This PR closes the accidental-default-speed root cause without an over-broad rewrite. ## Testing - `dotnet build -c Release` clean; `dotnet test -c Release` green (52/52, +3 factory tests). AOT-safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The GPU LCD panel repeatedly freezes its on-screen sensor widgets ("stuck at 25W / 17W with no movement", sometimes going blank). This was misdiagnosed before as an NVML power-read bug; it is not.
Root cause (confirmed two ways)
docs/PROTOCOL.mdandREADME.mdalready state the E3 SensorFeed must be pushed at ~1 Hz or the widgets freeze.AorusLcdService.exe(GCC) shows its feed loop pushes the E3 frame (opcode0xE3= 227) with a fixedThread.Sleep(1000)= exactly 1 Hz, unconditionally, regardless of widget count or the dashboard rotation interval.SensorFeedTiming.PollIntervalMsconflated the E3 push cadence with the widget rotation interval. For a multi-widget dashboard it returnedClamp(rotationIntervalSeconds * 1000, 1000, 5000), so a 2-widget dashboard with a 4s rotation interval pushed E3 only every 4000 ms (0.25 Hz), starving the keep-alive and freezing the widgets. Single-widget setups returned 1000 ms and never froze, which matches the observed behavior (only multi-widget configs freeze).Fix
SensorFeedTiming: removed the adaptivePollIntervalMsoverloads andMinPollMs/MaxPollMs; replaced with a singlepublic const int KeepAlivePollMs = 1000(fixed ~1 Hz keep-alive, decoupled from rotation).FeedWorker.cs: useSensorFeedTiming.KeepAlivePollMs. The rotation interval (config.IntervalSeconds) is still passed separately toSensorFeedLoop.RunAsyncand forwarded to E1SetDisplayfor panel-side rotation, unchanged.docs/PROTOCOL.md: corrected the stale "adaptive (1-5 s)" note to document the fixed ~1 Hz keep-alive and why.SensorFeedTimingTests.cspinning the cadence at 1000 ms and asserting it never exceeds 1 Hz.Why this is the durable fix
The earlier
RetryingI2cBuschange (#17) rides over transient-1write dropouts but did not address the fundamental starvation: at 4s between frames the firmware keep-alive times out and freezes regardless of write success. Matching GCC's fixed 1 Hz push removes the starvation entirely.Testing
dotnet buildclean under warnings-as-errors.dotnet test44/44 pass (2 new).PollIntervalMs/MinPollMs/MaxPollMs.