Skip to content

feat: log when we see a new key we have never seen before #658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Lash-L merged 1 commit into from
Dec 10, 2025
Merged

feat: log when we see a new key we have never seen before #658

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion roborock/data/containers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from dataclasses import asdict, dataclass, field
from enum import Enum
from functools import cached_property
from typing import Any, NamedTuple, get_args, get_origin
from typing import Any, ClassVar, NamedTuple, get_args, get_origin

from .code_mappings import (
SHORT_MODEL_TO_ENUM,
Expand Down Expand Up @@ -64,6 +64,8 @@ def _attr_repr(obj: Any) -> str:
class RoborockBase:
"""Base class for all Roborock data classes."""

_missing_logged: ClassVar[set[str]] = set()
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The shared class variable _missing_logged is not thread-safe. If from_dict is called concurrently from multiple threads, race conditions could occur when checking membership and adding to the set. Consider using a threading.Lock or a thread-safe alternative like threading.RLock() to protect access to this shared state, or use a thread-safe data structure.

Copilot uses AI. Check for mistakes.

@staticmethod
def _convert_to_class_obj(class_type: type, value):
if get_origin(class_type) is list:
Expand Down Expand Up @@ -93,6 +95,14 @@ def from_dict(cls, data: dict[str, Any]):
for orig_key, value in data.items():
key = _decamelize(orig_key)
if (field_type := field_types.get(key)) is None:
if (log_key := f"{cls.__name__}.{key}") not in RoborockBase._missing_logged:
_LOGGER.debug(
"Key '%s' (decamelized: '%s') not found in %s fields, skipping",
orig_key,
key,
cls.__name__,
)
RoborockBase._missing_logged.add(log_key)
Comment on lines +98 to +105
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The new logging behavior for unknown keys lacks test coverage. Consider adding a test that verifies: (1) a debug log is emitted when an unknown key is encountered for the first time, (2) the log is not emitted again for subsequent occurrences of the same key in the same class, and (3) the class name and both original and decamelized keys are included in the log message. The existing test_ignore_unknown_keys test could be extended to verify this behavior using a mock logger or caplog fixture.

Copilot uses AI. Check for mistakes.
continue
if value == "None" or value is None:
result[key] = None
Expand Down