Add Category enum and mixin for NACamDoorTag#553
Add Category enum and mixin for NACamDoorTag#553farkasdi wants to merge 4 commits intojabesq-org:developmentfrom
Conversation
Reviewer's GuideIntroduce a Category enum and CategoryMixin to expose the NACamDoorTag module's category field (door, window, etc.) as a first-class feature and wire the mixin into the NACamDoorTag device type. Class diagram for new Category enum and CategoryMixin integration into NACamDoorTagclassDiagram
class Category {
<<enumeration>>
door
furniture
garage
gate
other
window
unknown
+_missing_(key: object) Category
}
class EntityBase {
}
class CategoryMixin {
+category: Category | None
+__init__(home: Home, module: ModuleT)
}
class Module {
}
class StatusMixin {
}
class FirmwareMixin {
}
class BatteryMixin {
}
class RfMixin {
}
class NACamDoorTag {
}
CategoryMixin --|> EntityBase
NACamDoorTag --|> Module
NACamDoorTag --|> StatusMixin
NACamDoorTag --|> FirmwareMixin
NACamDoorTag --|> BatteryMixin
NACamDoorTag --|> RfMixin
NACamDoorTag --|> CategoryMixin
CategoryMixin --> Category
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
CategoryMixin.__init__you assign toself.appliance_typeinstead ofself.category, which makes the mixin inconsistent with its own type annotation and the newcategoryfeature; this likely should beself.category = module.get("category", Category.unknown). - Consider aligning naming between
CategoryMixinand theCategoryenum docstring: the mixin is generic but the enum is documented as specific to NACamDoorTag/Home+Security, so either narrow the mixin’s scope or generalize the docstring to avoid confusion about intended usage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CategoryMixin.__init__` you assign to `self.appliance_type` instead of `self.category`, which makes the mixin inconsistent with its own type annotation and the new `category` feature; this likely should be `self.category = module.get("category", Category.unknown)`.
- Consider aligning naming between `CategoryMixin` and the `Category` enum docstring: the mixin is generic but the enum is documented as specific to NACamDoorTag/Home+Security, so either narrow the mixin’s scope or generalize the docstring to avoid confusion about intended usage.
## Individual Comments
### Comment 1
<location path="src/pyatmo/modules/module.py" line_range="324-330" />
<code_context>
+class CategoryMixin(EntityBase):
+ """Mixin for category data."""
+
+ category: Category | None
+
+ def __init__(self, home: Home, module: ModuleT) -> None:
+ """Initialize category mixin."""
+
+ super().__init__(home, module)
+ self.appliance_type: Category | None = module.get(
+ "category",
+ Category.unknown,
</code_context>
<issue_to_address>
**issue (bug_risk):** Mismatch between declared `category` attribute and assigned `appliance_type` plus inconsistent optional typing.
`CategoryMixin` defines `category: Category | None`, but `__init__` sets `self.appliance_type: Category | None = module.get("category", Category.unknown)`. This likely leaves `self.category` unset and introduces an unintended `appliance_type` attribute. Also, the type allows `None` but the default is `Category.unknown`, so `None` is never stored. Please align the attribute name and type, e.g.:
```python
self.category: Category = Category(module.get("category", Category.unknown))
```
(or make the annotation non-optional) so naming, typing, and the stored value are consistent.
</issue_to_address>
### Comment 2
<location path="src/pyatmo/modules/module.py" line_range="330-332" />
<code_context>
+ """Initialize category mixin."""
+
+ super().__init__(home, module)
+ self.appliance_type: Category | None = module.get(
+ "category",
+ Category.unknown,
+ )
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Raw category value is stored without validating or converting it to the `Category` enum.
`module.get("category", Category.unknown)` is assigned directly to a `Category | None`, but `module` likely stores raw strings. This can leave `appliance_type` as a plain string instead of a `Category` member, breaking enum comparisons later. Convert the value via `Category(...)` (handling `ValueError` as needed) so the mixin always exposes a valid enum instance.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
2026-03-11 10:31:33.688 DEBUG (MainThread) [homeassistant.components.netatmo.binary_sensor] Netatmo device: {'entity_id': 'XX:XX:XX:XX:XX:XX', 'name': 'Minibar', 'history_features_values': {}, 'history_features': set(), 'device_type': <DeviceType.NACamDoorTag: 'NACamDoorTag'>, 'home': <pyatmo.home.Home object at 0xXXXXXXXXXX>, 'room_id': 'XXXXXXXXX', 'reachable': True, 'bridge': 'XX:XX:XX:XX:XX:XX', 'modules': None, 'device_category': <DeviceCategory.opening: 'opening'>, 'features': {'status', 'rf_strength', 'battery', 'reachable', 'doortag_category'}, 'doortag_category': <DoorTagCategory.furniture: 'furniture'>, 'rf_strength': 55, 'battery_state': 'high', 'battery_level': 5420, 'battery_percent': None, 'firmware_revision': 58, 'firmware_name': None, 'status': 'closed'} |
For
NACamDoorTagthere is acategorythat describes what type of opening the doortag is used. The change is proposing adoortag_categoryamong the devicefeature-s to expose this.Summary by Sourcery
Introduce a category concept for NACamDoorTag modules and expose it via the module feature set.
New Features: