Skip to content

Commit 332b86e

Browse files
Beennnnclaudeedenhaus
authored
fix(messages): skip map-related legacy fallback for mower devices (#1565)
* fix(messages): skip map-related legacy fallback for mower devices When a mower (e.g. Ecovacs GOAT G1, A1600 RTK, A3000 LiDAR Pro) is mowing, its firmware pushes spontaneous `onMapTrace` MQTT messages. They flow through `messages.get_message()` which falls back via `_LEGACY_USE_GET_COMMAND` to `GetMapTrace`, whose parser does not support the on-wire format used by recent mower firmware (observed: 1.15.13). The parse fails inside `_handle_error_or_analyse` and emits a WARNING per push. In one user setup this produced 217 520 `Could not parse getMapTrace` warnings over 3 days (~70 k/day, ~50/min during mowing), bloating the HA `homeassistant.log` and the recorder DB by ~100 MB. Mowers do not expose a `map=` capability in their hardware definitions, so successfully parsed map-trace data would have nowhere to land anyway — the fallback is pure noise on these devices. This adds an optional `device_type` argument to `get_message` and `get_legacy_message`. When the device is a `MOWER`, map-related legacy messages (`onMapTrace`, `onMapSet`, `onMapSubSet`, `onMinorMap`, `onCachedMapInfo`, `onMultiMapState` and their `get/off/report` variants) are skipped silently before reaching the parser. Vacuums and devices with no specified type fall through unchanged. `Device._handle_message` reads `device_type` defensively via `getattr` so existing test mocks that don't expose the dataclass field on `spec_set=Capabilities` continue to work — they fall back to the no-filter path. Tests: - `test_get_messages` (existing 7 cases) — pass unchanged. - `test_get_messages_device_type_filter` (10 new cases) — verify mowers skip map-related legacy fallbacks while non-map messages and other device types remain unaffected. - Full suite: 707/707 pass, no regression. Refs: #1376 (Disable getMapTrace for Goat) Refs: #852 (GOAT A1600 RTK Support) * review: gate on map capability instead of mower device_type Address review feedback on #1565: - Drop the explanatory comment block above ``_MAP_LEGACY_COMMANDS`` - Replace the ``device_type == DeviceType.MOWER`` check with a capability-based one. ``get_message`` now takes ``has_map: bool`` (keyword-only) and ``get_legacy_message`` skips map-related fallbacks when the device does not expose a ``map`` capability — independent of whether the device is a mower, vacuum, or future device class. - ``Device._handle_message`` derives ``has_map`` from ``capabilities.map`` via ``getattr`` so test mocks unaffected. * review: pass StaticDeviceInfo to get_message, remove getattr Address review feedback from edenhaus: - Pass the whole `static: StaticDeviceInfo` object to `get_message()` instead of extracting `data_type` and `has_map` separately at the call site. - Remove `getattr` in device.py — access `capabilities.map` directly since the attribute is defined on the dataclass. - Remove `lru_cache` on `get_message` since `StaticDeviceInfo` contains non-trivially-hashable `Capabilities` objects. The function is cheap enough without caching (string lookups + one regex). - Update tests to use real `StaticDeviceInfo` fixtures (yna5xi with map, xmp9ds without map) instead of passing primitive params. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Minor sorting * refactor tests * forgotten prek * Use realistic bot for test --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Robert Resch <robert@resch.dev>
1 parent 7266b58 commit 332b86e

5 files changed

Lines changed: 80 additions & 51 deletions

File tree

deebot_client/device.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,10 @@ def _handle_message(
228228
try:
229229
_LOGGER.debug("Try to handle message %s: %s", message_name, message_data)
230230

231-
if message := get_message(message_name, self._device_info.static.data_type):
231+
if message := get_message(
232+
message_name,
233+
self._device_info.static,
234+
):
232235
result = message.handle(self.events, message_data)
233236
if result.state == HandlingState.SUCCESS and result.requested_commands:
234237
_LOGGER.debug(

deebot_client/messages/__init__.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from __future__ import annotations
44

5-
from functools import lru_cache
65
from typing import TYPE_CHECKING
76

87
from deebot_client.const import DataType
@@ -13,6 +12,7 @@
1312

1413
if TYPE_CHECKING:
1514
from deebot_client.message import Message
15+
from deebot_client.models import StaticDeviceInfo
1616

1717
_LOGGER = get_logger(__name__)
1818

@@ -22,15 +22,17 @@
2222
}
2323

2424

25-
@lru_cache(maxsize=256)
26-
def get_message(message_name: str, data_type: DataType) -> type[Message] | None:
25+
def get_message(
26+
message_name: str,
27+
static: StaticDeviceInfo,
28+
) -> type[Message] | None:
2729
"""Try to find the message for the given name.
2830
2931
If there exists no exact match, some conversations are performed on the name to get message object similar to the name.
3032
"""
31-
messages = MESSAGES.get(data_type)
33+
messages = MESSAGES.get(static.data_type)
3234
if messages is None:
33-
_LOGGER.warning("Datatype %s is not supported.", data_type)
35+
_LOGGER.warning("Datatype %s is not supported.", static.data_type)
3436
return None
3537

3638
if message_type := messages.get(message_name, None):
@@ -43,8 +45,10 @@ def get_message(message_name: str, data_type: DataType) -> type[Message] | None:
4345
if message_type := messages.get(converted_name, None):
4446
return message_type
4547

46-
if data_type == DataType.JSON and (
47-
found_message := get_legacy_message(message_name, converted_name)
48+
if static.data_type == DataType.JSON and (
49+
found_message := get_legacy_message(
50+
message_name, converted_name, has_map=static.capabilities.map is not None
51+
)
4852
):
4953
return found_message
5054

deebot_client/messages/json/__init__.py

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -54,37 +54,49 @@
5454

5555
MESSAGES: dict[str, type[Message]] = {message.NAME: message for message in _MESSAGES}
5656

57-
_LEGACY_USE_GET_COMMAND = [
58-
"getAdvancedMode",
59-
"getBreakPoint",
60-
"getCachedMapInfo",
61-
"getCarpertPressure",
62-
"getChargeState",
63-
"getCleanCount",
64-
"getCleanInfo",
65-
"getCleanPreference",
66-
"getEfficiency",
67-
"getError",
68-
"getLifeSpan",
69-
"getMapSet",
70-
"getMapSubSet",
71-
"getMapTrace",
72-
"getMinorMap",
73-
"getMultiMapState",
74-
"getNetInfo",
75-
"getPos",
76-
"getSpeed",
77-
"getSweepMode",
78-
"getTotalStats",
79-
"getTrueDetect",
80-
"getVoiceAssistantState",
81-
"getVolume",
82-
"getWaterInfo",
83-
"getWorkMode",
84-
]
85-
86-
87-
def get_legacy_message(message_name: str, converted_name: str) -> type[Message] | None:
57+
_MAP_LEGACY_COMMANDS = frozenset(
58+
{
59+
"getCachedMapInfo",
60+
"getMapSet",
61+
"getMapSubSet",
62+
"getMapTrace",
63+
"getMinorMap",
64+
"getMultiMapState",
65+
}
66+
)
67+
68+
_LEGACY_USE_GET_COMMAND = _MAP_LEGACY_COMMANDS | frozenset(
69+
{
70+
"getAdvancedMode",
71+
"getBreakPoint",
72+
"getCarpertPressure",
73+
"getChargeState",
74+
"getCleanCount",
75+
"getCleanInfo",
76+
"getCleanPreference",
77+
"getEfficiency",
78+
"getError",
79+
"getLifeSpan",
80+
"getNetInfo",
81+
"getPos",
82+
"getSpeed",
83+
"getSweepMode",
84+
"getTotalStats",
85+
"getTrueDetect",
86+
"getVoiceAssistantState",
87+
"getVolume",
88+
"getWaterInfo",
89+
"getWorkMode",
90+
}
91+
)
92+
93+
94+
def get_legacy_message(
95+
message_name: str,
96+
converted_name: str,
97+
*,
98+
has_map: bool = True,
99+
) -> type[Message] | None:
88100
"""Try to find the message for the given name using legacy way."""
89101
# Handle message starting with "on","off","report" the same as "get" commands
90102
converted_name = re.sub(
@@ -97,6 +109,13 @@ def get_legacy_message(message_name: str, converted_name: str) -> type[Message]
97109
_LOGGER.debug('Unknown message "%s"', message_name)
98110
return None
99111

112+
if not has_map and converted_name in _MAP_LEGACY_COMMANDS:
113+
_LOGGER.debug(
114+
'Skipping legacy map fallback for "%s" on device without map capability',
115+
message_name,
116+
)
117+
return None
118+
100119
from deebot_client.commands.json import ( # noqa: PLC0415
101120
COMMANDS,
102121
)

tests/conftest.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ def device_class() -> str:
135135
async def static_device_info(device_class: str) -> StaticDeviceInfo:
136136
info = await get_static_device_info(device_class)
137137
assert info is not None
138-
assert info.capabilities.map is not None
139138
return info
140139

141140

tests/messages/test_get_messages.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,33 @@
55
import pytest
66

77
from deebot_client.commands.json.error import GetError
8-
from deebot_client.const import DataType
98
from deebot_client.messages import get_message
109
from deebot_client.messages.json.battery import OnBattery
1110
from deebot_client.messages.json.stats import OnStats
1211

1312
if TYPE_CHECKING:
1413
from deebot_client.message import Message
14+
from deebot_client.models import StaticDeviceInfo
1515

1616

1717
@pytest.mark.parametrize(
18-
("name", "data_type", "expected"),
18+
("device_class", "name", "expected"),
1919
[
20-
("onBattery", DataType.JSON, OnBattery),
21-
("onBattery_V2", DataType.JSON, OnBattery),
22-
("onError", DataType.JSON, GetError),
23-
("onStats", DataType.JSON, OnStats),
24-
("GetCleanLogs", DataType.JSON, None),
25-
("unknown", DataType.JSON, None),
26-
("unknown", DataType.XML, None),
20+
("yna5xi", "onBattery", OnBattery),
21+
("qhe2o2", "onBattery_V2", OnBattery),
22+
("yna5xi", "onError", GetError),
23+
("yna5xi", "onStats", OnStats),
24+
("yna5xi", "GetCleanLogs", None),
25+
("yna5xi", "unknown", None),
26+
("2pv572", "unknown", None),
27+
("xmp9ds", "onMapTrace", None),
28+
("xmp9ds", "onMapSet", None),
29+
("xmp9ds", "onMinorMap", None),
30+
("xmp9ds", "onBattery", OnBattery),
2731
],
2832
)
2933
def test_get_messages(
30-
name: str, data_type: DataType, expected: type[Message] | None
34+
static_device_info: StaticDeviceInfo, name: str, expected: type[Message] | None
3135
) -> None:
3236
"""Test get messages."""
33-
assert get_message(name, data_type) == expected
37+
assert get_message(name, static_device_info) == expected

0 commit comments

Comments
 (0)