Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@ docs/_build/

# GitHub App credentials
gha-creds-*.json

# pickle files
*.p
*.pickle
Comment on lines +22 to +25
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated but i have a few of these in my repo and just wanted to stop them from filling up my git diff

81 changes: 78 additions & 3 deletions roborock/devices/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,22 @@

import asyncio
import datetime
import json
import logging
from abc import ABC
from collections.abc import Callable
from typing import Any

from roborock.callbacks import CallbackList
from roborock.data import HomeDataDevice, HomeDataProduct
from roborock.data import HomeDataDevice, HomeDataProduct, RoborockErrorCode, RoborockStateCode
from roborock.diagnostics import redact_device_data
from roborock.exceptions import RoborockException
from roborock.roborock_message import RoborockMessage
from roborock.roborock_message import (
ROBOROCK_DATA_STATUS_PROTOCOL,
RoborockDataProtocol,
RoborockMessage,
RoborockMessageProtocol,
)
from roborock.util import RoborockLoggerAdapter

from .traits import Trait
Expand Down Expand Up @@ -219,8 +225,77 @@ async def close(self) -> None:
self._unsub = None

def _on_message(self, message: RoborockMessage) -> None:
"""Handle incoming messages from the device."""
"""Handle incoming messages from the device.

Note: Protocol updates (data points) are only sent via cloud/MQTT, not local connection.
"""
self._logger.debug("Received message from device: %s", message)
if self.v1_properties is None:
# Ensure we are only doing below logic for set-up V1 devices.
return

# Only process messages that can contain protocol updates
# RPC_RESPONSE (102), GENERAL_REQUEST (4), and GENERAL_RESPONSE (5)
if message.protocol not in {
RoborockMessageProtocol.RPC_RESPONSE,
RoborockMessageProtocol.GENERAL_RESPONSE,
}:
return

if not message.payload:
return

try:
payload = json.loads(message.payload.decode())
dps = payload.get("dps", {})

if not dps:
return

# Process each data point in the message
for data_point_number, data_point in dps.items():
# Skip RPC responses (102) as they're handled by the RPC channel
if data_point_number == "102":
continue

try:
data_protocol = RoborockDataProtocol(int(data_point_number))
self._logger.debug(f"Got device update for {data_protocol.name}: {data_point}")
self._handle_protocol_update(data_protocol, data_point)
except ValueError:
# Unknown protocol number
self._logger.debug(
f"Got unknown data protocol {data_point_number}, data: {data_point}. "
f"This may allow for faster updates in the future."
)
except (json.JSONDecodeError, UnicodeDecodeError, KeyError) as ex:
self._logger.debug(f"Failed to parse protocol message: {ex}")

def _handle_protocol_update(self, protocol: RoborockDataProtocol, data_point: Any) -> None:
"""Handle a protocol update for a specific data protocol.

Args:
protocol: The data protocol number.
data_point: The data value for this protocol.
"""
# Handle status protocol updates
if protocol in ROBOROCK_DATA_STATUS_PROTOCOL and self.v1_properties and self.v1_properties.status:
# Update the specific field in the status trait
match protocol:
case RoborockDataProtocol.ERROR_CODE:
self.v1_properties.status.error_code = RoborockErrorCode(data_point)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the decision about which field in the trait to update should live inside the specific trait. I think we should pass in the protocol and data point and let it decide to update or not or which field to update.

case RoborockDataProtocol.STATE:
self.v1_properties.status.state = RoborockStateCode(data_point)
case RoborockDataProtocol.BATTERY:
self.v1_properties.status.battery = data_point
case RoborockDataProtocol.CHARGE_STATUS:
self.v1_properties.status.charge_status = data_point
case _:
# There is also fan power and water box mode, but for now those are skipped
return
Comment on lines +284 to +295
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The match statement only handles ERROR_CODE, STATE, BATTERY, and CHARGE_STATUS protocols, but ROBOROCK_DATA_STATUS_PROTOCOL also includes FAN_POWER and WATER_BOX_MODE (as mentioned in the comment on line 294). These two protocols will match the condition on line 282 but then fall through to the default case without updating the trait or calling notify_update. This creates inconsistent behavior where some status protocols trigger updates while others silently do nothing. Either handle these protocols explicitly or exclude them from the condition on line 282.

Copilot uses AI. Check for mistakes.

self._logger.debug("Updated status.%s to %s", protocol.name.lower(), data_point)
self.v1_properties.status.notify_update()
Comment on lines +237 to +298
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The protocol update parsing logic introduced in _on_message and _handle_protocol_update lacks test coverage. Given that this repository has comprehensive automated testing, tests should be added to verify that protocol messages are correctly parsed and that the appropriate trait fields are updated when data points are received.

Copilot uses AI. Check for mistakes.

def diagnostic_data(self) -> dict[str, Any]:
"""Return diagnostics information about the device."""
Expand Down
14 changes: 8 additions & 6 deletions roborock/devices/rpc/v1_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,14 @@ async def subscribe(self, callback: Callable[[RoborockMessage], None]) -> Callab
loop = asyncio.get_running_loop()
self._reconnect_task = loop.create_task(self._background_reconnect())

if not self.is_local_connected:
# We were not able to connect locally, so fallback to MQTT and at least
# establish that connection explicitly. If this fails then raise an
# error and let the caller know we failed to subscribe.
self._mqtt_unsub = await self._mqtt_channel.subscribe(self._on_mqtt_message)
self._logger.debug("V1Channel connected to device via MQTT")
# Always subscribe to MQTT to receive protocol updates (data points)
# even if we have a local connection. Protocol updates only come via cloud/MQTT.
# Local connection is used for RPC commands, but push notifications come via MQTT.
self._mqtt_unsub = await self._mqtt_channel.subscribe(self._on_mqtt_message)
if self.is_local_connected:
self._logger.debug("V1Channel connected via local and MQTT (for protocol updates)")
else:
self._logger.debug("V1Channel connected via MQTT only")

def unsub() -> None:
"""Unsubscribe from all messages."""
Expand Down
21 changes: 21 additions & 0 deletions roborock/devices/traits/v1/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,23 @@
This is an internal library and should not be used directly by consumers.
"""

from __future__ import annotations

import logging
from abc import ABC, abstractmethod
from collections.abc import Callable
from dataclasses import dataclass, fields
from typing import ClassVar, Self

from roborock.callbacks import CallbackList
from roborock.data import RoborockBase
from roborock.protocols.v1_protocol import V1RpcChannel
from roborock.roborock_typing import RoborockCommand

_LOGGER = logging.getLogger(__name__)

V1ResponseData = dict | list | int | str
V1TraitUpdateCallback = Callable[["V1TraitMixin"], None]


@dataclass
Expand Down Expand Up @@ -74,6 +79,7 @@ def __post_init__(self) -> None:
device setup code.
"""
self._rpc_channel = None
self._update_callbacks: CallbackList[V1TraitMixin] = CallbackList()

@property
def rpc_channel(self) -> V1RpcChannel:
Expand All @@ -97,6 +103,21 @@ def _update_trait_values(self, new_data: RoborockBase) -> None:
new_value = getattr(new_data, field.name, None)
setattr(self, field.name, new_value)

def add_update_callback(self, callback: V1TraitUpdateCallback) -> Callable[[], None]:
"""Add a callback to be notified when the trait is updated.

The callback will be called with the updated trait instance whenever
a protocol message updates the trait.

Returns:
A callable that can be used to remove the callback.
"""
return self._update_callbacks.add_callback(callback)

def notify_update(self) -> None:
"""Notify all registered callbacks that the trait has been updated."""
self._update_callbacks(self)
Comment on lines +106 to +119
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new callback functionality (add_update_callback and notify_update methods) lacks test coverage. Given that this repository has comprehensive automated testing, tests should be added to verify that callbacks are properly invoked when traits are updated via protocol messages.

Copilot uses AI. Check for mistakes.


def _get_value_field(clazz: type[V1TraitMixin]) -> str:
"""Get the name of the field marked as the main value of the RoborockValueBase."""
Expand Down
Loading