fix: Fix exception when sending dyad/zeo requests

[allenporter]

The bug was introduced in #645.

Add tests that exercise the actual request encoding. This changes the ID QUERY value encoding by passing in a function, which is another variation on the first version of #645 where the json encoding happened inside the decode function.

Fixes #623

[Copilot]

Pull request overview

This PR fixes an exception when sending dyad/zeo requests by changing how ID_QUERY values are encoded. Instead of manually converting protocol lists to strings, the fix introduces a value_encoder parameter to encode_mqtt_payload() that allows passing a function (like json.dumps) to handle value encoding consistently.

Key Changes

• Added value_encoder parameter to encode_mqtt_payload() and send_decoded_command() functions to enable flexible value encoding
• Updated DyadApi.query_values() and ZeoApi.query_values() to pass value_encoder=json.dumps for proper list-to-string conversion
• Added comprehensive tests for encoding behavior, including edge cases with lists and complex data structures

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
roborock/protocols/a01_protocol.py	Adds value_encoder parameter to encode_mqtt_payload() with default _no_encode() function
roborock/devices/a01_channel.py	Updates send_decoded_command() overloads to accept and pass through value_encoder parameter
roborock/devices/traits/a01/init.py	Updates query_values() methods to use value_encoder=json.dumps for ID_QUERY encoding; adds explicit identity encoder to ZeoApi.set_value()
tests/protocols/test_a01_protocol.py	Adds unit tests for encode_mqtt_payload() with various data types, though tests have critical issue with missing value_encoder parameter
tests/protocols/common.py	Adds build_a01_message() helper function for creating test response messages
tests/devices/traits/a01/test_init.py	Converts from mocked tests to integration-style tests using FakeChannel to verify actual encoding behavior
tests/test_a01_api.py	Refactors to use build_a01_message() helper and removes duplicate message construction code
tests/devices/test_a01_channel.py	Adds value_encoder parameter to test call to match new function signature

Comments suppressed due to low confidence (1)

tests/protocols/test_a01_protocol.py:105

• The tests test_encode_mqtt_payload_basic, test_encode_mqtt_payload_list_conversion, and test_encode_mqtt_payload_complex_data don't pass the value_encoder parameter to encode_mqtt_payload(), but their assertions expect values to be JSON-encoded as strings.

Without value_encoder=json.dumps, the default _no_encode function is used, which would result in:

• Line 39: {200: {"test": "data", "number": 42}} instead of {200: '{"test": "data", "number": 42}'}
• Line 68: {"dps": {"10000": [101, 102]}} instead of {"dps": {"10000": "[101, 102]"}}
• Lines 94-104: Raw dict/string values instead of JSON-encoded strings

These tests should pass value_encoder=json.dumps to match production usage and verify the fix correctly.

def test_encode_mqtt_payload_basic():
    """Test basic MQTT payload encoding."""
    # Test data with proper protocol keys
    data: dict[RoborockDyadDataProtocol | RoborockZeoProtocol, Any] = {
        RoborockDyadDataProtocol.START: {"test": "data", "number": 42}
    }

    result = encode_mqtt_payload(data)

    # Verify result is a RoborockMessage
    assert isinstance(result, RoborockMessage)
    assert result.protocol == RoborockMessageProtocol.RPC_REQUEST
    assert result.version == b"A01"
    assert result.payload is not None
    assert isinstance(result.payload, bytes)
    assert len(result.payload) % 16 == 0  # Should be padded to AES block size

    # Decode the payload to verify structure
    decoded_data = decode_rpc_response(result)
    assert decoded_data == {200: {"test": "data", "number": 42}}


def test_encode_mqtt_payload_empty_data():
    """Test encoding with empty data."""
    data: dict[RoborockDyadDataProtocol | RoborockZeoProtocol, Any] = {}

    result = encode_mqtt_payload(data)

    assert isinstance(result, RoborockMessage)
    assert result.protocol == RoborockMessageProtocol.RPC_REQUEST
    assert result.payload is not None

    # Decode the payload to verify structure
    decoded_data = decode_rpc_response(result)
    assert decoded_data == {}


def test_value_encoder():
    """Test that value_encoder is applied to all values."""
    data: dict[RoborockDyadDataProtocol | RoborockZeoProtocol, Any] = {RoborockDyadDataProtocol.ID_QUERY: [101, 102]}

    result = encode_mqtt_payload(data, value_encoder=json.dumps)

    # Decode manually to check the raw JSON structure
    decoded_json = json.loads(unpad(result.payload, AES.block_size).decode())

    # ID_QUERY (10000) should be a string "[101, 102]", not a list [101, 102]
    assert decoded_json["dps"]["10000"] == "[101, 102]"
    assert isinstance(decoded_json["dps"]["10000"], str)


def test_encode_mqtt_payload_complex_data():
    """Test encoding with complex nested data."""
    data: dict[RoborockDyadDataProtocol | RoborockZeoProtocol, Any] = {
        RoborockDyadDataProtocol.STATUS: {
            "nested": {"deep": {"value": 123}},
            "list": [1, 2, 3, "test"],
            "boolean": True,
            "null": None,
        },
        RoborockZeoProtocol.MODE: "simple_value",
    }

    result = encode_mqtt_payload(data)

    assert isinstance(result, RoborockMessage)
    assert result.protocol == RoborockMessageProtocol.RPC_REQUEST
    assert result.payload is not None
    assert isinstance(result.payload, bytes)

    # Decode the payload to verify structure
    decoded_data = decode_rpc_response(result)
    assert decoded_data == {
        201: {
            "nested": {"deep": {"value": 123}},
            # Note: The list inside the dictionary is NOT converted because
            # our fix only targets top-level list values in the dps map
            "list": [1, 2, 3, "test"],
            "boolean": True,
            "null": None,
        },
        204: "simple_value",
    }


def test_decode_rpc_response_valid_message():

---

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

[Copilot]

This comment states that lists inside dictionaries are not converted because "our fix only targets top-level list values in the dps map". However, this is misleading. The fix doesn't specifically target lists - it applies the value_encoder function to ALL values in the dps map. When value_encoder=json.dumps, ALL values (not just lists) are JSON-encoded as strings.

The reason lists inside nested dictionaries aren't double-encoded is because json.dumps serializes the entire value (including nested structures) as a single JSON string. Consider updating the comment to:

# Note: The list inside the dictionary is preserved within the JSON string
# because json.dumps serializes the entire dictionary structure, not individual nested elements

Suggested change

	# Note: The list inside the dictionary is NOT converted because
	# our fix only targets top-level list values in the dps map
	# Note: The list inside the dictionary is preserved within the JSON string
	# because json.dumps serializes the entire dictionary structure, not individual nested elements