fix: handle random length bytes before version bytes

[Lash-L]

So our expected behavior is that if it does not start with the verison bytes, it should start with 4 bytes that give the payload length.

That is not always happening - likely due to the buffer, but it's hard to say. This will fix the problem while we can explore more in depth about the buffer

Partly related to home-assistant/core#157599 ?

Talks about the decoding issue there

[Copilot]

Pull request overview

This PR enhances the protocol decoder to handle variable-length garbage bytes before valid version headers, fixing an issue where the decoder only handled fixed 4-byte padding. The implementation now scans forward to find valid version headers (1.0, A01, B01, L01) at any position in the stream.

• Replaces fixed 4-byte skip with dynamic scanning to find valid version headers
• Adds debug logging to track when non-standard padding lengths are encountered
• Implements comprehensive test coverage for clean messages, fixed padding, variable padding, and split-chunk scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
roborock/protocol.py	Updated PrefixedStruct._parse to scan for valid version headers at any offset instead of skipping only 4 bytes
tests/devices/test_local_decoder_padding.py	Added new test file with 4 test cases covering clean messages, 4-byte padding, variable padding, and split padding scenarios

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Calling stream.read() without arguments reads all remaining data from the stream, which exhausts it. After this call, the stream position is at the end. This could be inefficient for large streams and may consume excessive memory if the stream contains a large amount of data. Consider reading in chunks or limiting the read size.

Suggested change

	# Read remaining data to find a valid header
	data = stream.read()
	start_index = -1
	# Find the earliest occurrence of any valid version
	candidates = [idx for v in valid_versions if (idx := data.find(v)) != -1]
	if candidates:
	start_index = min(candidates)
	if start_index != -1:
	# Found a valid version header at `start_index`.
	# Seek to that position (original_pos + index).
	# Read remaining data in chunks to find a valid header
	CHUNK_SIZE = 4096
	buffer = b""
	start_index = -1
	found = False
	while True:
	chunk = stream.read(CHUNK_SIZE)
	if not chunk:
	break
	buffer += chunk
	# Check for any valid version header in the buffer
	candidates = [idx for v in valid_versions if (idx := buffer.find(v)) != -1]
	if candidates:
	start_index = min(candidates)
	found = True
	break
	if found and start_index != -1:

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
roborock/protocol.py	81.75% <100.00%> (+2.41%)	⬆️
tests/devices/test_local_channel.py	95.47% <100.00%> (ø)
tests/devices/test_local_decoder_padding.py	100.00% <100.00%> (ø)
tests/devices/test_mqtt_channel.py	97.10% <100.00%> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[allenporter]

I think theres something you can do like with caplog.at_level(logging.WARNING): as well surrounding this, here and below

[allenporter]

We currently have the pattern that each test module corresponds to an actual model. I know AI likes to make up random test files depending on the subject, but i'd rather see this correspond to a module.

Should this be in tests/test_protocol.py?

[allenporter]

I believe you can write the first three tests as one test like this:

Suggested change

	def test_decoder_clean_message():
	encoder = create_local_encoder(TEST_LOCAL_KEY)
	decoder = create_local_decoder(TEST_LOCAL_KEY)
	msg = RoborockMessage(
	protocol=RoborockMessageProtocol.RPC_REQUEST,
	payload=b"test_payload",
	version=b"1.0",
	seq=1,
	random=123,
	)
	encoded = encoder(msg)
	decoded = decoder(encoded)
	assert len(decoded) == 1
	assert decoded[0].payload == b"test_payload"
	@pytest.mark.parametrize(
	("garbage"),
	[
	b"",
	b"\x00\x00\x05\xa1"
	b"\x00\x00\x05\xa1\xff\xff"
	],
	)
	def test_decoder_clean_message(garbage: bytes):
	encoder = create_local_encoder(TEST_LOCAL_KEY)
	decoder = create_local_decoder(TEST_LOCAL_KEY)
	msg = RoborockMessage(
	protocol=RoborockMessageProtocol.RPC_REQUEST,
	payload=b"test_payload",
	version=b"1.0",
	seq=1,
	random=123,
	)
	encoded = encoder(msg)
	decoded = decoder(garbage + encoded)
	assert len(decoded) == 1
	assert decoded[0].payload == b"test_payload"

[allenporter]

Please make the requested changes in a followup PR. I'd like to get this fix in.