Send NACK for unsupported client commands (0x76, 0x77)#2
Open
balloob wants to merge 1 commit into
Open
Conversation
Partial-update (0x76) and buzzer (0x77) commands that the client sends
and blocks on hit the dispatch default case, which only logged and
returned nothing. The client waited for an ACK that never arrived and
timed out.
Reply with the firmware's standard 4-byte NACK frame
{0xFF, cmd, error, 0x00} so the client fails fast: the Python client's
parse_nack recognizes it for 0x76 (clean fallback to full upload) and
validate_ack_response rejects it as a non-ACK for 0x77 (prompt error).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
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
1 participant
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 client sends partial-update (0x76) and buzzer (0x77) commands and blocks awaiting a response. On this firmware neither is implemented, so both fall through to the dispatch_command default case, which only logs and returns nothing. The client never receives an ACK and eventually times out, failing slowly with an unhelpful error.
Fix: Handle 0x76 and 0x77 in dispatch_command and reply with the firmware's existing 4-byte NACK frame convention
{0xFF, cmd, error, 0x00}(the same shape already used for config/LED errors). This lets the client fail fast instead of hanging until timeout. Scope is limited to exactly these two opcodes; genuinely-internal/no-response opcodes still hit the unchanged default (silent debug log), so existing flows are untouched.Opcodes + frame:
0x76(DIRECT_WRITE_PARTIAL_START) ->FF 76 01 000x77(BUZZER_ACTIVATE) ->FF 77 01 00Byte 0
0xFF= error, byte 1 = command low byte, byte 2 = error code (RESP_ERR_UNSUPPORTED_CMD = 0x01), byte 3 =0x00. Sent viasend_response, so it is encrypted when a session is active, matching the other command handlers.Client-recognition check (py-opendisplay):
0x76:partial.parse_nackmatches any 4-byte{0xFF, op, err, 0x00}frame, so_maybe_upload_partialtreats START as rejected and cleanly falls back to a full upload instead of timing out.0x77:activate_buzzercallsresponses.validate_ack_response(..., BUZZER_ACTIVATE). The frame's first two bytes decode to0xFF77, which is not in the accepted ACK set{0x0077, 0x8077}, so it raisesInvalidResponseErrorpromptly rather than blocking until the refresh timeout. In neither case is the frame mis-parsed as an ACK.Testing: The nRF ARM toolchain (
arm-none-eabi-gcc) is not available in this environment (build-nrf52/Makefileerrors "ARM GCC toolchain not found"), so a full firmware build was not run. The changed switch/NACK construct was host syntax-checked withgcc -std=c99 -Wall -Wextra -fsyntax-only(clean), and the frame bytes were cross-checked against the Python client parsing described above.Companion: a Silabs firmware PR applies the equivalent fix on that platform.
🤖 Generated with Claude Code
https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
Companion PR: OpenDisplay/Firmware_Silabs#7 (same MJ-18 fix, Silabs side). Note: Silabs uses err code
0x07(ERR_PARTIAL_UNSUPPORTED) vs this PR's0x01— both trigger the client's fallback; consider aligning on0x07.