Skip to content

Diagnostics JSON tooling can't serialize bytes in quirk entity info (command_args / attribute_value) #845

Description

@zigpy-review-bot

Summary

ZhaJsonEncoder (tests/common.py:641) only special-cases set, so any bytes value reaching Device.get_diagnostics_json() makes the entire diagnostics JSON unserializable. Quirks v2 can put bytes there through two supported builder arguments:

  • .command_button(..., command_args=(b"\x00",))CommandButtonEntityInfo.args (zha/application/platforms/button/__init__.py:44)
  • .write_attr_button(attr_name, b"\x00", ...)WriteAttributeButtonEntityInfo.attribute_value (zha/application/platforms/button/__init__.py:52)

Consequence: a device whose quirk uses either can never be added to tests/data/devices/. python -m tools.import_diagnostics <ha-dump.json> (and tools/regenerate_diagnostics.py, and the json.dumps(..., cls=ZhaJsonEncoder) calls in tests/test_discover.py) die with TypeError, so the device gets zero coverage from test_devices_from_files.

Home Assistant's own diagnostics download is not affected — HA's encoder falls back to {"__type": "<class 'bytes'>", "repr": "b'\\x00'"} — so the dumps users attach to PRs are fine. It's only ZHA's snapshot tooling that can't consume them.

Reproduction

Self-contained test against dev (82a14cd8):

"""Repro: bytes in quirk entity info break diagnostics JSON serialization."""

import json
from typing import Final

from zhaquirks.builder import QuirkBuilder
from zhaquirks.clusters import CustomCluster
from zhaquirks.device import CustomZigpyDevice
from zigpy.profiles import zha
import zigpy.types as t
from zigpy.zcl.clusters import general
from zigpy.zcl.clusters.manufacturer_specific import ManufacturerSpecificCluster
import zigpy.zcl.foundation as zcl_f

from tests.common import (
    SIG_EP_INPUT,
    SIG_EP_OUTPUT,
    SIG_EP_PROFILE,
    SIG_EP_TYPE,
    ZhaJsonEncoder,
    create_mock_zigpy_device,
    join_zigpy_device,
    patch_cluster_for_testing,
)
from zha.application.gateway import Gateway
from zha.quirks import DeviceRegistry


class FakeCluster(CustomCluster, ManufacturerSpecificCluster):
    """Fake manufacturer cluster with an octet-string attribute and command."""

    cluster_id = 0xFC11
    ep_attribute = "fake_cluster"

    class AttributeDefs(zcl_f.BaseAttributeDefs):
        """Attribute definitions."""

        raw_attr: Final = zcl_f.ZCLAttributeDef(id=0x0000, type=t.LVBytes)

    class ServerCommandDefs(zcl_f.BaseCommandDefs):
        """Server command definitions."""

        raw_command: Final = zcl_f.ZCLCommandDef(id=0x00, schema={"payload": t.LVBytes})


async def test_bytes_entity_info_diagnostics(zha_gateway: Gateway) -> None:
    """Bytes in command_args / attribute_value make diagnostics unserializable."""
    registry = DeviceRegistry()
    zigpy_device = create_mock_zigpy_device(
        zha_gateway,
        {
            1: {
                SIG_EP_INPUT: [general.Basic.cluster_id, FakeCluster.cluster_id],
                SIG_EP_OUTPUT: [],
                SIG_EP_TYPE: zha.DeviceType.REMOTE_CONTROL,
                SIG_EP_PROFILE: zha.PROFILE_ID,
            }
        },
        manufacturer="Fake_Manufacturer",
        model="Fake_Model",
    )

    (
        QuirkBuilder("Fake_Manufacturer", "Fake_Model")
        .replaces(FakeCluster)
        .command_button(
            FakeCluster.ServerCommandDefs.raw_command.name,
            FakeCluster.cluster_id,
            command_args=(b"\x00",),
            translation_key="raw_command",
            fallback_name="Raw command",
        )
        .add_to_registry(registry)
    )

    zigpy_device = registry.resolve(zigpy_device)
    assert isinstance(zigpy_device, CustomZigpyDevice)
    patch_cluster_for_testing(zigpy_device.endpoints[1].fake_cluster)

    zha_device = await join_zigpy_device(zha_gateway, zigpy_device)

    # what tools/import_diagnostics.py and tools/regenerate_diagnostics.py do
    json.dumps(zha_device.get_diagnostics_json(), indent=2, cls=ZhaJsonEncoder)
TypeError: Object of type bytes is not JSON serializable
when serializing tuple item 0
when serializing dict item 'args'
when serializing dict item 'info_object'
when serializing list item 0
when serializing collections.defaultdict item <Platform.BUTTON: 'button'>
when serializing dict item 'zha_lib_entities'

Swapping the .command_button(...) above for

        .write_attr_button(
            FakeCluster.AttributeDefs.raw_attr.name,
            b"\x00",
            FakeCluster.cluster_id,
            translation_key="raw_attr",
            fallback_name="Raw attr",
        )

fails the same way, at when serializing dict item 'attribute_value'.

Suggested fix

Teaching ZhaJsonEncoder to emit Home Assistant's representation is enough:

    def default(self, obj):
        """Convert non-JSON types."""
        if isinstance(obj, set):
            return sorted(obj, key=repr)

        if isinstance(obj, bytes):
            return {"__type": str(type(obj)), "repr": repr(obj)}

        return super().default(obj)

Verified locally: the repro above passes with this change, and it is symmetric with tools/import_diagnostics.py's parse_legacy_value (line 51), which already reads exactly that shape back for <class 'bytes'> / LVBytes values. The snapshot round-trip is unaffected, since test_devices_from_files compares re-serialized JSON against the stored file and the representation is deterministic.

Alternatively, a general {"__type": ..., "repr": ...} fallback for any otherwise-unserializable object — what HA's ExtendedJSONEncoder does — would also cover zigpy Struct subclasses and other custom types that a quirk can pass as command arguments. The trade-off is that a blanket fallback would silently absorb future encoding regressions instead of failing loudly.

Context

Surfaced while reviewing zigpy/zha-device-handlers#5201 (new SONOFF TP-WGZBA thermostat quirk, 69 entities), which has:

    .command_button(
        SonoffTPWGZBAPrivateCluster.ServerCommandDefs.temporary_mode.name,
        SonoffTPWGZBAPrivateCluster.cluster_id,
        command_args=(bytes([TEMPORARY_MODE_EXIT]),),
        ...
    )

With that one argument replaced by a non-bytes value, importing the PR's attached HA diagnostics dump succeeds and produces a clean 69-entity snapshot — so this is the only thing standing between that device and CI coverage. (A bytes subclass such as the PR's RawBytes doesn't help; isinstance(obj, bytes) is what matters either way.)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugConfirmed bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions