From 9f4c058326932a1dd8b136ce466f3e36f0e41b42 Mon Sep 17 00:00:00 2001 From: Rob Hofmann Date: Wed, 23 Sep 2026 22:33:21 +0200 Subject: [PATCH 1/3] VRF gateways: ask all sub-device list forms, hold sent values until confirmed Port of the VRF fixes that landed on master in #507 and #508. #454 needs no port: 5.0 already gives each indoor unit its own device. Sub-device list: a gateway answers one or more of three request forms, depending on its WiFi module firmware (device key, generic key, subDev). The old single hybrid form was never confirmed on hardware. V1 gateways now get all three forms and the lists are joined by MAC; V2 gets the device key form. The generic key form is encrypted with the device key but answered with the generic key, so request_json() takes an optional response_cipher. Gateways from a scan are handled side by side. On the GR-Gcloud V3.2.M gateway from #507 the forms gave 4, 3 and 4 units, joined 4. Stale state: a gateway answers from a cache for a few seconds after a command, so the read right after it showed the old value again. The device state now holds sent values until the device reports them, for at most 8 s. The coordinator polls again every 2 s while values are held. A standalone unit confirms on the first read and gets no extra poll. Suite: 247 passed (227 before). --- custom_components/gree_custom/aiogree/api.py | 292 ++++++++++++------ .../gree_custom/aiogree/const.py | 7 + .../gree_custom/aiogree/device.py | 14 +- .../gree_custom/aiogree/device_state.py | 128 +++++++- .../gree_custom/aiogree/transport.py | 11 +- custom_components/gree_custom/coordinator.py | 55 +++- docs/architecture.md | 12 +- docs/development.md | 12 +- docs/protocol.md | 35 +++ docs/troubleshooting.md | 4 + tests/fakes/device.py | 41 ++- tests/fakes/vrf.py | 97 ++++-- tests/test_device.py | 56 ++++ tests/test_device_state.py | 159 ++++++++++ tests/test_discovery_vrf.py | 187 ++++++++++- 15 files changed, 970 insertions(+), 140 deletions(-) diff --git a/custom_components/gree_custom/aiogree/api.py b/custom_components/gree_custom/aiogree/api.py index ad0dfb6..6532a75 100755 --- a/custom_components/gree_custom/aiogree/api.py +++ b/custom_components/gree_custom/aiogree/api.py @@ -1,5 +1,6 @@ """Contains the API to interface with the Gree device.""" +import asyncio from collections.abc import Mapping from dataclasses import dataclass, fields, replace from enum import IntEnum, StrEnum, unique @@ -479,6 +480,19 @@ class GreeCommand(StrEnum): SCAN = "scan" +@unique +class SubListForm(StrEnum): + """The request forms a VRF gateway may answer with its sub-device list. + + Different WiFi module firmwares answer different forms, so all of them are + asked. See docs/protocol.md for the details of each form. + """ + + DEVICE_KEY = "device-key" + GENERIC_KEY = "generic-key" + SUB_DEV = "subDev" + + class DeviceScanInfoResponse(BaseModel): """Response data for a Gree device returned in a UDP scan.""" @@ -561,6 +575,7 @@ async def gree_get_response( transport: GreeBaseTransport, max_attempts: int | None = None, timeout: float | None = None, + response_cipher: CipherBase | None = None, ) -> dict: """Send a request to the device and return the decoded response. @@ -569,6 +584,10 @@ async def gree_get_response( json_data: JSON payload to send cipher: Device cipher to encrypt and decrypt the JSON pack, if present transport: Transport to send the emssage throuhg + max_attempts: Attempts for this request instead of the transport's own + timeout: Reply timeout for this request instead of the transport's own + response_cipher: Cipher to decrypt the reply with, if it differs from + the request cipher Returns: Decrypted JSON response @@ -577,7 +596,12 @@ async def gree_get_response( try: data = await transport.request_json( - mac_controller, json_data, cipher, max_attempts, timeout + mac_controller, + json_data, + cipher, + max_attempts, + timeout, + response_cipher=response_cipher, ) except GreeConnectionError: raise @@ -659,21 +683,42 @@ def _create_bind_pack(mac_addr_controller: str, uid: int, cipher: CipherBase) -> return pack -def _create_get_subdevices_pack(mac_addr_controller: str) -> dict: - """Create a sub-device list request pack. +def _create_get_subdevices_payload( + form: SubListForm, mac_addr_controller: str, uid: int +) -> dict: + """Create the request for one form of the sub-device list query. + + Every form encrypts its pack with the bound device key. Only the reply + differs, see `_get_sub_devices_list`. Args: + form: Which form of the query to build mac_addr_controller: The MAC address of the device that controls the sub devices + uid: User ID for the device Returns: - The created get sub-devices pack + The full payload, with the pack still in clear text """ - pack: dict = {"mac": mac_addr_controller, "i": 1} + if form is SubListForm.DEVICE_KEY: + pack: dict = {"mac": mac_addr_controller, "t": "subList", "i": 0} + payload_type, i = "pack", 0 + elif form is SubListForm.GENERIC_KEY: + pack = {"mac": mac_addr_controller, "i": 1} + payload_type, i = "subList", 1 + else: + # Older W06 class modules do not answer subList at all, only subDev. + pack = { + "cid": mac_addr_controller, + "i": 0, + "mac": mac_addr_controller, + "t": "subDev", + } + payload_type, i = "pack", 0 - _LOGGER.debug("Sub Bind Pack: %s", pack) - return pack + _LOGGER.debug("Sub-device list pack (%s): %s", form, pack) + return _create_payload(pack, payload_type, i, mac_addr_controller, uid) def _create_get_status_pack(mac_addr: str, props: list[str]) -> dict: @@ -1213,6 +1258,40 @@ def extract_fw_version(hid: str) -> tuple[str | None, str | None]: return fw_version, fw_code +def _parse_sub_devices_list( + mac_addr_controller: str, form: SubListForm, response: dict[str, Any] +) -> list[dict[str, Any]] | None: + """Take the sub-device list out of one reply. + + The list may be at the top level (some firmwares) or inside a pack. + Reply in format: + {"t":"subList","i":0,"c":6,"r":200,"list":[{"mac":"09c4a41d000000","mid":"6049"},...]} + + Returns: + The list, or None if the reply holds no list at all + + """ + + if isinstance(response.get("list"), list): + sub_devs: list[dict[str, Any]] = response["list"] + where = "top-level" + else: + pack = response.get("pack") + if not isinstance(pack, dict) or not isinstance(pack.get("list"), list): + return None + sub_devs = pack["list"] + where = "pack" + + _LOGGER.debug( + "[%s] Found %d sub-units (%s, %s)", + mac_addr_controller, + len(sub_devs), + form, + where, + ) + return sub_devs + + async def _get_sub_devices_list( mac_addr_controller: str, uid: int, @@ -1223,14 +1302,23 @@ async def _get_sub_devices_list( ) -> list[GreeDiscoveredDevice]: """Retrieve the list of sub-devices exposed by a main controller device. + Different WiFi module firmwares answer different forms of the query, and + some gateways return a different subset of units in each form. So with V1 + all three forms are asked and the lists are joined by MAC, in the order + device key, generic key, subDev. With V2 only the device key form is known + to work. A form that gets no answer, or an answer that cannot be read, is + skipped. Each form uses the retries and timeout of the transport. + Args: mac_addr_controller: The MAC address of the device that controls the connection uid: User ID for the device - cipher: Device cipher to encrypt and decrypt the JSON pack, if present + cipher: Bound device cipher of the gateway transport: Transport used to communicate with the device + parent_device: The gateway as it was discovered. Sub-devices copy its fields + expected: The number of sub-devices the gateway promised in its scan reply Returns: - List of sub-devices directly from the controller response + List of sub-devices, empty if no form was answered """ @@ -1238,93 +1326,102 @@ async def _get_sub_devices_list( "Retrieving subdevices for '%s' using '%s'", mac_addr_controller, transport ) - discovered_subdevices: list[GreeDiscoveredDevice] = [] - try: - pack = _create_get_subdevices_pack(mac_addr_controller) + forms: list[SubListForm] = [SubListForm.DEVICE_KEY] + if cipher.version == EncryptionVersion.V1: + forms += [SubListForm.GENERIC_KEY, SubListForm.SUB_DEV] - json_payload = _create_payload( - pack, - "subList", - 0, - mac_addr_controller, - uid, - ) + merged: dict[str, dict[str, Any]] = {} + counts: dict[SubListForm, int | None] = {} - response = await gree_get_response( - mac_addr_controller, - json_payload, - cipher, - transport, + for form in forms: + # The generic key form is encrypted with the device key, but the + # gateway answers it with the generic key, like a scan or a bind. + response_cipher = ( + get_cipher(cipher.version) if form is SubListForm.GENERIC_KEY else None ) - except GreeConnectionError: - raise - - except Exception as err: - raise GreeProtocolError( - f"Error fetching sub-device list for '{mac_addr_controller}'" - ) from err - - else: - # The list may be at the top level (some firmwares) or inside a pack. - # Response pack in format: - # {"t":"subList","i":0,"c":6,"r":200,"list":[{"mac":"09c4a41d000000","mid":"6049"},...]} - - sub_devs: list[dict[str, Any]] = [] - - if isinstance(response.get("list"), list): - sub_devs = response.get("list", []) - _LOGGER.debug( - "[%s] Found %d sub-units (top-level)", + try: + response = await gree_get_response( mac_addr_controller, - len(sub_devs), + _create_get_subdevices_payload(form, mac_addr_controller, uid), + cipher, + transport, + response_cipher=response_cipher, ) - else: - sub_devs = response.get("pack", {}).get("list", []) + except GreeConnectionError, GreeProtocolError: _LOGGER.debug( - "[%s] Found %d sub-units (pack)", + "[%s] No usable answer to the %s sub-device list form", mac_addr_controller, - len(sub_devs), + form, ) + counts[form] = None + continue - if expected and len(sub_devs) != expected: - _LOGGER.warning( - "[%s] Expected %d sub-devices and found %d", - mac_addr_controller, - expected, - len(sub_devs), - ) + sub_devs = _parse_sub_devices_list(mac_addr_controller, form, response) + counts[form] = None if sub_devs is None else len(sub_devs) - for sub_dev in sub_devs: - new_dev: GreeDiscoveredDevice - if parent_device: - # TODO: get real-data from VRF discovery to check the result list fields - new_dev = replace( - parent_device, - name=sub_dev.get( - "name", - f"{sub_dev.get('mac', '')[-5:]} VRF at {parent_device.name}", - ), - mac=sub_dev.get("mac", ""), - mid=sub_dev.get("mid", ""), - key=cipher.key, - ) - else: - new_dev = GreeDiscoveredDevice( - name=sub_dev.get( - "name", - f"{sub_dev.get('mac', '')[-5:]} VRF at {mac_addr_controller[-5:]}", - ), - mac=sub_dev.get("mac", ""), - mac_controller_local=mac_addr_controller, - host=transport.ip_addr, - port=transport.port, - mid=sub_dev.get("mid", ""), - key=cipher.key, - ) - discovered_subdevices.append(new_dev) + for sub_dev in sub_devs or []: + sub_mac = sub_dev.get("mac", "") + # A unit without a MAC cannot be addressed, so it is left out. + if sub_mac and sub_mac not in merged: + merged[sub_mac] = sub_dev + + per_form = ", ".join( + f"{form}={'n/a' if count is None else count}" for form, count in counts.items() + ) + _LOGGER.debug( + "[%s] Sub-device list per form: %s, merged=%d", + mac_addr_controller, + per_form, + len(merged), + ) + + if all(count is None for count in counts.values()): + _LOGGER.warning( + "[%s] VRF gateway did not answer any form of the sub-device list request. Its sub-devices will be ignored", + mac_addr_controller, + ) + return [] + + if expected and len(merged) != expected: + _LOGGER.warning( + "[%s] Expected %d sub-devices and found %d", + mac_addr_controller, + expected, + len(merged), + ) + + discovered_subdevices: list[GreeDiscoveredDevice] = [] + for sub_mac, sub_dev in merged.items(): + new_dev: GreeDiscoveredDevice + if parent_device: + # TODO: get real-data from VRF discovery to check the result list fields + new_dev = replace( + parent_device, + name=sub_dev.get( + "name", + f"{sub_mac[-5:]} VRF at {parent_device.name}", + ), + mac=sub_mac, + mid=sub_dev.get("mid", ""), + key=cipher.key, + ) + else: + new_dev = GreeDiscoveredDevice( + name=sub_dev.get( + "name", + f"{sub_mac[-5:]} VRF at {mac_addr_controller[-5:]}", + ), + mac=sub_mac, + mac_controller_local=mac_addr_controller, + host=transport.ip_addr, + port=transport.port, + mid=sub_dev.get("mid", ""), + key=cipher.key, + ) + discovered_subdevices.append(new_dev) - return discovered_subdevices + return discovered_subdevices async def _process_local_scan_response( @@ -1362,7 +1459,6 @@ async def _process_local_scan_response( # If we are dealing wiht a VRF gateway, procceed by scaning its subdevices # The gateway itselft is not a valid dicovered device - # TODO: Ingest subdevices, need real-world tests. transport = GreeUdpTransport(ip_address, DEFAULT_DEVICE_PORT, max_retries, timeout) try: _LOGGER.debug("Obtaining device binding encryption info for the VRF controller") @@ -1474,15 +1570,31 @@ async def gree_discover_devices_local( get_cipher(EncryptionVersion.V1), ) + scan_packs: list[tuple[str, dict]] = [] for address, response in responses.items(): if response is not None: pack = response.get("pack") if pack is not None and pack.get("t") == "dev": - discovered_devices.extend( - await _process_local_scan_response( - address, pack, timeout, max_retries, user_id - ) - ) + scan_packs.append((address, pack)) + + # A VRF gateway needs a bind and a sub-device list, which can take several + # seconds per gateway. Run them side by side, in scan order. Each gateway + # has its own transport, so the replies cannot cross. + results = await asyncio.gather( + *( + _process_local_scan_response(address, pack, timeout, max_retries, user_id) + for address, pack in scan_packs + ), + return_exceptions=True, + ) + + for result in results: + # An unexpected error still breaks discovery, as it did when the + # devices were handled one after another. Waiting for all of them + # first means no request is left running in the background. + if isinstance(result, BaseException): + raise result + discovered_devices.extend(result) _LOGGER.info("Found total of %d local devices", len(discovered_devices)) return discovered_devices diff --git a/custom_components/gree_custom/aiogree/const.py b/custom_components/gree_custom/aiogree/const.py index fd8e0a7..2759e33 100755 --- a/custom_components/gree_custom/aiogree/const.py +++ b/custom_components/gree_custom/aiogree/const.py @@ -24,6 +24,13 @@ # row are normal. This many in a row means the device stopped talking. MAX_UNANSWERED_IN_A_ROW = 5 +# After a command, the sent values are shown for at most this many seconds +# while the device still reports the old ones. A VRF gateway caches the state of +# its indoor units and can answer with the old state for a few seconds after a +# command. After this time the reported value wins, so a command the device +# rejected is not shown for ever. +HELD_VALUE_TTL = 8.0 + MIN_TEMP_C = 16 MAX_TEMP_C = 30 diff --git a/custom_components/gree_custom/aiogree/device.py b/custom_components/gree_custom/aiogree/device.py index 225199e..93ea087 100755 --- a/custom_components/gree_custom/aiogree/device.py +++ b/custom_components/gree_custom/aiogree/device.py @@ -314,11 +314,13 @@ async def push_device_status(self) -> None: self._state.set(GreeProp.BEEPER_NEW, 1 if self._beeper else 0) try: - await self._client.set_props( - {k.value: v for k, v in self._state.pending.items()} - ) + sent = dict(self._state.pending) + await self._client.set_props({k.value: v for k, v in sent.items()}) _LOGGER.debug("[%s:%s] Device status set", self.unique_id, self.transport) + # Keep showing what was sent until the device reports it. A VRF + # gateway answers the read below with its old cached state. + self._state.hold(sent) self._state.clear_pending() await self.fetch_device_status() @@ -405,6 +407,7 @@ def gather_diagnostics(self) -> dict[str, Any]: data["state_info"] = dict(self._state.info) data["state"] = {str(k): v for k, v in self._state.raw.items()} data["state_pending"] = {str(k): v for k, v in self._state.pending.items()} + data["state_held"] = {str(k): v for k, v in self._state.held.items()} data["state_unknown"] = {str(k): v for k, v in self._state.unknown.items()} return data @@ -558,6 +561,11 @@ def available(self) -> bool: """Return True if the device is bound and last connection was successful.""" return self._client.bound and self._client.available + @property + def has_held_values(self) -> bool: + """Return True if sent values still wait for the device to confirm them.""" + return bool(self._state.held) + @property def is_bound(self) -> bool: """Return True if the device is bound.""" diff --git a/custom_components/gree_custom/aiogree/device_state.py b/custom_components/gree_custom/aiogree/device_state.py index 7ecd7b6..07288e0 100755 --- a/custom_components/gree_custom/aiogree/device_state.py +++ b/custom_components/gree_custom/aiogree/device_state.py @@ -1,7 +1,8 @@ """Contains the ``DeviceState`` class that holds and manages the device state.""" -from collections.abc import Iterable +from collections.abc import Callable, Iterable, Mapping import logging +import time from types import MappingProxyType from .api import ( @@ -11,6 +12,7 @@ GreeProp, InfoProp, ) +from .const import HELD_VALUE_TTL _LOGGER = logging.getLogger(__name__) @@ -18,12 +20,29 @@ class DeviceState: """Represents the local state of a Gree device.""" - def __init__(self, device_id: str, capabilities: Iterable[GreeProp]) -> None: - """Initialize the device state.""" + def __init__( + self, + device_id: str, + capabilities: Iterable[GreeProp], + clock: Callable[[], float] = time.monotonic, + ) -> None: + """Initialize the device state. + + Args: + device_id: Name of the device in log lines + capabilities: The props this device may be told to change + clock: Source of monotonic seconds for the held values. Tests pass + their own so they do not have to wait. + + """ self._device_id: str = device_id + self._clock = clock self._raw: dict[GreeProp, int] = {} self._pending: dict[GreeProp, int] = {} + # Values that were sent and are not confirmed yet, with the monotonic + # time at which the hold ends. + self._held: dict[GreeProp, tuple[int, float]] = {} self._info: dict[InfoProp, str] = {} self._unknown: dict[str, str] = {} @@ -39,9 +58,9 @@ def __init__(self, device_id: str, capabilities: Iterable[GreeProp]) -> None: def get(self, prop: GreeProp, default: int | None = None) -> int | None: """Get the raw value of a property. - Returns the pending value from ``pending`` if present, otherwise the - last known value from ``raw``. If the property does not exist in - either state, returns ``default``. + Returns the pending value from ``pending`` if present, then the held + value from ``held``, otherwise the last known value from ``raw``. If + the property does not exist in any of them, returns ``default``. """ # Query first the transient state, so we can make changes to the device state @@ -49,6 +68,10 @@ def get(self, prop: GreeProp, default: int | None = None) -> int | None: if prop in self._pending: return self._pending[prop] + held = self._held_value(prop) + if held is not None: + return held + if prop in self._raw: return self._raw[prop] @@ -88,6 +111,76 @@ def clear_pending(self) -> None: """Clear the pending state.""" self._pending.clear() + # + # Held values + # + + def hold(self, values: Mapping[GreeProp, int]) -> None: + """Keep showing values that were just sent, until the device confirms them. + + A VRF gateway can answer with its old cached state for a few seconds + after a command. While a prop is held, `get()` returns the sent value + instead of the reported one. The hold ends when the device reports the + sent value, or after `HELD_VALUE_TTL` seconds, after which the reported + value wins again. That way a command the device rejected is not shown + for ever. + + Props that are not polled, like the beeper, are never reported, so + they are not held. + """ + expires = self._clock() + HELD_VALUE_TTL + for prop, value in values.items(): + if prop in self._props_to_poll: + self._held[prop] = (value, expires) + + def _held_value(self, prop: GreeProp) -> int | None: + """Return the held value of a prop, or None when it is not held.""" + entry = self._held.get(prop) + if entry is None: + return None + + value, expires = entry + if self._clock() >= expires: + del self._held[prop] + _LOGGER.debug( + "[%s] Device did not confirm %s=%d in time, using the reported value", + self._device_id, + prop, + value, + ) + return None + + return value + + def _drop_expired_holds(self) -> None: + """Drop every hold whose time is up.""" + for prop in list(self._held): + self._held_value(prop) + + def _confirm_hold(self, prop: GreeProp, reported: int) -> None: + """End the hold on a prop if the device reported the sent value. + + This compares with what the device really sent, never with the held + value that `get()` shows. + """ + entry = self._held.get(prop) + if entry is None: + return + + if reported == entry[0]: + del self._held[prop] + _LOGGER.debug( + "[%s] Device confirmed %s=%d", self._device_id, prop, reported + ) + else: + _LOGGER.debug( + "[%s] Device still reports %s=%d, holding the sent value %d", + self._device_id, + prop, + reported, + entry[0], + ) + # # Raw protocol processing # @@ -97,6 +190,8 @@ def process_new_state(self, new_state: dict[str, str]) -> None: unknown = [] errors = [] + self._drop_expired_holds() + for key, value in new_state.items(): try: if key in PROP_KEY_TO_ENUM: @@ -104,6 +199,7 @@ def process_new_state(self, new_state: dict[str, str]) -> None: if prop in self._props_to_poll: self._raw[prop] = int(value) + self._confirm_hold(prop, self._raw[prop]) elif key in INFOPROP_KEY_TO_ENUM: self._info[INFOPROP_KEY_TO_ENUM[key]] = value @@ -143,6 +239,7 @@ def remove(self, prop: GreeProp) -> None: self._props_to_poll = tuple(p for p in self._props_to_poll if p != prop) self._raw.pop(prop, None) self._pending.pop(prop, None) + self._held.pop(prop, None) _LOGGER.debug( "[%s] No longer updating property: %s", self._device_id, repr(prop) ) @@ -181,8 +278,17 @@ def invalidate_missing_property_group( @property def has_pending_updates(self) -> bool: - """Does the state have pending values to be committed.""" - return any(self._raw.get(k) != v for k, v in self._pending.items()) + """Does the state have pending values to be committed. + + A pending value is compared with the held value first, because that + is what the device was last told. Comparing with a stale reported + value would skip a change back to that value. + """ + return any( + (held if (held := self._held_value(k)) is not None else self._raw.get(k)) + != v + for k, v in self._pending.items() + ) # # Read-only views @@ -202,6 +308,12 @@ def pending(self) -> MappingProxyType[GreeProp, int]: """The pending uncommitted device state values.""" return MappingProxyType(self._pending) + @property + def held(self) -> MappingProxyType[GreeProp, int]: + """The values that were sent and are not confirmed by the device yet.""" + self._drop_expired_holds() + return MappingProxyType({k: v for k, (v, _) in self._held.items()}) + @property def info(self) -> MappingProxyType[InfoProp, str]: """The Device Info property values.""" diff --git a/custom_components/gree_custom/aiogree/transport.py b/custom_components/gree_custom/aiogree/transport.py index c4db3d9..5dc557c 100755 --- a/custom_components/gree_custom/aiogree/transport.py +++ b/custom_components/gree_custom/aiogree/transport.py @@ -79,8 +79,15 @@ async def request_json( cipher: CipherBase, max_attempts: int | None = None, timeout: float | None = None, + response_cipher: CipherBase | None = None, ) -> dict[str, Any]: - """Send and receive a JSON payload.""" + """Send and receive a JSON payload. + + The request pack is encrypted with cipher. The reply pack is decrypted + with response_cipher when it is given, and with cipher otherwise. Only + one form of the VRF sub-device list query needs this: its request uses + the device key and its reply uses the generic key. + """ requests: list[dict[str, Any]] @@ -115,7 +122,7 @@ async def request_json( ) response = json.loads(raw_response) - response = gree_decrypt_pack(response, cipher) + response = gree_decrypt_pack(response, response_cipher or cipher) responses.append(response) diff --git a/custom_components/gree_custom/coordinator.py b/custom_components/gree_custom/coordinator.py index b71f320..524b781 100755 --- a/custom_components/gree_custom/coordinator.py +++ b/custom_components/gree_custom/coordinator.py @@ -1,12 +1,13 @@ """Data update coordinator for Gree integration.""" -from datetime import timedelta +from datetime import datetime, timedelta import logging from typing import Any, override from homeassistant.config_entries import ConfigEntry -from homeassistant.core import HomeAssistant +from homeassistant.core import CALLBACK_TYPE, HomeAssistant from homeassistant.exceptions import ConfigEntryAuthFailed +from homeassistant.helpers.event import async_call_later from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from .aiogree.api import OperationMode @@ -16,6 +17,11 @@ _LOGGER = logging.getLogger(__name__) +# Seconds between a command and one extra poll, when the device did not confirm +# the command on the read right after it. A VRF gateway needs a moment to pass +# a command on to the indoor unit and to update its cached state. +FOLLOW_UP_REFRESH_DELAY = 2.0 + # Home Assistant config entry where the runtime data are Gree coordinators keyed by normalized MAC addresses ("xxxxxxxxxxxx"). type GreeConfigEntry = ConfigEntry[dict[str, GreeCoordinator]] @@ -51,6 +57,7 @@ def __init__( self.device: GreeDevice = device self._feature_auto_xfan: bool = False self._feature_auto_light: bool = False + self._unsub_follow_up_refresh: CALLBACK_TYPE | None = None async def _setup(self) -> None: """Bind to the device before the first coordinator refresh. @@ -70,6 +77,7 @@ def _device_pushed_status(self, status: dict[str, str]) -> None: @override async def async_shutdown(self) -> None: """Clean up the coordinator and Gree device resources.""" + self._cancel_follow_up_refresh() self.device.api_client.remove_status_listener(self._device_pushed_status) await self.device.unbind_device() @@ -123,6 +131,49 @@ async def push_device_status(self) -> None: # retry once after recovering IP await self.device.push_device_status() + self._schedule_follow_up_refresh() + + def _schedule_follow_up_refresh(self) -> None: + """Poll again shortly after a command the device did not confirm yet. + + The read right after a command can still show the old state on a VRF + gateway. The sent values are held meanwhile, see `DeviceState.hold()`. + This extra poll lets the device confirm them within a few seconds + instead of at the next scan interval. A standalone unit confirms on + the first read, so it never gets this extra poll. + + `async_refresh()` is used instead of `async_request_refresh()`, + because the request debouncer would push a second request within its + cooldown back to 10 seconds. + """ + if self._unsub_follow_up_refresh is not None: + return + + if not self.device.has_held_values: + return + + self._unsub_follow_up_refresh = async_call_later( + self.hass, FOLLOW_UP_REFRESH_DELAY, self._async_follow_up_refresh + ) + + async def _async_follow_up_refresh(self, _now: datetime) -> None: + """Run the follow-up poll, and plan the next one while values are held. + + The polls repeat until the device confirms the sent values or their + hold ends. The poll after the hold ends shows what the device really + reports, so a rejected command is not shown until the next scan + interval. + """ + self._unsub_follow_up_refresh = None + await self.async_refresh() + self._schedule_follow_up_refresh() + + def _cancel_follow_up_refresh(self) -> None: + """Cancel a follow-up poll that has not run yet.""" + if self._unsub_follow_up_refresh is not None: + self._unsub_follow_up_refresh() + self._unsub_follow_up_refresh = None + def get_coordinator_diagnostics(self) -> dict[str, Any]: """Return diagnostic information for the coordinator. diff --git a/docs/architecture.md b/docs/architecture.md index 4b645c6..d052371 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -26,7 +26,7 @@ No Home Assistant imports here. | File | What it does | |---|---| | `__init__.py` | Entry setup. Builds transports and devices, binds them, starts one `GreeCoordinator` per device. | -| `coordinator.py` | `GreeCoordinator`. Polls on `scan_interval` and listens for status pushed by the device. | +| `coordinator.py` | `GreeCoordinator`. Polls on `scan_interval` and listens for status pushed by the device. Polls again every 2 s after a command while the device has not confirmed it, until the hold ends. | | `config_flow.py` | Setup, reconfigure, reauth and YAML import flows. Local discovery, cloud login, device picker, per device options. `async_step_import` turns one validated YAML item into a config entry. | | `config_schema.py` | `CONFIG_SCHEMA` for the `gree_custom:` block in `configuration.yaml`. Validates the YAML, normalizes the MAC addresses and fills the defaults. It also holds the form schemas that the config flow shows. | | `migration.py` | Moves a 4.x setup (domain `gree`) to this integration: config entries, the `gree:` YAML block, and the device and entity registry rows. See [config-entry.md](config-entry.md#migration-from-4x). | @@ -48,7 +48,10 @@ Discovery runs before any device object exists, and it has its own path. - `gree_discover_device_local()` scans one host. - A host that answers with `subCnt` above zero is a VRF gateway. It is bound, asked for its sub-device list, and then left out of the result itself. Only - the units behind it are returned. + the units behind it are returned. The list request has three forms, see + [protocol.md](protocol.md#vrf-gateways). +- `gree_discover_devices_local()` handles the scan replies side by side, so + one slow gateway does not hold up the others. Both functions build their own `GreeUdpTransport` and neither closes it. The socket opens on the first request and is closed again when the function returns, @@ -75,8 +78,9 @@ Entities are only created for props the device supports. A device with few featu State lives in `DeviceState`, one per device. - `raw` is what the device last reported, as integers. -- `pending` is what we want to send next. `set()` writes here. Reads check `pending` first, then `raw`. -- `push_device_status()` sends the pending values in one command and then refreshes `raw`. +- `pending` is what we want to send next. `set()` writes here. Reads check `pending` first, then `held`, then `raw`. +- `held` is what was sent in the last commands and is not confirmed by the device yet. A hold ends when the device reports the sent value, or after 8 s. See [protocol.md](protocol.md#stale-state-after-a-command). +- `push_device_status()` sends the pending values in one command, moves them to `held`, and then refreshes `raw`. - `info` holds the `InfoProp` values as strings. - `unknown` holds columns the device sent that we do not know. They show up in diagnostics. - `supports(prop)` is true only if the prop is in `raw` and in the capability list. The beeper is always supported. diff --git a/docs/development.md b/docs/development.md index 470fbab..288f8d9 100644 --- a/docs/development.md +++ b/docs/development.md @@ -40,10 +40,10 @@ report. | `test_transport_udp.py` | Retries, backoff, the split of a command when batching is off | | `test_transport_mqtt.py` | Topics, matching a response to its request, pushed status | | `test_discovery_local.py` | Scan, silence, several devices, the listen window, broken replies | -| `test_discovery_vrf.py` | A gateway with sub-devices, in both reply shapes | +| `test_discovery_vrf.py` | A gateway with sub-devices: the three request forms, joining their lists, both reply shapes, gateways side by side | | `test_discovery_merge.py` | Cloud discovery and merging it with the local list | | `test_cloud_api.py` | Login, homes, devices, duplicates, firmware info | -| `test_device_state.py` | Reads, pending values, what counts as supported, pruning | +| `test_device_state.py` | Reads, pending values, held values and their TTL, what counts as supported, pruning | | `test_device_api_client.py` | Bind, the column probe, diagnostic sweeps, listeners | | `test_device.py` | The poll cycle and the rules between features | @@ -79,7 +79,8 @@ none of that. - `tests/fakes/device.py` has `FakeGreeDevice`. It answers `scan`, `bind`, `status` and `cmd` over UDP, and it records every request. - `tests/fakes/vrf.py` has `FakeVrfGateway`. It adds `subCnt` to the scan reply - and answers the sub-device list, at the top level or inside a pack. + and answers the sub-device list, at the top level or inside a pack. `forms` + picks which of the three request forms it answers, each with its own units. - `tests/fakes/cloud.py` has `FakeGreeCloud`. It serves the cloud REST API over real HTTP, with the same encryption the app uses. - `tests/fakes/transport.py` has `FakePushTransport`. It answers from a @@ -91,8 +92,9 @@ none of that. A new failure mode is a new keyword on `FakeGreeDevice`, not a new class. The ones that exist are `answer_scan`, `answer_bind`, `scan_delay`, `reply_delay`, `max_columns`, `unsupported_props`, `ignore_first`, `drop_after`, `raw_reply`, -`reply_key`, `answer_status` and `scan_info`. There is also `rotate_key()`, for a device that -hands out a new session key. +`reply_key`, `answer_status`, `scan_info`, `stale_reads_after_cmd` and +`apply_commands`. There is also `rotate_key()`, for a device that hands out a +new session key, and `catch_up()`, which ends the stale reads at once. The fake encrypts with the component's own cipher. That is a trade-off: it keeps the fake short and gives V2 for free, but a bug in the cipher could diff --git a/docs/protocol.md b/docs/protocol.md index 0ed400c..5749380 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -39,6 +39,41 @@ Facts about how Gree devices behave on the wire. Most of these are not visible f - `extra_scan_networks` and `extra_scan_hosts` exist for devices on another subnet, where a broadcast does not reach. Those are probed with unicast. - A device that does not answer the scan cannot be bound. Setup retries the scan with the normal transport retries. +## VRF gateways + +A VRF gateway is one WiFi module (seen: GR-Gcloud, firmware V3.2.M) with several indoor units behind it. It answers the scan with `subCnt` above zero. Discovery binds it with the normal bind and then asks it for the list of its units. + +### The sub-device list request + +There are three forms of the request. Different WiFi module firmwares answer different forms, and some gateways return a different subset of units in each form. All three were seen on real hardware in PR 507 of the 4.x line. + +| Form (`SubListForm`) | Envelope `t`, `i` | Pack | Reply encrypted with | +|---|---|---|---| +| `device-key` | `pack`, `0` | `{"mac": , "t": "subList", "i": 0}` | the bound device key | +| `generic-key` | `subList`, `1` | `{"mac": , "i": 1}` | the generic key | +| `subDev` | `pack`, `0` | `{"cid": , "i": 0, "mac": , "t": "subDev"}` | the bound device key | + +- The request pack is always encrypted with the bound device key. Only the `generic-key` form has a reply in another key: the generic key, as for a scan or a bind. `transport.request_json()` takes a `response_cipher` for this one case. Every other request decrypts the reply with the request cipher. +- The `subDev` form is for older W06 class modules (seen: `362001067012+U-W06AV30.bin`, ver `V1.1.0.0`). They do not answer `subList` at all. +- With V1, `_get_sub_devices_list()` sends all three forms in the order of the table and joins the lists by `mac`, in the order the units were first seen. On the GR-Gcloud V3.2.M gateway from PR 507 the counts were device key 4, generic key 3, subDev 4, joined 4. One debug line per gateway shows the count per form and the joined count. +- With V2 (GCM) only the `device-key` form is sent. It is the only form known to work with V2. +- The list is at the top level of the reply on some firmwares and inside the pack on others. Both are read. +- A form that gets no answer, or an answer that cannot be decrypted or holds no list, is skipped. Each form uses the retries and the timeout of the transport. With the discovery defaults (2 attempts, 2 s) a silent form costs about 4.5 s, so a gateway that answers only the last form takes about 9 s longer. +- If no form answers, one warning is logged and the gateway gives no units. Discovery of the other devices goes on. If the joined list has another length than `subCnt`, a warning says so. +- Before this, the request was one hybrid form: envelope `subList` with `i: 0` and the pack of the `generic-key` form, with the reply read with the device key. It was never confirmed on hardware, and it is no longer sent. +- Discovery handles the scan replies side by side (`asyncio.gather`), so gateways do not wait for each other. Each gateway gets its own `GreeUdpTransport`, which uses a connected socket, so a reply from one gateway cannot reach the request of another. + +### Stale state after a command + +A gateway keeps a cached copy of the state of every indoor unit. For a few seconds after a command it can answer a status request from that cache, with the old values. Without a guard, the UI would jump back to the old value. + +- After a successful command, `push_device_status()` calls `DeviceState.hold()` with the values it sent. While a prop is held, `get()` returns the sent value instead of the reported one. +- A hold ends when the device reports the sent value. That check uses the value the device reported, never the held value. +- A hold also ends after `HELD_VALUE_TTL` (8 s) seconds. After that the reported value wins, so a command the device rejected is not shown for ever. +- Props that are not polled, like the beeper, are not held, because they are never reported. +- This applies to every device. A standalone unit reports the new value on the read right after the command, so its hold ends at once. +- While a hold is open after a command, the coordinator polls again every 2 s (`FOLLOW_UP_REFRESH_DELAY`), so the UI shows the confirmed value soon instead of at the next scan interval. The polls stop when the device confirms, or with the first poll after the hold ends. That poll shows what the device really reports, so a rejected command is visible within about 8 s. A standalone unit confirms on the read right after the command and gets no extra poll. + ## MAC addresses MACs are written in lower case with no separators. Talking to a device needs two MACs: the device MAC (the unit to control) and the controller MAC (the unit that manages it). diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 0b0bf8f..e10ed22 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -70,6 +70,10 @@ The integration raises repair issues under **Settings** > **System** > **Repairs **The Gree app logs me out.** Gree allows one session per account, and every login by the integration ends the app session. The integration logs in only when it has no session token yet. That is at the first setup of an account, at a YAML import with new account details, and at reauthentication. Loading or reloading the entry uses the stored token. See [Gree account](configuration.md#gree-account). +**A setting jumps back after a few seconds.** After a change, the integration shows the new value for up to 8 seconds while the device still reports the old one. A VRF controller does this for a few seconds after every change. If the device still reports the old value after 8 seconds, it did not take the change, and the old value is shown again. Check that the setting is allowed in the current mode. + +**Not every indoor unit of a VRF system is found.** The controller is asked for its units in three ways, because WiFi modules differ. Turn on [debug logging](#enable-debug-logging) and run the discovery again. The line `Sub-device list per form` shows how many units each way returned. Attach it to an issue. + **The device is on another VLAN and is not found.** Broadcasts do not cross VLANs. Add the network or the IP under **Extra Networks** or **Extra Hosts**, and allow UDP 7000 in the firewall. **Two Gree integrations show up.** Home Assistant ships its own `gree` integration. This one is called **Gree Climate** in the setup dialog and has the domain `gree_custom`. Both can be installed, but do not add the same device to both. diff --git a/tests/fakes/device.py b/tests/fakes/device.py index 66801f6..8dc4246 100644 --- a/tests/fakes/device.py +++ b/tests/fakes/device.py @@ -60,6 +60,8 @@ def __init__( raw_reply: bytes | None = None, reply_key: str | None = None, scan_info: dict[str, Any] | None = None, + stale_reads_after_cmd: int = 0, + apply_commands: bool = True, ) -> None: """Set up the device. @@ -85,6 +87,12 @@ def __init__( reply_key: Encrypt replies with this key instead of the session key, which is what a device with a rotated key looks like. scan_info: Extra fields for the scan reply, for example subCnt. + stale_reads_after_cmd: After a command, answer this many status + requests with the values from before the command, as a VRF + gateway does from its cache. Counted per request, and one poll + can be several requests. `catch_up()` ends it at once. + apply_commands: False means a command is acknowledged but the + values do not change. That is a command the unit rejected. """ self.mac = mac @@ -104,6 +112,13 @@ def __init__( self.raw_reply = raw_reply self.reply_key = reply_key self.scan_info = scan_info + self.stale_reads_after_cmd = stale_reads_after_cmd + self.apply_commands = apply_commands + + # The cached state a gateway answers with after a command, and how + # many more status requests it is used for. + self._stale_values: dict[str, Any] | None = None + self._stale_reads_left = 0 # A device does not type its values. Info columns come back as text # and some units answer an int where others answer a string. @@ -150,6 +165,11 @@ def reset_record(self) -> None: self.request_times.clear() self.keys_used.clear() + def catch_up(self) -> None: + """Stop answering with the state from before the last command.""" + self._stale_values = None + self._stale_reads_left = 0 + def rotate_key(self) -> None: """Hand out a new session key, as a device does after a power cycle.""" self.session_key = ROTATED_SESSION_KEY @@ -327,21 +347,36 @@ def build_status(self, pack: dict[str, Any]) -> dict[str, Any]: # A unit at its column limit answers r=200 with nothing in it. return {"t": "dat", "mac": self.mac, "r": 200, "cols": [], "dat": []} + values = self.values + if self._stale_values is not None and self._stale_reads_left > 0: + values = self._stale_values + self._stale_reads_left -= 1 + if self._stale_reads_left == 0: + self._stale_values = None + answered = [col for col in cols if col not in self.unsupported_props] return { "t": "dat", "mac": self.mac, "r": 200, "cols": answered, - "dat": [self.values.get(col, 0) for col in answered], + "dat": [values.get(col, 0) for col in answered], } def build_command_result(self, pack: dict[str, Any]) -> dict[str, Any]: """Acknowledge a command by mirroring its options and values.""" options = list(pack.get("opt", [])) values = list(pack.get("p", [])) - for option, value in zip(options, values, strict=False): - self.values[option] = value + + if self.stale_reads_after_cmd > 0: + # Keep the oldest cache when commands follow each other quickly. + if self._stale_values is None: + self._stale_values = dict(self.values) + self._stale_reads_left = self.stale_reads_after_cmd + + if self.apply_commands: + for option, value in zip(options, values, strict=False): + self.values[option] = value return { "t": "res", "mac": self.mac, diff --git a/tests/fakes/vrf.py b/tests/fakes/vrf.py index be7f939..3652e91 100644 --- a/tests/fakes/vrf.py +++ b/tests/fakes/vrf.py @@ -1,17 +1,35 @@ """A fake Gree VRF gateway: one controller that fronts several indoor units. A gateway answers a scan with `subCnt` above zero. The client then asks for the -sub-device list. Real gateways answer that in two shapes, one with the list at -the top level and one with the list inside an encrypted pack, so the fake can -produce both. +sub-device list. Real gateways know three forms of that request, and a given +WiFi module may answer only some of them (see docs/protocol.md): + +- device-key: a `pack` envelope with `t: subList` inside. The reply uses the + bound device key. +- generic-key: a `subList` envelope with `i: 1`. The request uses the bound + device key, but the reply uses the generic key. +- subDev: a `pack` envelope with `t: subDev` inside, for older W06 modules. + The reply uses the bound device key. + +Each form can be switched on or off on its own, and each can return its own +subset of the units. A reply has the list at the top level or inside an +encrypted pack, so the fake can produce both shapes. """ +from collections.abc import Mapping, Sequence +import time from typing import Any, override +from aiogree.api import SubListForm + from .device import DEFAULT_SESSION_KEY, FakeGreeDevice GATEWAY_MAC = "9424b8fd5ba3" +# A request that matches none of the known forms, for example the old request +# with a `subList` envelope and `i: 0`. +UNKNOWN_FORM = "unknown" + def sub_device_macs(gateway_mac: str, count: int) -> list[str]: """Build the MACs of the indoor units under a gateway. @@ -34,6 +52,7 @@ def __init__( returned: int | None = None, list_shape: str = "top", answer_sublist: bool = True, + forms: Mapping[SubListForm, Sequence[int] | None] | None = None, **kwargs: Any, ) -> None: """Set up the gateway. @@ -47,8 +66,13 @@ def __init__( them. A smaller number is a gateway that promised more than it delivers. list_shape: "top" puts the list at the top level of the reply, - "pack" puts it inside a pack encrypted with the session key. - answer_sublist: False means the gateway never answers the request. + "pack" puts it inside an encrypted pack. + answer_sublist: False means the gateway answers no form at all. + forms: The forms this gateway answers, each with the indexes of + the units that form returns. None as a value means the first + `returned` units. A form that is not in the mapping gets no + answer. None for the whole mapping means every form answers + with the first `returned` units. kwargs: Anything FakeGreeDevice accepts. """ @@ -62,33 +86,63 @@ def __init__( self.returned = sub_count if returned is None else returned self.list_shape = list_shape self.answer_sublist = answer_sublist + self.forms: Mapping[SubListForm, Sequence[int] | None] = ( + dict.fromkeys(SubListForm) if forms is None else forms + ) - # What the gateway saw on the sub-device request. + # What the gateway saw on the sub-device requests, one entry per + # request: the envelope, the form, the key that opened the pack and + # the monotonic time it came in. self.sublist_requests: list[dict[str, Any]] = [] - self.sublist_key_used: str | None = None + self.sublist_forms: list[str] = [] + self.sublist_keys: list[str] = [] + self.sublist_times: list[float] = [] + + def sub_devices(self, form: SubListForm) -> list[dict[str, str]]: + """Return the units the gateway reports for one form.""" + indexes = self.forms.get(form) + if indexes is None: + indexes = range(self.returned) + + macs = sub_device_macs(self.mac, max([self.sub_count, *indexes]) + 1) + return [{"mac": macs[index], "mid": "6049"} for index in indexes] - def sub_devices(self) -> list[dict[str, str]]: - """Return the units the gateway really reports.""" - return [ - {"mac": mac, "mid": "6049"} - for mac in sub_device_macs(self.mac, self.sub_count)[: self.returned] - ] + @staticmethod + def sublist_form(envelope: dict[str, Any], pack: dict[str, Any]) -> str | None: + """Tell which form a request is, or None if it is not a sub-device request.""" + if envelope.get("t") == "subList": + return SubListForm.GENERIC_KEY if envelope.get("i") == 1 else UNKNOWN_FORM + + if envelope.get("t") == "pack" and pack.get("t") == "subList": + return SubListForm.DEVICE_KEY + + if envelope.get("t") == "pack" and pack.get("t") == "subDev": + return SubListForm.SUB_DEV + + return None @override def handle_pack( self, envelope: dict[str, Any], pack: dict[str, Any] ) -> dict[str, Any] | None: """Answer the sub-device list, or fall back to normal device behaviour.""" - if envelope.get("t") != "subList": + form = self.sublist_form(envelope, pack) + if form is None: return super().handle_pack(envelope, pack) self.sublist_requests.append(envelope) - self.sublist_key_used = self.keys_used[-1] if self.keys_used else None + self.sublist_forms.append(form) + self.sublist_keys.append(self.keys_used[-1] if self.keys_used else "none") + self.sublist_times.append(time.monotonic()) + + if not self.answer_sublist or form == UNKNOWN_FORM: + return None - if not self.answer_sublist: + known_form = SubListForm(form) + if known_form not in self.forms: return None - units = self.sub_devices() + units = self.sub_devices(known_form) body: dict[str, Any] = { "t": "subList", "i": 0, @@ -98,6 +152,13 @@ def handle_pack( } if self.list_shape == "pack": - return self._wrap(body, self.session_cipher()) + # The generic key form is answered with the generic key, the + # other two with the bound key. + cipher = ( + self.generic_cipher() + if known_form is SubListForm.GENERIC_KEY + else self.session_cipher() + ) + return self._wrap(body, cipher) return body diff --git a/tests/test_device.py b/tests/test_device.py index 6c3ad5a..39c89ff 100644 --- a/tests/test_device.py +++ b/tests/test_device.py @@ -272,6 +272,62 @@ async def test_the_beeper_setting_is_forced_on_every_push( assert bound.beeper is True +async def test_a_standalone_unit_confirms_a_command_on_the_first_read( + unit: FakeGreeDevice, bound: GreeDevice +) -> None: + """The read right after the command already shows the new value.""" + bound.set_power_mode(False) + + await bound.push_device_status() + + assert not bound.has_held_values + assert bound.power_mode is False + assert bound.gather_diagnostics()["state_held"] == {} + + +async def test_a_stale_read_after_a_command_keeps_the_sent_value( + unit: FakeGreeDevice, bound: GreeDevice +) -> None: + """A VRF gateway answers from its cache for a while. The UI must not flip back.""" + unit.stale_reads_after_cmd = 100 + bound.set_power_mode(False) + + await bound.push_device_status() + + assert unit.values[GreeProp.POWER.value] == 0 + assert bound.has_held_values + assert bound.power_mode is False + assert bound.gather_diagnostics()["state_held"] == {GreeProp.POWER.value: 0} + + await bound.fetch_device_status() + + assert bound.power_mode is False + + unit.catch_up() + await bound.fetch_device_status() + + assert not bound.has_held_values + assert bound.power_mode is False + + +async def test_a_command_the_unit_ignores_stays_held_until_the_ttl( + unit: FakeGreeDevice, bound: GreeDevice +) -> None: + """The unit acknowledges but keeps its old value. That is never a confirmation. + + What happens when the TTL runs out is tested in test_device_state.py with a + fake clock. + """ + unit.apply_commands = False + bound.set_power_mode(False) + + await bound.push_device_status() + + assert unit.values[GreeProp.POWER.value] == 1 + assert bound.has_held_values + assert bound.power_mode is False + + async def test_a_poll_that_gets_no_answer_raises( unit: FakeGreeDevice, bound: GreeDevice ) -> None: diff --git a/tests/test_device_state.py b/tests/test_device_state.py index 0d6f3fd..3b07723 100644 --- a/tests/test_device_state.py +++ b/tests/test_device_state.py @@ -4,6 +4,10 @@ read returns, when a property counts as supported, and when a property is dropped from polling. A property only ever leaves the poll list, it never comes back, so a wrong drop is permanent for the life of the object. + +Held values are what was sent in the last command and is not confirmed yet. A +VRF gateway can report its old cached state for a few seconds after a command. +The hold tests use a fake clock, so no test waits for the real time to pass. """ # pylint: disable=redefined-outer-name @@ -11,6 +15,7 @@ # pytest pattern, not shadowing. from aiogree.api import GreeProp, InfoProp +from aiogree.const import HELD_VALUE_TTL from aiogree.device_state import DeviceState import pytest @@ -25,6 +30,34 @@ def state() -> DeviceState: return DeviceState(device_id=DEVICE_ID, capabilities=list(GreeProp)) +class FakeClock: + """Monotonic seconds that only move when a test says so.""" + + def __init__(self) -> None: + """Start at a fixed time.""" + self.now = 1000.0 + + def __call__(self) -> float: + """Return the current time.""" + return self.now + + def advance(self, seconds: float) -> None: + """Move the time forward.""" + self.now += seconds + + +@pytest.fixture +def clock() -> FakeClock: + """Return a clock the test controls.""" + return FakeClock() + + +@pytest.fixture +def timed_state(clock: FakeClock) -> DeviceState: + """Build a state object that reads the time from the fake clock.""" + return DeviceState(device_id=DEVICE_ID, capabilities=list(GreeProp), clock=clock) + + def seed(state: DeviceState, values: dict[GreeProp, int]) -> None: """Fill the raw state the way a status reply would.""" state.process_new_state({prop.value: str(value) for prop, value in values.items()}) @@ -199,3 +232,129 @@ def test_the_views_are_read_only(state: DeviceState) -> None: with pytest.raises(TypeError): state.raw[GreeProp.POWER] = 0 # type: ignore[index] + + +# +# Held values +# + + +def test_a_stale_report_does_not_undo_a_sent_value(timed_state: DeviceState) -> None: + """The gateway still reports the old value, but the read shows the sent one.""" + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 21}) + timed_state.hold({GreeProp.TARGET_TEMPERATURE: 24}) + + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 21}) + + assert timed_state.get(GreeProp.TARGET_TEMPERATURE) == 24 + assert timed_state.raw[GreeProp.TARGET_TEMPERATURE] == 21 + assert timed_state.held == {GreeProp.TARGET_TEMPERATURE: 24} + + +def test_a_report_of_the_sent_value_ends_the_hold(timed_state: DeviceState) -> None: + """Once confirmed, later reports count again, for example from a remote.""" + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 21}) + timed_state.hold({GreeProp.TARGET_TEMPERATURE: 24}) + + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 24}) + + assert timed_state.held == {} + + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 19}) + + assert timed_state.get(GreeProp.TARGET_TEMPERATURE) == 19 + + +def test_a_hold_ends_when_its_time_is_up( + timed_state: DeviceState, clock: FakeClock +) -> None: + """After the TTL the reported value wins, even without a new report.""" + seed(timed_state, {GreeProp.POWER: 1}) + timed_state.hold({GreeProp.POWER: 0}) + seed(timed_state, {GreeProp.POWER: 1}) + + clock.advance(HELD_VALUE_TTL - 0.1) + assert timed_state.get(GreeProp.POWER) == 0 + + clock.advance(0.1) + assert timed_state.get(GreeProp.POWER) == 1 + assert timed_state.held == {} + + +def test_a_rejected_command_is_not_shown_for_ever( + timed_state: DeviceState, clock: FakeClock +) -> None: + """A device that keeps its old value gets the last word after the TTL.""" + seed(timed_state, {GreeProp.FAN_SPEED: 0}) + timed_state.hold({GreeProp.FAN_SPEED: 3}) + + for _ in range(4): + seed(timed_state, {GreeProp.FAN_SPEED: 0}) + assert timed_state.get(GreeProp.FAN_SPEED) == 3 + clock.advance(HELD_VALUE_TTL / 4) + + seed(timed_state, {GreeProp.FAN_SPEED: 0}) + + assert timed_state.get(GreeProp.FAN_SPEED) == 0 + assert timed_state.held == {} + + +def test_a_new_command_starts_a_new_hold( + timed_state: DeviceState, clock: FakeClock +) -> None: + """The TTL counts from the last time a value was sent.""" + seed(timed_state, {GreeProp.POWER: 1}) + timed_state.hold({GreeProp.POWER: 0}) + clock.advance(HELD_VALUE_TTL - 1) + + timed_state.hold({GreeProp.POWER: 0}) + clock.advance(2) + + assert timed_state.get(GreeProp.POWER) == 0 + + +def test_a_pending_value_wins_over_a_held_one(timed_state: DeviceState) -> None: + """A change that is not sent yet is newer than the one that was sent.""" + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 21}) + timed_state.hold({GreeProp.TARGET_TEMPERATURE: 24}) + timed_state.set(GreeProp.TARGET_TEMPERATURE, 25) + + assert timed_state.get(GreeProp.TARGET_TEMPERATURE) == 25 + assert timed_state.pending == {GreeProp.TARGET_TEMPERATURE: 25} + assert timed_state.held == {GreeProp.TARGET_TEMPERATURE: 24} + + timed_state.clear_pending() + + assert timed_state.get(GreeProp.TARGET_TEMPERATURE) == 24 + + +def test_a_change_back_to_the_stale_value_is_still_sent( + timed_state: DeviceState, +) -> None: + """The device was told 24, so going back to 21 is a real change.""" + seed(timed_state, {GreeProp.TARGET_TEMPERATURE: 21}) + timed_state.hold({GreeProp.TARGET_TEMPERATURE: 24}) + + timed_state.set(GreeProp.TARGET_TEMPERATURE, 21) + assert timed_state.has_pending_updates + + timed_state.set(GreeProp.TARGET_TEMPERATURE, 24) + assert not timed_state.has_pending_updates + + +def test_props_that_are_not_polled_are_not_held(timed_state: DeviceState) -> None: + """The beeper is never reported, so there is nothing to wait for.""" + seed(timed_state, {GreeProp.POWER: 1}) + timed_state.hold({GreeProp.POWER: 0, GreeProp.BEEPER: 1, GreeProp.BEEPER_NEW: 0}) + + assert timed_state.held == {GreeProp.POWER: 0} + + +def test_removing_a_prop_drops_its_hold(timed_state: DeviceState) -> None: + """A prop that is no longer polled can never be confirmed.""" + seed(timed_state, {GreeProp.FEAT_LIGHT: 1}) + timed_state.hold({GreeProp.FEAT_LIGHT: 0}) + + timed_state.remove(GreeProp.FEAT_LIGHT) + + assert timed_state.held == {} diff --git a/tests/test_discovery_vrf.py b/tests/test_discovery_vrf.py index f97083e..3cee056 100644 --- a/tests/test_discovery_vrf.py +++ b/tests/test_discovery_vrf.py @@ -4,6 +4,10 @@ for the list of indoor units. Real gateways answer that in two shapes, with the list at the top level or inside an encrypted pack, and both have to work. +The request itself has three forms (device key, generic key, subDev), and a +WiFi module may answer only some of them, each with its own subset of units. +The client asks all of them and joins the lists by MAC. + These tests were written against the code before PR 514, where discovery of a device with `subCnt` above zero was broken: the request went out with the generic key and the reply was read as a pack, so the gateway answer @@ -23,11 +27,14 @@ from collections.abc import AsyncIterator, Awaitable, Callable from typing import Any +from aiogree.api import SubListForm, _get_sub_devices_list +from aiogree.cipher import EncryptionVersion, get_cipher +from aiogree.transport_udp import GreeUdpTransport import pytest from .conftest import DISCOVERY_PORT, DiscoverAll, DiscoverOne, RecordingHandler from .fakes.device import DEFAULT_SESSION_KEY, FakeGreeDevice -from .fakes.vrf import GATEWAY_MAC, FakeVrfGateway +from .fakes.vrf import GATEWAY_MAC, FakeVrfGateway, sub_device_macs GatewayFactory = Callable[..., Awaitable[FakeVrfGateway]] @@ -115,16 +122,156 @@ async def test_every_sub_unit_is_addressable( assert {dev.host for dev in found} == {gateway.host} -async def test_the_sub_device_request_uses_the_bound_key( +async def test_every_form_is_sent_with_the_bound_key( gateway_factory: GatewayFactory, discover_one: DiscoverOne ) -> None: - """The gateway is bound first, so the request is readable with its key.""" + """The gateway is bound first, so each form is readable with its key.""" gateway = await gateway_factory(sub_count=4) await discover_one(gateway.host, timeout=1) - assert gateway.sublist_key_used == "session" - assert gateway.sublist_requests[0]["i"] == 0 + assert gateway.sublist_forms == [ + SubListForm.DEVICE_KEY, + SubListForm.GENERIC_KEY, + SubListForm.SUB_DEV, + ] + assert gateway.sublist_keys == ["session", "session", "session"] + assert [(req["t"], req["i"]) for req in gateway.sublist_requests] == [ + ("pack", 0), + ("subList", 1), + ("pack", 0), + ] + + +@pytest.mark.parametrize("list_shape", ["top", "pack"]) +async def test_the_forms_are_joined_by_mac( + gateway_factory: GatewayFactory, + discover_one: DiscoverOne, + gree_logs: RecordingHandler, + list_shape: str, +) -> None: + """Four units from the device key form and three from the generic one make four. + + This is what the GR-Gcloud V3.2.M gateway in PR 507 answered. + """ + gateway = await gateway_factory( + sub_count=4, + list_shape=list_shape, + forms={ + SubListForm.DEVICE_KEY: [0, 1, 2, 3], + SubListForm.GENERIC_KEY: [0, 1, 2], + }, + ) + + found = await discover_one(gateway.host, timeout=1) + + assert [dev.mac for dev in found] == sub_device_macs(GATEWAY_MAC, 4) + assert not any("sub-devices" in line for line in gree_logs.warnings()) + assert ( + f"[{GATEWAY_MAC}] Sub-device list per form: device-key=4, " + "generic-key=3, subDev=n/a, merged=4" + ) in gree_logs.messages() + + +async def test_units_that_only_one_form_knows_are_kept_in_first_seen_order( + gateway_factory: GatewayFactory, discover_one: DiscoverOne +) -> None: + """Each form adds the units the earlier forms did not have.""" + gateway = await gateway_factory( + sub_count=4, + forms={ + SubListForm.DEVICE_KEY: [0, 1], + SubListForm.GENERIC_KEY: [3, 1], + SubListForm.SUB_DEV: [2, 0], + }, + ) + + found = await discover_one(gateway.host, timeout=1) + + macs = sub_device_macs(GATEWAY_MAC, 4) + assert [dev.mac for dev in found] == [macs[0], macs[1], macs[3], macs[2]] + + +async def test_a_gateway_that_only_answers_the_generic_key_form( + gateway_factory: GatewayFactory, + discover_one: DiscoverOne, + gree_logs: RecordingHandler, +) -> None: + """One unit behind the gateway, found through the generic key form alone. + + The reply comes in a pack encrypted with the generic key, while the request + used the bound key. Reading the reply with the bound key would fail. + """ + gateway = await gateway_factory( + sub_count=1, list_shape="pack", forms={SubListForm.GENERIC_KEY: None} + ) + + found = await discover_one(gateway.host, timeout=1) + + assert [dev.mac for dev in found] == sub_device_macs(GATEWAY_MAC, 1) + assert found[0].key == DEFAULT_SESSION_KEY + assert not any("sub-devices" in line for line in gree_logs.warnings()) + + +async def test_a_gateway_that_only_answers_the_sub_dev_form( + gateway_factory: GatewayFactory, + discover_one: DiscoverOne, + gree_logs: RecordingHandler, +) -> None: + """An older W06 module only knows subDev.""" + gateway = await gateway_factory( + sub_count=2, list_shape="pack", forms={SubListForm.SUB_DEV: None} + ) + + found = await discover_one(gateway.host, timeout=1) + + assert [dev.mac for dev in found] == sub_device_macs(GATEWAY_MAC, 2) + assert not any("sub-devices" in line for line in gree_logs.warnings()) + + +async def test_a_gateway_that_answers_no_form_gives_nothing_and_warns( + gateway_factory: GatewayFactory, + discover_one: DiscoverOne, + gree_logs: RecordingHandler, +) -> None: + """Each form is tried, then one warning says why no units came back.""" + gateway = await gateway_factory(sub_count=4, answer_sublist=False) + + found = await discover_one(gateway.host, timeout=1) + + assert found == [] + assert len(gateway.sublist_forms) == 3 + assert [line for line in gree_logs.warnings() if "sub-devices" in line] == [ + f"[{GATEWAY_MAC}] VRF gateway did not answer any form of the sub-device list request. Its sub-devices will be ignored" + ] + + +async def test_a_v2_gateway_is_only_asked_the_device_key_form() -> None: + """Only the device key form is known to work with AES-GCM. + + This calls the list query straight away with the bound key, because the + fake answers a scan in V2 and discovery reads scan replies in V1. + """ + gateway = FakeVrfGateway( + sub_count=4, list_shape="pack", encryption_version=EncryptionVersion.V2 + ) + await gateway.start() + transport = GreeUdpTransport(gateway.host, gateway.port, max_retries=1, timeout=1) + + try: + found = await _get_sub_devices_list( + GATEWAY_MAC, + 0, + get_cipher(EncryptionVersion.V2, DEFAULT_SESSION_KEY), + transport, + expected=4, + ) + finally: + await transport.disconnect() + gateway.close() + + assert [dev.mac for dev in found] == sub_device_macs(GATEWAY_MAC, 4) + assert gateway.sublist_forms == [SubListForm.DEVICE_KEY] async def test_a_gateway_that_never_answers_the_bind_does_not_lose_the_others( @@ -177,6 +324,36 @@ async def test_a_gateway_that_never_answers_the_list_does_not_lose_the_others( assert [dev.mac for dev in found] == ["f4911e3f1ac8"] +async def test_gateways_are_asked_for_their_units_at_the_same_time( + loopback_ips: Callable[[int], list[str]], + discover_all: DiscoverAll, +) -> None: + """Two slow gateways do not wait for each other. + + Both only answer the last form, so each spends about two seconds on the + two forms before it. One after another, the second gateway would get its + first sub-device request about two seconds after the first one. + """ + first_ip, second_ip = loopback_ips(2) + first = FakeVrfGateway(sub_count=2, forms={SubListForm.SUB_DEV: None}) + second = FakeVrfGateway( + mac="9424b8fd5ba4", sub_count=2, forms={SubListForm.SUB_DEV: None} + ) + await first.start(first_ip, DISCOVERY_PORT) + await second.start(second_ip, DISCOVERY_PORT) + + try: + found = await discover_all([first_ip, second_ip], timeout=1) + finally: + first.close() + second.close() + + assert sorted(dev.mac for dev in found) == sorted( + sub_device_macs(first.mac, 2) + sub_device_macs(second.mac, 2) + ) + assert abs(first.sublist_times[0] - second.sublist_times[0]) < 0.5 + + async def test_a_plain_device_is_untouched_by_any_of_this( loopback_ip: str, discover_one: DiscoverOne, gree_logs: RecordingHandler ) -> None: From a3d08f680ea7b87ba40c06191770d2a25603d2e7 Mon Sep 17 00:00:00 2001 From: Rob Hofmann Date: Thu, 24 Sep 2026 00:14:43 +0200 Subject: [PATCH 2/3] Only hold sent values for VRF sub-units The stale cache was only seen on VRF gateways. For a standalone unit a hold has a cost: when the unit corrects a value it cannot take, for example an unsupported swing mode, it reports its own value, and the hold kept the refused value on screen for up to 8 s. Standalone units now behave as they did before the hold. To find out whether a standalone unit or the MQTT transport caches too, every device now logs at debug level when the read right after a command does not report the sent value, with the transport and whether it is a sub-unit. Suite: 249 passed. --- .../gree_custom/aiogree/device.py | 35 +++++++++++- docs/architecture.md | 2 +- docs/protocol.md | 7 ++- tests/test_device.py | 56 ++++++++++++++++++- 4 files changed, 91 insertions(+), 9 deletions(-) diff --git a/custom_components/gree_custom/aiogree/device.py b/custom_components/gree_custom/aiogree/device.py index 93ea087..b03bcf0 100755 --- a/custom_components/gree_custom/aiogree/device.py +++ b/custom_components/gree_custom/aiogree/device.py @@ -318,12 +318,17 @@ async def push_device_status(self) -> None: await self._client.set_props({k.value: v for k, v in sent.items()}) _LOGGER.debug("[%s:%s] Device status set", self.unique_id, self.transport) - # Keep showing what was sent until the device reports it. A VRF - # gateway answers the read below with its old cached state. - self._state.hold(sent) + # A VRF gateway answers the read below from its cache, with the old + # state of the indoor unit. So for a sub-unit, keep showing what was + # sent until the gateway reports it. Other units are not held: a + # value the unit corrects, for example an unsupported swing mode, + # then shows at once. + if self.is_sub_unit: + self._state.hold(sent) self._state.clear_pending() await self.fetch_device_status() + self._log_unconfirmed_values(sent) except GreeConnectionError, GreeProtocolError: _LOGGER.exception( @@ -561,6 +566,30 @@ def available(self) -> bool: """Return True if the device is bound and last connection was successful.""" return self._client.bound and self._client.available + def _log_unconfirmed_values(self, sent: Mapping[GreeProp, int]) -> None: + """Log sent values that the read right after the command does not show. + + This runs for every device, held or not. It shows in the field which + devices and transports answer with an old or a corrected value. + """ + for prop, value in sent.items(): + reported = self._state.raw.get(prop) + if reported is not None and reported != value: + _LOGGER.debug( + "[%s:%s] The read right after the command reports %s=%d, but %d was sent (sub-unit: %s)", + self.unique_id, + self.transport, + prop, + reported, + value, + self.is_sub_unit, + ) + + @property + def is_sub_unit(self) -> bool: + """Return True if the device is an indoor unit behind a VRF gateway.""" + return self.mac_address != self.mac_address_controller + @property def has_held_values(self) -> bool: """Return True if sent values still wait for the device to confirm them.""" diff --git a/docs/architecture.md b/docs/architecture.md index d052371..99f44eb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -79,7 +79,7 @@ State lives in `DeviceState`, one per device. - `raw` is what the device last reported, as integers. - `pending` is what we want to send next. `set()` writes here. Reads check `pending` first, then `held`, then `raw`. -- `held` is what was sent in the last commands and is not confirmed by the device yet. A hold ends when the device reports the sent value, or after 8 s. See [protocol.md](protocol.md#stale-state-after-a-command). +- `held` is what was sent in the last commands to a VRF sub-unit and is not confirmed by the gateway yet. A hold ends when the device reports the sent value, or after 8 s. See [protocol.md](protocol.md#stale-state-after-a-command). - `push_device_status()` sends the pending values in one command, moves them to `held`, and then refreshes `raw`. - `info` holds the `InfoProp` values as strings. - `unknown` holds columns the device sent that we do not know. They show up in diagnostics. diff --git a/docs/protocol.md b/docs/protocol.md index 5749380..2f74bc9 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -67,12 +67,13 @@ There are three forms of the request. Different WiFi module firmwares answer dif A gateway keeps a cached copy of the state of every indoor unit. For a few seconds after a command it can answer a status request from that cache, with the old values. Without a guard, the UI would jump back to the old value. -- After a successful command, `push_device_status()` calls `DeviceState.hold()` with the values it sent. While a prop is held, `get()` returns the sent value instead of the reported one. +- After a successful command to a sub-unit, `push_device_status()` calls `DeviceState.hold()` with the values it sent. While a prop is held, `get()` returns the sent value instead of the reported one. - A hold ends when the device reports the sent value. That check uses the value the device reported, never the held value. - A hold also ends after `HELD_VALUE_TTL` (8 s) seconds. After that the reported value wins, so a command the device rejected is not shown for ever. - Props that are not polled, like the beeper, are not held, because they are never reported. -- This applies to every device. A standalone unit reports the new value on the read right after the command, so its hold ends at once. -- While a hold is open after a command, the coordinator polls again every 2 s (`FOLLOW_UP_REFRESH_DELAY`), so the UI shows the confirmed value soon instead of at the next scan interval. The polls stop when the device confirms, or with the first poll after the hold ends. That poll shows what the device really reports, so a rejected command is visible within about 8 s. A standalone unit confirms on the read right after the command and gets no extra poll. +- Only sub-units are held (`GreeDevice.is_sub_unit`, true when the device MAC differs from the controller MAC). The stale cache was only seen on VRF gateways. A standalone unit is not held, because a hold has a cost there: when the unit corrects a value it cannot take, for example a swing mode it does not support, it reports its own value, and a hold would keep the refused value on screen for up to 8 s. +- If a standalone unit or the MQTT transport turns out to cache as well, the UI shows the old value for a moment after a command, as it did before the hold existed. For every device, `push_device_status()` logs at debug level when the read right after a command does not report the sent value, with the transport and whether it is a sub-unit. That line is how to find out, before the hold is extended. +- While a hold is open after a command, the coordinator polls again every 2 s (`FOLLOW_UP_REFRESH_DELAY`), so the UI shows the confirmed value soon instead of at the next scan interval. The polls stop when the device confirms, or with the first poll after the hold ends. That poll shows what the device really reports, so a rejected command is visible within about 8 s. A standalone unit is never held, so it gets no extra poll. ## MAC addresses diff --git a/tests/test_device.py b/tests/test_device.py index 39c89ff..5bbc864 100644 --- a/tests/test_device.py +++ b/tests/test_device.py @@ -105,6 +105,20 @@ async def bound(unit: FakeGreeDevice) -> AsyncIterator[GreeDevice]: await transport.disconnect() +@pytest.fixture +async def bound_sub_unit(unit: FakeGreeDevice) -> AsyncIterator[GreeDevice]: + """Bind an indoor unit that sits behind the fake unit as its VRF gateway.""" + transport = GreeUdpTransport(unit.host, unit.port, max_retries=1, timeout=0.3) + device = GreeDevice(name="Zolderkamer VRF", mac_addr=f"{unit.mac}01") + await device.bind_with_transport( + local_controller_mac=unit.mac, local_transport=transport + ) + try: + yield device + finally: + await transport.disconnect() + + # # Bind and poll # @@ -286,9 +300,10 @@ async def test_a_standalone_unit_confirms_a_command_on_the_first_read( async def test_a_stale_read_after_a_command_keeps_the_sent_value( - unit: FakeGreeDevice, bound: GreeDevice + unit: FakeGreeDevice, bound_sub_unit: GreeDevice ) -> None: """A VRF gateway answers from its cache for a while. The UI must not flip back.""" + bound = bound_sub_unit unit.stale_reads_after_cmd = 100 bound.set_power_mode(False) @@ -311,13 +326,14 @@ async def test_a_stale_read_after_a_command_keeps_the_sent_value( async def test_a_command_the_unit_ignores_stays_held_until_the_ttl( - unit: FakeGreeDevice, bound: GreeDevice + unit: FakeGreeDevice, bound_sub_unit: GreeDevice ) -> None: """The unit acknowledges but keeps its old value. That is never a confirmation. What happens when the TTL runs out is tested in test_device_state.py with a fake clock. """ + bound = bound_sub_unit unit.apply_commands = False bound.set_power_mode(False) @@ -328,6 +344,42 @@ async def test_a_command_the_unit_ignores_stays_held_until_the_ttl( assert bound.power_mode is False +async def test_a_unit_that_is_not_behind_a_gateway_is_never_held( + unit: FakeGreeDevice, bound: GreeDevice, gree_logs: RecordingHandler +) -> None: + """Only sub-units are held. Other units behave as before the hold existed.""" + assert not bound.is_sub_unit + unit.stale_reads_after_cmd = 100 + bound.set_power_mode(False) + + await bound.push_device_status() + + assert not bound.has_held_values + assert bound.power_mode is True + assert any( + "The read right after the command reports Pow=1, but 0 was sent" in line + and "sub-unit: False" in line + for line in gree_logs.messages() + ) + + +async def test_a_value_the_unit_corrects_shows_at_once( + unit: FakeGreeDevice, bound: GreeDevice +) -> None: + """A unit that refuses a value reports its own one, and the UI follows it. + + For example a swing mode the unit does not support: it answers with a valid + one, and there is no hold that keeps the refused value on screen. + """ + unit.apply_commands = False + bound.set_power_mode(False) + + await bound.push_device_status() + + assert not bound.has_held_values + assert bound.power_mode is True + + async def test_a_poll_that_gets_no_answer_raises( unit: FakeGreeDevice, bound: GreeDevice ) -> None: From 11d3c7c5c3b91e022815a7e97746030624a571f0 Mon Sep 17 00:00:00 2001 From: Rob Hofmann Date: Thu, 24 Sep 2026 09:47:48 +0200 Subject: [PATCH 3/3] Send the generic-key sub-device list request with the generic key The generic-key form is answered with the generic key, but its request used the bound device key, so transport.request_json() needed a separate response_cipher for this one case. A probe on a GR-Gcloud V3.2.M gateway (PR 507) showed the gateway ignores the request pack of this form: device, generic and random keys all got the same answer, 3 units, readable with the generic key. The request now uses the generic key too, and response_cipher is removed again. The form stays in the union, because it adds units on some gateways. --- custom_components/gree_custom/aiogree/api.py | 26 +++++++------------ .../gree_custom/aiogree/transport.py | 11 ++------ docs/protocol.md | 4 +-- tests/fakes/vrf.py | 4 +-- tests/test_discovery_vrf.py | 16 ++++++++---- 5 files changed, 26 insertions(+), 35 deletions(-) diff --git a/custom_components/gree_custom/aiogree/api.py b/custom_components/gree_custom/aiogree/api.py index 6532a75..3efd3bd 100755 --- a/custom_components/gree_custom/aiogree/api.py +++ b/custom_components/gree_custom/aiogree/api.py @@ -575,7 +575,6 @@ async def gree_get_response( transport: GreeBaseTransport, max_attempts: int | None = None, timeout: float | None = None, - response_cipher: CipherBase | None = None, ) -> dict: """Send a request to the device and return the decoded response. @@ -586,8 +585,6 @@ async def gree_get_response( transport: Transport to send the emssage throuhg max_attempts: Attempts for this request instead of the transport's own timeout: Reply timeout for this request instead of the transport's own - response_cipher: Cipher to decrypt the reply with, if it differs from - the request cipher Returns: Decrypted JSON response @@ -596,12 +593,7 @@ async def gree_get_response( try: data = await transport.request_json( - mac_controller, - json_data, - cipher, - max_attempts, - timeout, - response_cipher=response_cipher, + mac_controller, json_data, cipher, max_attempts, timeout ) except GreeConnectionError: raise @@ -688,8 +680,8 @@ def _create_get_subdevices_payload( ) -> dict: """Create the request for one form of the sub-device list query. - Every form encrypts its pack with the bound device key. Only the reply - differs, see `_get_sub_devices_list`. + The `generic-key` form uses the generic key, the other two use the bound + device key, see `_get_sub_devices_list`. Args: form: Which form of the query to build @@ -1334,19 +1326,19 @@ async def _get_sub_devices_list( counts: dict[SubListForm, int | None] = {} for form in forms: - # The generic key form is encrypted with the device key, but the - # gateway answers it with the generic key, like a scan or a bind. - response_cipher = ( - get_cipher(cipher.version) if form is SubListForm.GENERIC_KEY else None + # The generic key form is answered with the generic key, like a scan + # or a bind. A GR-Gcloud gateway ignores the request pack of this form, + # so the generic key is used for the request as well. + form_cipher = ( + get_cipher(cipher.version) if form is SubListForm.GENERIC_KEY else cipher ) try: response = await gree_get_response( mac_addr_controller, _create_get_subdevices_payload(form, mac_addr_controller, uid), - cipher, + form_cipher, transport, - response_cipher=response_cipher, ) except GreeConnectionError, GreeProtocolError: _LOGGER.debug( diff --git a/custom_components/gree_custom/aiogree/transport.py b/custom_components/gree_custom/aiogree/transport.py index 5dc557c..c4db3d9 100755 --- a/custom_components/gree_custom/aiogree/transport.py +++ b/custom_components/gree_custom/aiogree/transport.py @@ -79,15 +79,8 @@ async def request_json( cipher: CipherBase, max_attempts: int | None = None, timeout: float | None = None, - response_cipher: CipherBase | None = None, ) -> dict[str, Any]: - """Send and receive a JSON payload. - - The request pack is encrypted with cipher. The reply pack is decrypted - with response_cipher when it is given, and with cipher otherwise. Only - one form of the VRF sub-device list query needs this: its request uses - the device key and its reply uses the generic key. - """ + """Send and receive a JSON payload.""" requests: list[dict[str, Any]] @@ -122,7 +115,7 @@ async def request_json( ) response = json.loads(raw_response) - response = gree_decrypt_pack(response, response_cipher or cipher) + response = gree_decrypt_pack(response, cipher) responses.append(response) diff --git a/docs/protocol.md b/docs/protocol.md index 2f74bc9..491e325 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -47,13 +47,13 @@ A VRF gateway is one WiFi module (seen: GR-Gcloud, firmware V3.2.M) with several There are three forms of the request. Different WiFi module firmwares answer different forms, and some gateways return a different subset of units in each form. All three were seen on real hardware in PR 507 of the 4.x line. -| Form (`SubListForm`) | Envelope `t`, `i` | Pack | Reply encrypted with | +| Form (`SubListForm`) | Envelope `t`, `i` | Pack | Key, request and reply | |---|---|---|---| | `device-key` | `pack`, `0` | `{"mac": , "t": "subList", "i": 0}` | the bound device key | | `generic-key` | `subList`, `1` | `{"mac": , "i": 1}` | the generic key | | `subDev` | `pack`, `0` | `{"cid": , "i": 0, "mac": , "t": "subDev"}` | the bound device key | -- The request pack is always encrypted with the bound device key. Only the `generic-key` form has a reply in another key: the generic key, as for a scan or a bind. `transport.request_json()` takes a `response_cipher` for this one case. Every other request decrypts the reply with the request cipher. +- The `generic-key` form is answered with the generic key, as a scan or a bind is, so its request uses the generic key too. The other two forms use the bound device key both ways. A GR-Gcloud V3.2.M gateway ignores the request pack of the `generic-key` form: a pack encrypted with the device key, with the generic key and with a random key all got the same answer, readable with the generic key (probe in PR 507). No gateway is known that reads the pack of this form. - The `subDev` form is for older W06 class modules (seen: `362001067012+U-W06AV30.bin`, ver `V1.1.0.0`). They do not answer `subList` at all. - With V1, `_get_sub_devices_list()` sends all three forms in the order of the table and joins the lists by `mac`, in the order the units were first seen. On the GR-Gcloud V3.2.M gateway from PR 507 the counts were device key 4, generic key 3, subDev 4, joined 4. One debug line per gateway shows the count per form and the joined count. - With V2 (GCM) only the `device-key` form is sent. It is the only form known to work with V2. diff --git a/tests/fakes/vrf.py b/tests/fakes/vrf.py index 3652e91..4133d94 100644 --- a/tests/fakes/vrf.py +++ b/tests/fakes/vrf.py @@ -6,8 +6,8 @@ - device-key: a `pack` envelope with `t: subList` inside. The reply uses the bound device key. -- generic-key: a `subList` envelope with `i: 1`. The request uses the bound - device key, but the reply uses the generic key. +- generic-key: a `subList` envelope with `i: 1`. Request and reply use the + generic key. - subDev: a `pack` envelope with `t: subDev` inside, for older W06 modules. The reply uses the bound device key. diff --git a/tests/test_discovery_vrf.py b/tests/test_discovery_vrf.py index 3cee056..5dcd965 100644 --- a/tests/test_discovery_vrf.py +++ b/tests/test_discovery_vrf.py @@ -122,10 +122,16 @@ async def test_every_sub_unit_is_addressable( assert {dev.host for dev in found} == {gateway.host} -async def test_every_form_is_sent_with_the_bound_key( +async def test_each_form_is_sent_with_its_own_key( gateway_factory: GatewayFactory, discover_one: DiscoverOne ) -> None: - """The gateway is bound first, so each form is readable with its key.""" + """The generic key form uses the generic key, the other two the bound key. + + The generic key form is answered with the generic key. On a GR-Gcloud + V3.2.M gateway the request pack of that form made no difference: the + device key, the generic key and a random key all got the same answer + (PR 507). So the request uses the generic key too. + """ gateway = await gateway_factory(sub_count=4) await discover_one(gateway.host, timeout=1) @@ -135,7 +141,7 @@ async def test_every_form_is_sent_with_the_bound_key( SubListForm.GENERIC_KEY, SubListForm.SUB_DEV, ] - assert gateway.sublist_keys == ["session", "session", "session"] + assert gateway.sublist_keys == ["session", "generic", "session"] assert [(req["t"], req["i"]) for req in gateway.sublist_requests] == [ ("pack", 0), ("subList", 1), @@ -199,8 +205,8 @@ async def test_a_gateway_that_only_answers_the_generic_key_form( ) -> None: """One unit behind the gateway, found through the generic key form alone. - The reply comes in a pack encrypted with the generic key, while the request - used the bound key. Reading the reply with the bound key would fail. + The reply comes in a pack encrypted with the generic key. Reading it with + the bound key would fail. """ gateway = await gateway_factory( sub_count=1, list_shape="pack", forms={SubListForm.GENERIC_KEY: None}