fix: partial-upload guards — MONO-only, 8-align, NACK fallback, etag commit (M1/M2/M10)#101
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…commit (M1/M2/M10) M1: firmware only allows partial refresh when getBitsPerPixel()==1 (MONO) and requires x/width multiples of 8. Restrict compute_partial_region to MONO and align to 8 pixels (BWRY/BWGBRY/GRAYSCALE_16 previously proceeded and aligned to 4/2, guaranteeing ERR_RECT_ALIGN/UNSUPPORTED). Treat every pre-refresh 0x76 and 0x71 NACK as fallback-to-full instead of raising ProtocolError and losing the upload; the firmware aborts the partial on such a NACK, so a full upload is safe. M2: the etag is only committed on the device via END-with-etag, which is skipped when the firmware auto-completes the upload. Thread an "etag committed" result out of _execute_upload/_dispatch_upload and only populate PartialState.etag when it was committed; otherwise invalidate partial state so the next upload goes full (prevents ERR_ETAG_MISMATCH churn and rendering against a stale old plane). M10: encrypted partial START now caps its inline payload at ENCRYPTED_CHUNK_SIZE (threaded max_start_payload), matching the compressed START, instead of the 200-byte default that could overflow the ~185-byte encrypted envelope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRrm95f1qNZzDM9r2SB6KW
balloob
force-pushed
the
fix/partial-upload-guards
branch
from
d8e5c81 to
63730ee
Compare
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
2 participants
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.
Summary
Three related partial-update correctness fixes.
M1 — partial only where firmware allows it, and fall back instead of aborting (🟠)
Firmware only allows partial refresh when
getBitsPerPixel() == 1(MONO) and requires x/width multiples of 8. Previously BWRY/BWGBRY/GRAYSCALE_16 proceeded and aligned to 4/2 pixels, guaranteeingERR_RECT_ALIGN/ERR_PARTIAL_UNSUPPORTED; and every non-etag NACK raisedProtocolError, losing the upload.compute_partial_regionnow restricts toColorScheme.MONOand aligns to 8 pixels.0x76/0x71NACK is treated as fallback-to-full (firmware aborts the partial on such a NACK, so a full upload is safe).M2 — don't commit an etag the device never stored (🟠)
The etag is only committed via END-with-etag, skipped when the firmware auto-completes. Previously
PartialState.etagwas populated unconditionally → uncompressed-only devices got permanentERR_ETAG_MISMATCHfallback, and a stale etag could render against the wrong old plane._execute_upload/_dispatch_uploadnow report whether the etag was committed; state stores it only then, otherwise invalidates so the next upload goes full.M10 — respect the encrypted payload budget on partial START (🟠)
build_direct_write_partial_startnow takesmax_start_payload, threaded withENCRYPTED_CHUNK_SIZEwhen a session is active (like the compressed START), instead of the 200-byte default that could overflow the ~185-byte encrypted envelope.Test plan
uv run pytest -q→ 443 passed (new + two updated tests)ruff,mypyclean🤖 Generated with Claude Code