Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/opendisplay/protocol/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,16 @@ def build_write_config_command(config_data: bytes) -> tuple[bytes, list[bytes]]:
Protocol:
- Single chunk (≤200 bytes): [0x00][0x41][config_data]
- Multi-chunk (>200 bytes):
- First: [0x00][0x41][total_size:2LE][first_198_bytes]
- First: [0x00][0x41][total_size:2LE][first_200_bytes]
- Rest: [0x00][0x42][chunk_data] (up to 200 bytes each)

The first chunk carries a full 200 data bytes (payload = size(2) + 200 = 202
bytes). Firmware only enters chunked mode when the payload length exceeds 200
and expects exactly [total:2LE][200 data]; a 198-byte first chunk (200-byte
payload) makes it take the single-chunk path and store the size header plus a
truncated config, then NACK every following 0x42 chunk, and also breaks its
``expectedChunks = ceil(total / 200)`` accounting.

Args:
config_data: Complete serialized config data

Expand All @@ -320,7 +327,7 @@ def build_write_config_command(config_data: bytes) -> tuple[bytes, list[bytes]]:

# Large config (>200 bytes)
first_cmd, chunks = build_write_config_command(large_config)
# first_cmd: [0x00][0x41][total_size:2LE][first_198_bytes]
# first_cmd: [0x00][0x41][total_size:2LE][first_200_bytes]
# chunks: [[0x00][0x42][chunk_data], ...]
"""
cmd_write = CommandCode.WRITE_CONFIG.to_bytes(2, byteorder="big")
Expand All @@ -333,9 +340,9 @@ def build_write_config_command(config_data: bytes) -> tuple[bytes, list[bytes]]:
return cmd_write + config_data, []

# Multi-chunk mode (>200 bytes)
# First chunk: [cmd][total_size:2LE][first_198_bytes]
# First chunk: [cmd][total_size:2LE][first_200_bytes]
total_size = config_len.to_bytes(2, byteorder="little")
first_chunk_data_size = CONFIG_CHUNK_SIZE - 2 # 198 bytes
first_chunk_data_size = CONFIG_CHUNK_SIZE # 200 bytes
first_chunk = cmd_write + total_size + config_data[:first_chunk_data_size]

# Remaining chunks: [cmd][chunk_data] (up to 200 bytes each)
Expand Down
41 changes: 41 additions & 0 deletions tests/unit/test_protocol_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from opendisplay.models.led_flash import LedFlashConfig, LedFlashStep
from opendisplay.protocol.commands import (
CHUNK_SIZE,
CONFIG_CHUNK_SIZE,
CommandCode,
build_buzzer_activate_command,
build_direct_write_data_command,
Expand All @@ -14,6 +15,7 @@
build_read_config_command,
build_read_fw_version_command,
build_reboot_command,
build_write_config_command,
)


Expand Down Expand Up @@ -214,3 +216,42 @@ def test_invalid_instance_raises(self):
config = BuzzerActivateConfig.single_tone(frequency_hz=1000, duration_ms=100)
with pytest.raises(ValueError, match="out of range"):
build_buzzer_activate_command(256, config)


class TestWriteConfigChunking:
"""WRITE_CONFIG chunking must match the firmware's expectations (C4)."""

def test_single_chunk_when_within_limit(self):
data = b"\xab" * CONFIG_CHUNK_SIZE # exactly 200 bytes -> single chunk
first, chunks = build_write_config_command(data)
assert chunks == []
assert first == CommandCode.WRITE_CONFIG.to_bytes(2, "big") + data

def test_first_chunk_carries_full_200_data_bytes(self):
# >200 bytes triggers chunked mode; the first chunk must carry a full
# 200 data bytes (payload = size(2) + 200 = 202 > 200) so the firmware
# enters chunked mode instead of the single-chunk path.
total = CONFIG_CHUNK_SIZE + 50 # 250 bytes
data = bytes(range(256))[:total]
first, chunks = build_write_config_command(data)

cmd_write = CommandCode.WRITE_CONFIG.to_bytes(2, "big")
# first = cmd(2) + total_size(2 LE) + 200 data
assert first[:2] == cmd_write
assert first[2:4] == total.to_bytes(2, "little")
assert first[4:] == data[:CONFIG_CHUNK_SIZE]
assert len(first[4:]) == CONFIG_CHUNK_SIZE

# remaining 50 bytes in a single 0x42 continuation chunk
cmd_chunk = CommandCode.WRITE_CONFIG_CHUNK.to_bytes(2, "big")
assert len(chunks) == 1
assert chunks[0] == cmd_chunk + data[CONFIG_CHUNK_SIZE:]

def test_chunk_count_matches_ceil_total_over_200(self):
import math

for total in (201, 400, 401, 605):
data = bytes((i % 256 for i in range(total)))
first, chunks = build_write_config_command(data)
# first chunk (200) + continuations (200 each) == ceil(total/200) chunks
assert 1 + len(chunks) == math.ceil(total / CONFIG_CHUNK_SIZE)