Skip to content

Clear displayed_etag on etag-less full refresh#85

Open
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/clear-etag-on-etagless-refresh
Open

Clear displayed_etag on etag-less full refresh#85
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/clear-etag-on-etagless-refresh

Conversation

@balloob

@balloob balloob commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Problem (MJ-17)

handleDirectWriteEnd in src/display_service.cpp commits displayed_etag only inside a hasNewEtag && newEtag != 0 guard. An etag-less full upload (or an auto-complete END with no etag payload) changes the panel pixels but leaves displayed_etag at its previous value.

The partial-update path (0x76) validates with:

if (oldEtag == 0 || oldEtag != displayed_etag || newEtag == 0) { /* ETAG mismatch -> full fallback */ }

So a later partial from another client whose oldEtag happens to equal the now-stale displayed_etag passes this check and diffs against the wrong, outdated base image — silent visual corruption.

Fix

On a successful refresh where no valid etag was supplied, clear displayed_etag to 0 (invalid/unknown) instead of leaving a stale value. 0 is never a legitimate displayed_etag match (a partial with oldEtag == 0 is already rejected), so the next partial gets a clean ETAG mismatch and correctly falls back to a full upload. No bogus etag is invented; a later client simply re-uploads in full.

Behavior when a real etag IS supplied is unchanged, as is the partial-refresh commit path (displayed_etag = partialCtx.new_etag on success).

State transitions

  1. Etag-less full upload succeeds → displayed_etag = 0.
  2. Next partial (0x76) arrives with any oldEtagoldEtag != displayed_etag (0) → clean ETAG mismatch NACK.
  3. Client falls back to a full upload (supplying a real etag if it has one) → base image is correct again.

Previously step 1 left the old etag in place, so step 2 could false-match and diff against stale pixels.

Verification

Builds pass:

  • pio run -e nrf52840custom — SUCCESS
  • pio run -e esp32-s3-N16R8 — SUCCESS

🤖 Generated with Claude Code

https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR

An etag-less full upload (or auto-complete) changes the panel pixels but
left displayed_etag at its previous value, since it was only assigned
inside the `hasNewEtag && newEtag != 0` guard. A later partial update
(0x76) from another client whose oldEtag happened to match that stale
value would pass the mismatch check and diff against the wrong, outdated
base image, causing silent visual corruption.

On a successful refresh with no valid etag, clear displayed_etag to 0
(invalid/unknown) so any subsequent partial gets a clean ETAG mismatch
and falls back to a full upload. Behavior when a real etag is supplied is
unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
@balloob balloob requested a review from jonasniesner as a code owner July 5, 2026 21:19
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.

1 participant