Skip to content

Commit dd7b4e7

Browse files
authored
fix: offload map parsing to worker thread to prevent event loop blocking (#966)
Offload CPU-intensive map parsing and rendering to worker threads via `asyncio.to_thread` to prevent blocking the asyncio event loop. - In `MapContentTrait.refresh()`, offload `self.converter.convert` to `asyncio.to_thread`. - In `HomeTrait.discover_home()`, move cached map parsing to a helper function `_parse_cached_home_map_content` and run it via `asyncio.to_thread`. - Avoid reparsing cached maps in `HomeTrait` when raw bytes are unchanged. Addresses home-assistant/core#182656
1 parent 7be19bf commit dd7b4e7

3 files changed

Lines changed: 83 additions & 1 deletion

File tree

‎roborock/devices/traits/v1/home.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ async def discover_home(self) -> None:
9696
self._discovery_completed = True
9797
try:
9898
self._home_map_content = {
99-
k: self._map_content.converter.parse_map_content(base64.b64decode(v))
99+
k: await self._map_content.converter.async_parse_map_content(base64.b64decode(v))
100100
for k, v in (device_cache_data.home_map_content_base64 or {}).items()
101101
}
102102
except (ValueError, RoborockException) as ex:

‎roborock/devices/traits/v1/map_content.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
"""Trait for fetching the map content from Roborock devices."""
22

3+
import asyncio
34
import logging
45
from dataclasses import dataclass
56

67
from vacuum_map_parser_base.map_data import MapData
78

89
from roborock.data import RoborockBase
910
from roborock.devices.traits.v1 import common
11+
from roborock.exceptions import RoborockParsingException
1012
from roborock.map.map_parser import MapParser, MapParserConfig
1113
from roborock.roborock_typing import RoborockCommand
1214

@@ -78,6 +80,20 @@ def parse_map_content(self, response: bytes) -> MapContent:
7880
raw_api_response=response,
7981
)
8082

83+
async def async_parse_map_content(self, response: bytes) -> MapContent:
84+
"""Parse the map content from raw bytes asynchronously.
85+
86+
Args:
87+
response: The raw bytes of the map data from the API.
88+
89+
Returns:
90+
MapContent: The parsed map content.
91+
92+
Raises:
93+
RoborockException: If the map data cannot be parsed.
94+
"""
95+
return await asyncio.to_thread(self.parse_map_content, response)
96+
8197

8298
@common.map_rpc_channel
8399
class MapContentTrait(MapContent, common.V1TraitMixin):
@@ -90,3 +106,21 @@ def __init__(self, map_parser_config: MapParserConfig | None = None) -> None:
90106
"""Initialize MapContentTrait."""
91107
super().__init__()
92108
self.converter = MapContentConverter(MapParser(map_parser_config or MapParserConfig()))
109+
110+
async def refresh(self) -> None:
111+
"""Refresh the contents of this trait without blocking the event loop."""
112+
response = await self.rpc_channel.send_command(self.command)
113+
if self.raw_api_response is not None and self.raw_api_response == response:
114+
return
115+
if not isinstance(response, bytes):
116+
raise ValueError(f"Unexpected MapContentTrait response format: {type(response)}")
117+
try:
118+
new_data = await self.converter.async_parse_map_content(response)
119+
except (TypeError, ValueError) as err:
120+
raise RoborockParsingException(
121+
trait_name=type(self).__name__,
122+
command=self.command,
123+
payload=response,
124+
inner_error=err,
125+
) from err
126+
common.merge_trait_values(self, new_data)

‎tests/devices/traits/v1/test_map_content.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,51 @@ async def test_refresh_map_content_trait(
3737
assert map_content_trait.map_data is not None
3838

3939
mock_map_rpc_channel.send_command.assert_called_once_with(RoborockCommand.GET_MAP_V1)
40+
41+
42+
async def test_refresh_unchanged_content_skips_parsing(
43+
map_content_trait: MapContentTrait,
44+
mock_map_rpc_channel: AsyncMock,
45+
) -> None:
46+
"""Test that refresh avoids re-parsing when the map content is unchanged."""
47+
map_data = b"dummy_map_bytes"
48+
mock_map_rpc_channel.send_command.return_value = map_data
49+
mock_parsed_data = ParsedMapData(
50+
image_content=b"dummy_image_content",
51+
map_data=MagicMock(),
52+
)
53+
54+
with patch("roborock.devices.traits.v1.map_content.MapParser.parse", return_value=mock_parsed_data) as mock_parse:
55+
await map_content_trait.refresh()
56+
assert mock_parse.call_count == 1
57+
assert map_content_trait.image_content == b"dummy_image_content"
58+
59+
# Second refresh with identical response should not re-parse
60+
await map_content_trait.refresh()
61+
assert mock_parse.call_count == 1
62+
63+
# Third refresh with new response parses again
64+
new_map_data = b"new_map_bytes"
65+
new_parsed_data = ParsedMapData(
66+
image_content=b"new_image_content",
67+
map_data=MagicMock(),
68+
)
69+
mock_map_rpc_channel.send_command.return_value = new_map_data
70+
mock_parse.return_value = new_parsed_data
71+
await map_content_trait.refresh()
72+
assert mock_parse.call_count == 2
73+
assert map_content_trait.image_content == b"new_image_content"
74+
75+
76+
async def test_async_parse_map_content(map_content_trait: MapContentTrait) -> None:
77+
"""Test async_parse_map_content on MapContentConverter."""
78+
map_data = b"dummy_map_bytes"
79+
mock_parsed_data = ParsedMapData(
80+
image_content=b"dummy_image_content",
81+
map_data=MagicMock(),
82+
)
83+
with patch("roborock.devices.traits.v1.map_content.MapParser.parse", return_value=mock_parsed_data) as mock_parse:
84+
result = await map_content_trait.converter.async_parse_map_content(map_data)
85+
mock_parse.assert_called_once_with(map_data)
86+
assert result.image_content == b"dummy_image_content"
87+
assert result.raw_api_response == map_data

0 commit comments

Comments
 (0)