Skip to content

Conversation

@dirixmjm
Copy link
Contributor

@dirixmjm dirixmjm commented Aug 31, 2025

Summary by CodeRabbit

  • New Features

    • Per-parameter persistent sense hysteresis for humidity and temperature (enable/disable, upper/lower bounds, direction) exposed in node state and applied on device wake.
  • Improvements

    • Hysteresis states included in sensor statistics; live switch-group updates and stricter input validation; streamlined motion sensor configuration flow.
  • Tests

    • New tests covering sense hysteresis validation, restore and apply flows.
  • Chores

    • Package version bumped to 0.45.0 and changelog updated.

@coderabbitai
Copy link

coderabbitai bot commented Aug 31, 2025

Walkthrough

Adds per-parameter sense hysteresis (API enum/dataclass and Sense node state), cache persistence and validation, awake-time hysteresis configuration and switch‑group handling, a new SenseConfigureHysteresisRequest, firmware feature mapping, an inlined scan configure flow, tests exercising hysteresis, and a version bump.

Changes

Cohort / File(s) Summary
API additions
plugwise_usb/api.py
Added NodeFeature.SENSE_HYSTERESIS; new frozen SenseHysteresisConfig dataclass; extended SenseStatistics with temperature_hysteresis_state and humidity_hysteresis_state.
Message request: Sense hysteresis
plugwise_usb/messages/requests.py
Added SenseConfigureHysteresisRequest (_identifier=b"0104", _reply_identifier=b"0100"), argument encoding and async send() returning `NodeAckResponse
Firmware feature map
plugwise_usb/nodes/helpers/firmware.py
Added mapping NodeFeature.SENSE_HYSTERESIS: 2.0 to FEATURE_SUPPORTED_AT_FIRMWARE.
Scan flow refactor
plugwise_usb/nodes/scan.py
Inlined previous scan_configure() into _configure_scan_task; removed public scan_configure(); get_state now returns computed motion_config property.
Sense hysteresis integration
plugwise_usb/nodes/sense.py
Added cache keys/defaults and SENSE_FEATURES update; cached SenseHysteresisConfig with dirty flag; getters/setters with validation and persistence; awake-time config via SenseConfigureHysteresisRequest; switch-group subscription/handling; state publishing, cache sync, @raise_not_loaded on sense_statistics, cleanup on unload; get_state returns hysteresis config.
Tests
tests/test_usb.py
Added async tests exercising Sense hysteresis load, validation, dirty-flag behavior, awake-time config cycles and persistence (duplicate test_sense_node present).
Packaging
pyproject.toml
Bumped project version from 0.44.14 to 0.45.0.
Changelog
CHANGELOG.md
Added top-level entry for 0.45.0 describing sense hysteresis addition.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as Caller
  participant Sense as PlugwiseSense
  participant Cache as Cache
  participant Dev as Device
  Note over Sense,Cache: Load-time restore
  Sense->>Cache: read hysteresis values (+dirty)
  Cache-->>Sense: values or defaults
  App->>Sense: set_hysteresis_*(...)
  Sense->>Sense: validate, set fields, dirty=True, publish update
  Note over Sense,Dev: Awake-time configuration (when awake & dirty)
  Sense->>Dev: SenseConfigureHysteresisRequest(humidity)
  Dev-->>Sense: NodeAckResponse / None / failure
  alt Ack accepted
    Sense->>Dev: SenseConfigureHysteresisRequest(temperature)
    Dev-->>Sense: NodeAckResponse
    Sense->>Cache: persist values, dirty=False
    Sense->>App: publish updated state
  else Ack failed / None
    Sense->>Sense: keep dirty=True
  end
Loading
sequenceDiagram
  autonumber
  participant Scan as PlugwiseScan
  participant Dev as Device
  participant Log as Logger
  Note over Scan: _configure_scan_task (inlined)
  alt motion_config.dirty
    Scan->>Dev: ScanConfigureRequest(reset_timer, sensitivity, daylight)
    Dev-->>Scan: NodeAckResponse or None
    alt None or SCAN_CONFIG_FAILED
      Scan->>Log: warn
      Scan-->>Scan: return False
    else SCAN_CONFIG_ACCEPTED
      Scan->>Scan: mark motion_config.dirty=False
      Scan->>Scan: _scan_configure_update()
      Scan-->>Scan: return True
    else other ack
      Scan->>Log: warn
      Scan-->>Scan: return False
    end
  else not dirty
    Scan-->>Scan: return True
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • bouwew
  • brefra
  • ArnoutD

Poem

Thump-thump, I tweak each bound with care,
I whisper 0x0104 into the air.
If ACKs reply I shake the dirt away,
I stash my carrots, cache them safe today.
Hops, sensors, and a tidy little sway. 🥕


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4cbe6c0 and f1abce9.

📒 Files selected for processing (1)
  • plugwise_usb/messages/requests.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugwise_usb/messages/requests.py
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mdi_sense

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Aug 31, 2025

Codecov Report

❌ Patch coverage is 87.35632% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.30%. Comparing base (106a731) to head (f1abce9).
⚠️ Report is 34 commits behind head on main.

Files with missing lines Patch % Lines
plugwise_usb/nodes/sense.py 79.79% 58 Missing ⚠️
tests/test_usb.py 97.52% 5 Missing ⚠️
plugwise_usb/messages/requests.py 83.33% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
+ Coverage   80.70%   81.30%   +0.60%     
==========================================
  Files          36       36              
  Lines        7560     8063     +503     
==========================================
+ Hits         6101     6556     +455     
- Misses       1459     1507      +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🧹 Nitpick comments (7)
plugwise_usb/messages/requests.py (2)

1422-1432: Docstring: wrong class name.

“Initialize ScanConfigureRequest message object.” should reference SenseConfigureHysteresisRequest.

-        """Initialize ScanConfigureRequest message object."""
+        """Initialize SenseConfigureHysteresisRequest message object."""

1422-1430: Validate argument ranges (defensive).

Bounds should be 0..0xFFFF as they serialize to 4 hex chars. Consider validating here to fail fast if an upstream conversion bug slips in.

     def __init__(  # noqa: PLR0913
         self,
         send_fn: Callable[[PlugwiseRequest, bool], Awaitable[PlugwiseResponse | None]],
         mac: bytes,
         temp_hum: bool,
         lower_bound: int,
         upper_bound: int,
         direction: bool,
     ):
+        if not (0 <= lower_bound <= 0xFFFF and 0 <= upper_bound <= 0xFFFF):
+            raise MessageError(
+                f"Bounds out of range: lower={lower_bound}, upper={upper_bound} (expected 0..65535)"
+            )
plugwise_usb/api.py (1)

264-293: Doc mismatch: describe bounds as float, not int.

The docstring says “int” for bounds, but the fields are typed as float. Align the doc/comments.

-        humidity_upper_bound: int | None: upper humidity switching value
-        humidity_lower_bound: int | None: lower humidity switching value
+        humidity_upper_bound: float | None: upper humidity switching value
+        humidity_lower_bound: float | None: lower humidity switching value
-        temperature_upper_bound: int | None: upper temperature switching value
-        temperature_lower_bound: int | None: lower temperature switching value
+        temperature_upper_bound: float | None: upper temperature switching value
+        temperature_lower_bound: float | None: lower temperature switching value
plugwise_usb/nodes/sense.py (4)

85-87: Docstring: wrong device name.

“Initialize Scan Device.” → “Initialize Sense Device.”

-        """Initialize Scan Device."""
+        """Initialize Sense Device."""

527-547: Typo in docstring and minor clarity.

“humitidy” → “temperature”. Consider clarifying “switch on when increasing” vs “active-high”.

-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+        """Configure temperature hysteresis direction (switch on when increasing vs decreasing).

55-73: Sanity-check defaults.

Upper and lower defaults equal (no hysteresis). If intentional, OK; otherwise consider small deltas.


566-583: State update: publish SENSE_HYSTERESIS as well when switch state changes.

Today it publishes NodeFeature.SENSE only. If consumers rely on hysteresis state, consider publishing both.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b7176ce and 06ae206.

📒 Files selected for processing (5)
  • plugwise_usb/api.py (2 hunks)
  • plugwise_usb/messages/requests.py (1 hunks)
  • plugwise_usb/nodes/helpers/firmware.py (1 hunks)
  • plugwise_usb/nodes/scan.py (1 hunks)
  • plugwise_usb/nodes/sense.py (8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-07T17:50:29.188Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#302
File: plugwise_usb/nodes/scan.py:132-142
Timestamp: 2025-08-07T17:50:29.188Z
Learning: In the plugwise_usb codebase, the `_load_defaults` method in node classes (scan.py, switch.py, sense.py) is only called once during node initialization, not during runtime. Therefore, flags like `_sed_node_info_update_task_scheduled` don't need to be reset at the start of `_load_defaults` as there's no risk of stale state from previous loads.

Applied to files:

  • plugwise_usb/nodes/scan.py
🧬 Code graph analysis (5)
plugwise_usb/nodes/helpers/firmware.py (1)
plugwise_usb/api.py (1)
  • NodeFeature (39-59)
plugwise_usb/messages/requests.py (6)
plugwise_usb/messages/responses.py (2)
  • PlugwiseResponse (98-215)
  • NodeAckResponse (902-924)
plugwise_usb/messages/__init__.py (1)
  • mac (53-57)
plugwise_usb/messages/properties.py (1)
  • Int (103-129)
tests/test_usb.py (1)
  • send (250-256)
plugwise_usb/connection/__init__.py (1)
  • send (227-238)
plugwise_usb/exceptions.py (1)
  • MessageError (20-21)
plugwise_usb/api.py (3)
plugwise_usb/nodes/sense.py (8)
  • humidity_enabled (289-293)
  • humidity_upper_bound (296-300)
  • humidity_lower_bound (303-307)
  • humidity_direction (310-314)
  • temperature_enabled (317-321)
  • temperature_upper_bound (324-328)
  • temperature_lower_bound (331-335)
  • temperature_direction (338-342)
plugwise_usb/nodes/scan.py (1)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (1)
  • dirty (336-338)
plugwise_usb/nodes/scan.py (2)
plugwise_usb/api.py (2)
  • NodeFeature (39-59)
  • motion_config (533-543)
plugwise_usb/nodes/node.py (1)
  • motion_config (249-255)
plugwise_usb/nodes/sense.py (1)
plugwise_usb/api.py (3)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-303)
🔇 Additional comments (3)
plugwise_usb/api.py (1)

58-58: LGTM: New NodeFeature.

SENSE_HYSTERESIS feature addition looks consistent with usages.

plugwise_usb/nodes/helpers/firmware.py (1)

162-163: LGTM: Firmware feature map updated.

Mapping SENSE_HYSTERESIS to 2.0 aligns with other Sense capabilities.

plugwise_usb/nodes/scan.py (1)

537-538: LGTM: Return public MotionConfig.

Returning self.motion_config makes the state consistent with other nodes’ public properties.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (3)
plugwise_usb/nodes/sense.py (3)

562-576: Runtime error: wrong call signature to _hysteresis_state_update.

Missing switch_group argument causes a TypeError at runtime.

         await gather(
             self._available_update_state(True, response.timestamp),
-            self._hysteresis_state_update(response.switch_state, response.timestamp),
+            self._hysteresis_state_update(
+                response.switch_state,
+                response.switch_group,
+                response.timestamp,
+            ),
         )

625-636: Bug: ‘dirty’ cleared on failure instead of success.

Only clear dirty after both configure tasks succeed.

-        if self._hysteresis_config.dirty:
-            configure_result = await gather(
+        if self._hysteresis_config.dirty:
+            configure_result = await gather(
                 self._configure_sense_humidity_task(),
                 self._configure_sense_temperature_task(),
             )
-            if not all(configure_result):
-                self._hysteresis_config = replace(self._hysteresis_config, dirty=False)
-                await self._sense_configure_update()
+            if all(configure_result):
+                self._hysteresis_config = replace(self._hysteresis_config, dirty=False)
+                await self._sense_configure_update()

691-744: Temperature configure: wrong temp_hum flag and f-string issue.

Must pass True for temperature; also fix message formatting.

-            if self.temperature_lower_bound > self.temperature_upper_bound:
-                raise ValueError(
-                    "Invalid temperature lower bound {self.temperature_lower_bound}. It must be equal or below the upper bound {self.temperature_upper_bound}."
-                )
+            if self.temperature_lower_bound > self.temperature_upper_bound:
+                raise ValueError(
+                    f"Invalid temperature lower bound {self.temperature_lower_bound}. It must be ≤ upper bound {self.temperature_upper_bound}."
+                )
@@
-        request = SenseConfigureHysteresisRequest(
+        request = SenseConfigureHysteresisRequest(
             self._send,
             self._mac_in_bytes,
-            False,
+            True,  # temperature
             temperature_lower_bound,
             temperature_upper_bound,
             self.temperature_direction,
         )
🧹 Nitpick comments (4)
plugwise_usb/nodes/sense.py (4)

401-432: Fix incorrect error message text for lower-bound validation.

Message should reference “≤ upper bound …”.

-            raise ValueError(
-                f"Invalid humidity lower bound {lower_bound}. It must be equal or above the lower bound {self._hysteresis_config.humidity_lower_bound}."
-            )
+            raise ValueError(
+                f"Invalid humidity lower bound {lower_bound}. It must be ≤ upper bound {self._hysteresis_config.humidity_upper_bound}."
+            )

433-453: Typos and clarity in docstring.

Fix “humitidy” and clarify wording.

-    async def set_hysteresis_humidity_direction(self, state: bool) -> bool:
-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+    async def set_hysteresis_humidity_direction(self, state: bool) -> bool:
+        """Configure humidity hysteresis: switch on/off on increasing/decreasing trend.

475-506: Unit wording: use °C for temperature messages.

Minor UX improvement.

-                f"Invalid temperature upper bound {upper_bound}. It must be between 1 and 60 degrees."
+                f"Invalid temperature upper bound {upper_bound}. It must be between 1 and 60 °C."
-                f"Invalid temperature lower bound {lower_bound}. It must be between 1 and 60 degrees."
+                f"Invalid temperature lower bound {lower_bound}. It must be between 1 and 60 °C."

Also applies to: 507-538


539-559: Typos and target in docstring.

Should say “temperature”, not “humitidy”.

-    async def set_hysteresis_temperature_direction(self, state: bool) -> bool:
-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+    async def set_hysteresis_temperature_direction(self, state: bool) -> bool:
+        """Configure temperature hysteresis: switch on/off on increasing/decreasing trend.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 06ae206 and f24d5c8.

📒 Files selected for processing (3)
  • plugwise_usb/api.py (2 hunks)
  • plugwise_usb/messages/requests.py (1 hunks)
  • plugwise_usb/nodes/sense.py (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • plugwise_usb/messages/requests.py
  • plugwise_usb/api.py
🧰 Additional context used
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (7)
plugwise_usb/api.py (5)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • unload (333-334)
  • name (322-323)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (6)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • _load_from_cache (144-173)
  • dirty (229-231)
  • _run_awake_tasks (424-434)
plugwise_usb/nodes/node.py (7)
  • initialize (412-420)
  • unload (644-651)
  • _load_from_cache (396-410)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
plugwise_usb/nodes/switch.py (2)
  • initialize (61-71)
  • unload (73-77)
plugwise_usb/nodes/sed.py (5)
  • initialize (127-137)
  • unload (117-125)
  • _load_from_cache (142-176)
  • dirty (336-338)
  • _run_awake_tasks (540-550)
🔇 Additional comments (2)
plugwise_usb/nodes/sense.py (2)

116-120: Subscription to switch-group looks correct.

Initializing and storing the unsubscribe handle is consistent with other nodes.


578-595: Verify switch_group mapping in firmware
In sense.py (lines 578–595) the code treats group 1 → temperature_state and group 2 → humidity_state; confirm these values match the official firmware specification to avoid flipped signals.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (7)
plugwise_usb/nodes/sense.py (7)

649-652: Format error message correctly (f-string) and use ≤ wording.
[ suggest_nitpick ]

-            if self.humidity_lower_bound > self.humidity_upper_bound:
-                raise ValueError(
-                    "Invalid humidity lower bound {self.humidity_lower_bound}. It must be equal or below the upper bound {self.humidity_upper_bound}."
-                )
+            if self.humidity_lower_bound > self.humidity_upper_bound:
+                raise ValueError(
+                    f"Invalid humidity lower bound {self.humidity_lower_bound}. It must be ≤ upper bound {self.humidity_upper_bound}."
+                )

703-706: Format error message correctly (f-string) and use ≤ wording.

-            if self.temperature_lower_bound > self.temperature_upper_bound:
-                raise ValueError(
-                    "Invalid temperature lower bound {self.temperature_lower_bound}. It must be equal or below the upper bound {self.temperature_upper_bound}."
-                )
+            if self.temperature_lower_bound > self.temperature_upper_bound:
+                raise ValueError(
+                    f"Invalid temperature lower bound {self.temperature_lower_bound}. It must be ≤ upper bound {self.temperature_upper_bound}."
+                )

416-422: Fix message: compare against upper bound, not lower.

The condition checks > upper_bound; message mentions “equal or above the lower bound”.
[ suggest_nitpick ]

-            raise ValueError(
-                f"Invalid humidity lower bound {lower_bound}. It must be equal or above the lower bound {self._hysteresis_config.humidity_lower_bound}."
-            )
+            raise ValueError(
+                f"Invalid humidity lower bound {lower_bound}. It must be ≤ upper bound {self._hysteresis_config.humidity_upper_bound}."
+            )

486-489: Use correct unit (°C) in temperature validation messages.

-                f"Invalid temperature upper bound {upper_bound}. It must be between 1 and 60 degrees."
+                f"Invalid temperature upper bound {upper_bound}. It must be between 1 and 60 °C."
-                f"Invalid temperature lower bound {lower_bound}. It must be between 1 and 60 degrees."
+                f"Invalid temperature lower bound {lower_bound}. It must be between 1 and 60 °C."

Also applies to: 518-521


273-286: Bug: returns non-existent self.dirty; use config’s flag.

This will raise AttributeError and break state reads.

         return SenseHysteresisConfig(
           humidity_enabled=self.humidity_enabled,
           humidity_upper_bound=self.humidity_upper_bound,
           humidity_lower_bound=self.humidity_lower_bound,
           humidity_direction=self.humidity_direction,
           temperature_enabled=self.temperature_enabled,
           temperature_upper_bound=self.temperature_upper_bound,
           temperature_lower_bound=self.temperature_lower_bound,
           temperature_direction=self.temperature_direction,
-          dirty=self.dirty,
+          dirty=self._hysteresis_config.dirty,
         )

717-724: Temperature request uses wrong temp_hum flag (should be True).

This configures humidity instead of temperature.

         request = SenseConfigureHysteresisRequest(
             self._send,
             self._mac_in_bytes,
-            False,
+            True,
             temperature_lower_bound,
             temperature_upper_bound,
             self.temperature_direction,
         )

773-773: Bug: caching wrong attribute; self.dirty is undefined.

Write the config’s dirty flag.

-        self._set_cache(CACHE_SENSE_HYSTERESIS_DIRTY, self.dirty)
+        self._set_cache(CACHE_SENSE_HYSTERESIS_DIRTY, self._hysteresis_config.dirty)
🧹 Nitpick comments (5)
plugwise_usb/nodes/sense.py (5)

184-185: Prefer logical OR over bitwise for booleans.

Improves clarity; behavior is identical.

-        dirty |= self._sense_hysteresis_dirty_from_cache()
+        dirty = dirty or self._sense_hysteresis_dirty_from_cache()

433-437: Fix typos and clarify docstrings.

“humitidy” → “humidity”; temperature method docstring references humidity.

-    async def set_hysteresis_humidity_direction(self, state: bool) -> bool:
-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+    async def set_hysteresis_humidity_direction(self, state: bool) -> bool:
+        """Configure humidity hysteresis to switch on or off on increasing or decreasing values.
-    async def set_hysteresis_temperature_direction(self, state: bool) -> bool:
-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+    async def set_hysteresis_temperature_direction(self, state: bool) -> bool:
+        """Configure temperature hysteresis to switch on or off on increasing or decreasing values.

Also applies to: 539-543


629-636: Consider sending config sequentially to avoid transport interleaving.

If StickController isn’t fully re-entrant, two concurrent 0104 requests may race. Sequential awaits reduce risk.

-            configure_result = await gather(
-                self._configure_sense_humidity_task(),
-                self._configure_sense_temperature_task(),
-            )
+            humidity_ok = await self._configure_sense_humidity_task()
+            temperature_ok = await self._configure_sense_temperature_task()
+            configure_result = (humidity_ok, temperature_ok)

591-595: Verify group-to-signal mapping (1=temperature, 2=humidity).

Confirm with protocol docs/fixtures; if reversed, UI states will be swapped. Add an else branch to log unexpected groups.


55-64: Optional: prefix cache keys to avoid collisions.

Bare names like "humidity_enabled" risk overlap with other features. Consider namespacing (e.g., "sense_hyst_humidity_enabled") with fallback loading for migration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f24d5c8 and e4b4fb2.

📒 Files selected for processing (1)
  • plugwise_usb/nodes/sense.py (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (8)
plugwise_usb/api.py (6)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • unload (333-334)
  • name (322-323)
  • save_cache (402-405)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (6)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • _load_from_cache (144-173)
  • dirty (229-231)
  • _run_awake_tasks (424-434)
plugwise_usb/nodes/node.py (9)
  • initialize (412-420)
  • unload (644-651)
  • _load_from_cache (396-410)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/sed.py (5)
  • initialize (127-137)
  • unload (117-125)
  • _load_from_cache (142-176)
  • dirty (336-338)
  • _run_awake_tasks (540-550)
plugwise_usb/nodes/switch.py (2)
  • initialize (61-71)
  • unload (73-77)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
plugwise_usb/nodes/sense.py (2)

731-738: Bug: temperature config sends humidity flag (temp_hum=False) – device misconfiguration

When configuring temperature hysteresis you must pass temp_hum=True. With False the device treats it as a humidity config.

         request = SenseConfigureHysteresisRequest(
             self._send,
             self._mac_in_bytes,
-            False,
+            True,  # temperature
             temperature_lower_bound,
             temperature_upper_bound,
             self.temperature_direction,
         )

423-428: Fix incorrect error message for humidity lower bound

The check enforces lower_bound ≤ upper_bound, but the message says “equal or above the lower bound”. This is confusing to users.

-            raise ValueError(
-                f"Invalid humidity lower bound {lower_bound}. It must be equal or above the lower bound {self._hysteresis_config.humidity_lower_bound}."
-            )
+            raise ValueError(
+                f"Invalid humidity lower bound {lower_bound}. It must be ≤ upper bound {self._hysteresis_config.humidity_upper_bound}."
+            )
🧹 Nitpick comments (6)
tests/test_usb.py (2)

2435-2438: Avoid flaky sleeps: await the node’s delayed task instead of arbitrary timeouts

Relying on await asyncio.sleep(0.001) makes the test race-prone on slower CI. Await the node’s _delayed_task spawned by _awake_response() so you deterministically wait for config processing. Repeat the same change for other sleeps in this test.

-        await asyncio.sleep(0.001)  # Ensure time for task to be executed
+        # Wait deterministically for the background config task to finish
+        assert test_sense._delayed_task is not None  # pylint: disable=protected-access
+        await asyncio.wait_for(asyncio.shield(test_sense._delayed_task), timeout=2)

2316-2321: Add an assertion to catch the temperature ‘temp_hum’ flag regression

There’s a real bug in Sense temperature configuration (see review on nodes/sense.py): the request uses the humidity flag. This test won’t catch that. Add an assertion by patching SenseConfigureHysteresisRequest.__init__ to record temp_hum and verify it’s True for temperature and False for humidity.

I can provide a small helper/fixture to capture the constructor args and assert the flag for both humidity and temperature update paths. Want me to draft it?

plugwise_usb/nodes/sense.py (4)

545-549: Docstring typos (‘humitidy’) and wrong subject in temperature setter

Minor but user-facing in generated docs.

-    async def set_hysteresis_temperature_direction(self, state: bool) -> bool:
-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+    async def set_hysteresis_temperature_direction(self, state: bool) -> bool:
+        """Configure temperature hysteresis to switch on or off on increase or decreasing direction.
-    async def set_hysteresis_humidity_direction(self, state: bool) -> bool:
-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+    async def set_hysteresis_humidity_direction(self, state: bool) -> bool:
+        """Configure humidity hysteresis to switch on or off on increase or decreasing direction.

Also applies to: 439-447


492-495: Use proper unit symbol for temperature bounds in error messages

Prefer “°C” over “degrees” for clarity.

-                f"Invalid temperature upper bound {upper_bound}. It must be between 1 and 60 degrees."
+                f"Invalid temperature upper bound {upper_bound}. It must be between 1 and 60 °C."
-                f"Invalid temperature lower bound {lower_bound}. It must be between 1 and 60 degrees."
+                f"Invalid temperature lower bound {lower_bound}. It must be between 1 and 60 °C."

Also applies to: 524-527


597-605: Handle unknown switch groups explicitly

Use elif to avoid double updates and log unexpected groups to aid diagnostics.

-        if switch_group == 1:
+        if switch_group == 1:
             self._sense_statistics.temperature_state = switch_state
-        if switch_group == 2:
+        elif switch_group == 2:
             self._sense_statistics.humidity_state = switch_state
+        else:
+            _LOGGER.debug(
+                "Ignoring unknown switch group %s for %s", switch_group, self.name
+            )

660-662: Align comments with actual sentinel values

Comments say “-1”, but code uses encoded sentinels. Make comments match reality to prevent future confusion.

-        # Set value to -1 for disabled
+        # Encoded sentinel for 'disabled' (humidity): 2621
-        # Set value to -1 for disabled
+        # Encoded sentinel for 'disabled' (temperature): 17099

Also applies to: 714-716

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e4b4fb2 and 80f2bf1.

📒 Files selected for processing (2)
  • plugwise_usb/nodes/sense.py (8 hunks)
  • tests/test_usb.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_usb.py (5)
plugwise_usb/nodes/node.py (5)
  • mac (235-237)
  • cache_enabled (160-162)
  • cache_enabled (165-167)
  • relay_state (298-302)
  • load (373-375)
plugwise_usb/messages/responses.py (7)
  • NodeAckResponse (902-924)
  • current_logaddress_pointer (638-640)
  • firmware (628-630)
  • hardware (623-625)
  • node_type (633-635)
  • relay_state (643-645)
  • NodeAwakeResponse (801-833)
plugwise_usb/api.py (8)
  • NodeEvent (30-36)
  • mac (350-351)
  • NodeType (62-73)
  • cache_enabled (392-393)
  • cache_enabled (396-397)
  • NodeInfoMessage (135-143)
  • relay_state (460-464)
  • load (330-331)
plugwise_usb/nodes/sense.py (19)
  • PlugwiseSense (74-828)
  • load (92-102)
  • hysteresis_config_dirty (346-348)
  • humidity_enabled (290-294)
  • humidity_upper_bound (297-301)
  • humidity_lower_bound (304-308)
  • humidity_direction (311-315)
  • temperature_enabled (318-322)
  • temperature_upper_bound (325-329)
  • temperature_lower_bound (332-336)
  • temperature_direction (339-343)
  • set_hysteresis_humidity_upper_bound (375-405)
  • set_hysteresis_humidity_enabled (354-373)
  • set_hysteresis_humidity_direction (439-458)
  • set_hysteresis_humidity_lower_bound (407-437)
  • set_hysteresis_temperature_upper_bound (481-511)
  • set_hysteresis_temperature_enabled (460-479)
  • set_hysteresis_temperature_direction (545-564)
  • set_hysteresis_temperature_lower_bound (513-543)
plugwise_usb/nodes/sed.py (2)
  • load (102-115)
  • _awake_response (408-465)
plugwise_usb/nodes/sense.py (8)
plugwise_usb/api.py (6)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • unload (333-334)
  • name (322-323)
  • save_cache (402-405)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (4)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (3)
  • initialize (127-137)
  • unload (117-125)
  • dirty (336-338)
plugwise_usb/nodes/switch.py (2)
  • initialize (61-71)
  • unload (73-77)
plugwise_usb/nodes/node.py (8)
  • initialize (412-420)
  • unload (644-651)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (1)
plugwise_usb/nodes/sense.py (1)

635-653: Awake config flow logic looks solid

Dirty clearing only after both humidity and temperature configs succeed; logs and retries on partial failure. Good.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
plugwise_usb/api.py (2)

264-293: Docstring type/units mismatch with fields.

The docstring lists several bounds as int while the dataclass uses float; also units aren’t stated. Align the docs to avoid confusion.

Apply:

@@
     Attributes:
-        humidity_enabled: bool | None: enable humidity hysteresis
-        humidity_upper_bound: int | None: upper humidity switching value
-        humidity_lower_bound: int | None: lower humidity switching value
-        humidity_direction: bool | None: True switch on on increasing humidity, False switch off on increasing humidity
-        temperature_enabled: bool | None: enable temperature hysteresis
-        temperature_upper_bound: int | None: upper temperature switching value
-        temperature_lower_bound: int | None: lower temperature switching value
-        temperature_direction: bool | None: True switch on on increasing temperature, False switch off on increasing temperature
+        humidity_enabled: bool | None: Enable humidity hysteresis.
+        humidity_upper_bound: float | None: Upper humidity switching value in %RH.
+        humidity_lower_bound: float | None: Lower humidity switching value in %RH.
+        humidity_direction: bool | None: True = switch ON when humidity rises past upper bound, False = active-low.
+        temperature_enabled: bool | None: Enable temperature hysteresis.
+        temperature_upper_bound: float | None: Upper temperature switching value in °C.
+        temperature_lower_bound: float | None: Lower temperature switching value in °C.
+        temperature_direction: bool | None: True = switch ON when temperature rises past upper bound, False = active-low.
         dirty: bool: Settings changed, device update pending

295-303: Document newly added SenseStatistics states.

Add brief attribute docs for temperature_state and humidity_state to clarify they reflect hysteresis switch state.

Apply:

 @dataclass
 class SenseStatistics:
-    """Sense statistics collection."""
+    """Sense statistics collection.
+
+    Attributes:
+        temperature: float | None
+        humidity: float | None
+        temperature_state: bool | None  # current temperature hysteresis output
+        humidity_state: bool | None     # current humidity hysteresis output
+    """
plugwise_usb/messages/requests.py (1)

1407-1456: Fix class/init docstring and clarify parameters.

Minor nits: the init docstring mentions Scan; also parameter names could be clearer.

Apply:

 class SenseConfigureHysteresisRequest(PlugwiseRequest):
     """Configure a Sense Hysteresis Switching Setting.
@@
-    temp_hum : configure temperature True or humidity False
+    temp_hum : True = configure temperature, False = configure humidity
     lower_bound : lower bound of the hysteresis
     upper_bound : upper bound of the hysteresis
-    direction   : Switch active high or active low
+    direction   : True = active-high (switch ON when rising past upper bound), False = active-low
@@
-        """Initialize ScanConfigureRequest message object."""
+        """Initialize SenseConfigureHysteresisRequest message object."""

Optional: consider renaming temp_hum to for_temperature (keeping temp_hum as a backward-compatible alias) for readability.

tests/test_usb.py (1)

1995-1997: DRY up awaiting of background “awake” tasks.

Repeated patterns assert _delayed_task and then await it. Factor into a small helper to reduce duplication and improve readability.

Apply (test-only helper):

+async def _await_delayed_task(node, timeout: float = 2.0) -> None:
+    assert getattr(node, "_delayed_task") is not None
+    await asyncio.wait_for(asyncio.shield(node._delayed_task), timeout=timeout)

Then replace the repeated assert/await blocks with:

-assert test_sed._delayed_task is not None
-await asyncio.wait_for(asyncio.shield(test_sed._delayed_task), timeout=2)
+await _await_delayed_task(test_sed)

(and similarly for test_scan/test_sense).

Also applies to: 2013-2015, 2039-2040, 2063-2065, 2083-2085, 2107-2109, 2213-2215, 2229-2231, 2249-2252, 2278-2280, 2445-2449, 2462-2466, 2494-2498, 2547-2551, 2579-2583

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 80f2bf1 and cd968b9.

📒 Files selected for processing (7)
  • plugwise_usb/api.py (2 hunks)
  • plugwise_usb/messages/requests.py (1 hunks)
  • plugwise_usb/nodes/helpers/firmware.py (1 hunks)
  • plugwise_usb/nodes/scan.py (1 hunks)
  • plugwise_usb/nodes/sense.py (8 hunks)
  • pyproject.toml (1 hunks)
  • tests/test_usb.py (12 hunks)
✅ Files skipped from review due to trivial changes (1)
  • pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugwise_usb/nodes/scan.py
  • plugwise_usb/nodes/sense.py
  • plugwise_usb/nodes/helpers/firmware.py
🧰 Additional context used
🧬 Code graph analysis (3)
plugwise_usb/api.py (3)
plugwise_usb/nodes/sense.py (8)
  • humidity_enabled (290-294)
  • humidity_upper_bound (297-301)
  • humidity_lower_bound (304-308)
  • humidity_direction (311-315)
  • temperature_enabled (318-322)
  • temperature_upper_bound (325-329)
  • temperature_lower_bound (332-336)
  • temperature_direction (339-343)
plugwise_usb/nodes/scan.py (1)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (1)
  • dirty (336-338)
plugwise_usb/messages/requests.py (6)
plugwise_usb/messages/responses.py (2)
  • PlugwiseResponse (98-215)
  • NodeAckResponse (902-924)
plugwise_usb/messages/__init__.py (1)
  • mac (53-57)
plugwise_usb/messages/properties.py (1)
  • Int (103-129)
tests/test_usb.py (1)
  • send (251-257)
plugwise_usb/connection/__init__.py (1)
  • send (227-238)
plugwise_usb/exceptions.py (1)
  • MessageError (20-21)
tests/test_usb.py (3)
plugwise_usb/nodes/node.py (6)
  • PlugwiseBaseNode (56-844)
  • mac (235-237)
  • cache_enabled (160-162)
  • cache_enabled (165-167)
  • node_info (225-227)
  • load (373-375)
plugwise_usb/nodes/sense.py (19)
  • PlugwiseSense (74-828)
  • load (92-102)
  • hysteresis_config_dirty (346-348)
  • humidity_enabled (290-294)
  • humidity_upper_bound (297-301)
  • humidity_lower_bound (304-308)
  • humidity_direction (311-315)
  • temperature_enabled (318-322)
  • temperature_upper_bound (325-329)
  • temperature_lower_bound (332-336)
  • temperature_direction (339-343)
  • set_hysteresis_humidity_upper_bound (375-405)
  • set_hysteresis_humidity_enabled (354-373)
  • set_hysteresis_humidity_direction (439-458)
  • set_hysteresis_humidity_lower_bound (407-437)
  • set_hysteresis_temperature_upper_bound (481-511)
  • set_hysteresis_temperature_enabled (460-479)
  • set_hysteresis_temperature_direction (545-564)
  • set_hysteresis_temperature_lower_bound (513-543)
plugwise_usb/nodes/sed.py (2)
  • load (102-115)
  • _awake_response (408-465)
🔇 Additional comments (4)
plugwise_usb/api.py (1)

58-58: New NodeFeature looks good.

SENSE_HYSTERESIS enum addition is consistent with the rest of the feature set.

tests/test_usb.py (3)

39-39: Sense import added — OK.

Importing nodes.sense is required for the new tests.


2420-2437: Bounds validation paths covered well.

Good coverage of invalid, no-op, and valid updates for both humidity and temperature, including direction and enabled flags.

Also applies to: 2502-2519, 2556-2568


2326-2586: ACK codes verified
NodeAckResponseType.SENSE_BOUNDARIES_ACCEPTED is defined as b"00B5" and SENSE_BOUNDARIES_FAILED as b"00B6" in plugwise_usb/messages/responses.py (lines 83–84), matching the test assumptions.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (7)
plugwise_usb/nodes/sense.py (7)

151-154: Tighten super-load result assignment.
Slight readability improvement—no behavior change.

Apply:

-        super_load_success = True
-        if not await super()._load_from_cache():
-            super_load_success = False
+        super_load_success = await super()._load_from_cache()

577-579: Demote “switch group received” to debug to reduce log noise.
These are normal events and can spam logs.

-        _LOGGER.warning("%s received %s", self.name, response)
+        _LOGGER.debug("%s received %s", self.name, response)

596-603: Handle unknown switch groups explicitly.
Avoid silent drops; helps future-proofing.

         if switch_group == 1:
             self._sense_statistics.temperature_state = switch_state
         elif switch_group == 2:
             self._sense_statistics.humidity_state = switch_state
+        else:
+            _LOGGER.debug(
+                "Ignoring unknown switch_group %s for %s", switch_group, self.name
+            )

662-665: Standardize bound validation messages (use ≤/≥).
Consistent, concise errors.

-                f"Invalid humidity lower bound {self.humidity_lower_bound}. It must be equal or below the upper bound {self.humidity_upper_bound}."
+                f"Invalid humidity lower bound {self.humidity_lower_bound}. It must be ≤ upper bound {self.humidity_upper_bound}."
-                f"Invalid temperature lower bound {self.temperature_lower_bound}. It must be equal or below the upper bound {self.temperature_upper_bound}."
+                f"Invalid temperature lower bound {self.temperature_lower_bound}. It must be ≤ upper bound {self.temperature_upper_bound}."

Also applies to: 716-719


666-675: Quantization: round to nearest device step instead of floor.
int() truncates; round() better matches target thresholds.

-            humidity_lower_bound = int(
+            humidity_lower_bound = int(round(
                 (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))
-            humidity_upper_bound = int(
+            humidity_upper_bound = int(round(
                 (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))

720-729: Quantization: round temperature mapping as well.
Aligns with above for consistency.

-            temperature_lower_bound = int(
+            temperature_lower_bound = int(round(
                 (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))
-            temperature_upper_bound = int(
+            temperature_upper_bound = int(round(
                 (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))

440-446: Fix typo and grammar in docstring.
Minor polish.

-        """Configure humitidy hysteresis to switch on or off on increase or decreasing direction.
+        """Configure humidity hysteresis to switch on or off on increasing or decreasing direction.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cd968b9 and 77c5196.

📒 Files selected for processing (1)
  • plugwise_usb/nodes/sense.py (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (8)
plugwise_usb/api.py (5)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • name (322-323)
  • save_cache (402-405)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (4)
  • _switch_group (351-365)
  • initialize (108-118)
  • _load_from_cache (144-173)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (3)
  • initialize (127-137)
  • _load_from_cache (142-176)
  • dirty (336-338)
plugwise_usb/nodes/node.py (8)
  • initialize (412-420)
  • _load_from_cache (396-410)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/switch.py (1)
  • initialize (61-71)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (2)
plugwise_usb/nodes/sense.py (2)

45-49: SENSE_HYSTERESIS correctly added to advertised features.
Makes the new capability discoverable and consistent with firmware support mapping.


596-600: Verify sensor group→state mapping
Confirm that switch_group == 1 maps to temperature_state and switch_group == 2 maps to humidity_state per the Plugwise Sense device specification or existing tests.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
plugwise_usb/nodes/sense.py (2)

660-663: Sentinel (-1%) for disabled humidity is correct (2621).

Matches observed hardware behavior.


714-717: Sentinel (-1 °C) for disabled temperature is correct (17099).

Acknowledged as device-specific; thanks for keeping the exact raw.

🧹 Nitpick comments (7)
plugwise_usb/nodes/sense.py (7)

636-655: Make awake tasks resilient to unexpected exceptions.

If cached bounds are invalid, a ValueError will bubble up and abort awake tasks. Capture exceptions from gather.

-            configure_result = await gather(
-                self._configure_sense_humidity_task(),
-                self._configure_sense_temperature_task(),
-            )
-            if all(configure_result):
+            results = await gather(
+                self._configure_sense_humidity_task(),
+                self._configure_sense_temperature_task(),
+                return_exceptions=True,
+            )
+            humidity_ok = (results[0] is True) and not isinstance(results[0], Exception)
+            temperature_ok = (results[1] is True) and not isinstance(results[1], Exception)
+            if humidity_ok and temperature_ok:
                 self._hysteresis_config = replace(self._hysteresis_config, dirty=False)
                 await self._sense_configure_update()
             else:
                 _LOGGER.warning(
                     "Sense hysteresis configuration partially failed for %s "
                     "(humidity=%s, temperature=%s); will retry on next wake.",
                     self.name,
-                    configure_result[0],
-                    configure_result[1],
+                    results[0],
+                    results[1],
                 )

668-677: Prefer rounding to nearest for raw humidity bounds.

Using int() floors; round() avoids systematic bias.

-            humidity_lower_bound = int(
+            humidity_lower_bound = int(round(
                 (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
-            humidity_upper_bound = int(
+            ))
+            humidity_upper_bound = int(round(
                 (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))

Also applies to: 673-677


722-731: Prefer rounding to nearest for raw temperature bounds.

Align conversion with sensor formula symmetry.

-            temperature_lower_bound = int(
+            temperature_lower_bound = int(round(
                 (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
-            temperature_upper_bound = int(
+            ))
+            temperature_upper_bound = int(round(
                 (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))

Also applies to: 727-731


697-701: Polish log grammar (“Successfully configured …”).

Minor readability tweak.

-            _LOGGER.debug(
-                "Successful configure humidity hysteresis settings for %s", self.name
-            )
+            _LOGGER.debug("Successfully configured humidity hysteresis for %s", self.name)
-            _LOGGER.debug(
-                "Successful configure temperature hysteresis settings for %s", self.name
-            )
+            _LOGGER.debug("Successfully configured temperature hysteresis for %s", self.name)

Also applies to: 752-755


438-441: Fix typo in docstring (“humitidy” → “humidity”).

Minor spelling correction.

-        """Configure humitidy hysteresis to switch on or off on increasing or decreasing direction.
+        """Configure humidity hysteresis to switch on or off on increasing or decreasing direction.

792-796: Publish normalized config object to subscribers.

Use hysteresis_config property so subscribers see defaults applied coherently.

-            self.publish_feature_update_to_subscribers(
-                NodeFeature.SENSE_HYSTERESIS,
-                self._hysteresis_config,
-            ),
+            self.publish_feature_update_to_subscribers(
+                NodeFeature.SENSE_HYSTERESIS,
+                self.hysteresis_config,
+            ),

656-709: Reduce duplication: factor raw bound conversion + sentinels.

Consider a small helper to convert float bounds to raw (with rounding) and to return the disabled sentinel when not enabled. Keeps humidity/temperature tasks symmetrical.

def _to_raw_bound(value: float, offset: float, multiplier: float) -> int:
    return int(round((value + offset) * 65536 / multiplier))

Usage: replace the two per-task conversion blocks with this helper.

Also applies to: 710-763

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 77c5196 and a4367c5.

📒 Files selected for processing (2)
  • plugwise_usb/nodes/sense.py (8 hunks)
  • pyproject.toml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • pyproject.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/nodes/sense.py
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (8)
plugwise_usb/api.py (6)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • unload (333-334)
  • name (322-323)
  • save_cache (402-405)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (5)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • _load_from_cache (144-173)
  • dirty (229-231)
plugwise_usb/nodes/node.py (9)
  • initialize (412-420)
  • unload (644-651)
  • _load_from_cache (396-410)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/switch.py (2)
  • initialize (61-71)
  • unload (73-77)
plugwise_usb/nodes/sed.py (4)
  • initialize (127-137)
  • unload (117-125)
  • _load_from_cache (142-176)
  • dirty (336-338)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
🔇 Additional comments (4)
plugwise_usb/nodes/sense.py (4)

45-49: Feature flag addition looks correct.

SENSE_HYSTERESIS is properly exposed in SENSE_FEATURES.


110-119: Switch-group subscription wired correctly.

Subscription to NODE_SWITCH_GROUP_ID is consistent with Scan/Switch nodes.


127-129: Clean unsubscription on unload.

Good resource cleanup to avoid leaked callbacks.


594-601: Verify switch_group mapping in Sense node
Confirm that group 1 corresponds to temperature and group 2 to humidity—otherwise the assignments in plugwise_usb/nodes/sense.py (lines 594–601) must be swapped.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
plugwise_usb/nodes/sense.py (4)

644-647: Avoid double-publishing SENSE_HYSTERESIS on success

_sense_configure_update already publishes; then you publish again unconditionally. Early-return after a successful configure to prevent duplicate notifications.

         if all(configure_result):
             self._hysteresis_config = replace(self._hysteresis_config, dirty=False)
             await self._sense_configure_update()
+            return
         else:
             _LOGGER.warning(
                 "Sense hysteresis configuration partially failed for %s "
                 "(humidity=%s, temperature=%s); will retry on next wake.",
                 self.name,
                 configure_result[0],
                 configure_result[1],
             )
-        await self.publish_feature_update_to_subscribers(
+        await self.publish_feature_update_to_subscribers(
             NodeFeature.SENSE_HYSTERESIS,
             self.hysteresis_config,
         )

Also applies to: 655-658


575-576: Reduce log level for routine switch-group events

Use debug (or info) instead of warning to avoid log noise during normal operations.

-        _LOGGER.warning("%s received %s", self.name, response)
+        _LOGGER.debug("%s received %s", self.name, response)

672-681: Round before casting to int to avoid systematic bias

Using int() truncates; rounding aligns better with the inverse transform used on reports. Hardware should accept it, but confirm on a device.

-            humidity_lower_bound = int(
+            humidity_lower_bound = int(round(
                 (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))
-            humidity_upper_bound = int(
+            humidity_upper_bound = int(round(
                 (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))

-            temperature_lower_bound = int(
+            temperature_lower_bound = int(round(
                 (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))
-            temperature_upper_bound = int(
+            temperature_upper_bound = int(round(
                 (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))

Also applies to: 726-735


384-387: Message style nit: remove space before '%'

Minor wording tweak for consistency.

-                f"Invalid humidity upper bound {upper_bound}. It must be between 1 and 99 %."
+                f"Invalid humidity upper bound {upper_bound}. It must be between 1 and 99%."
...
-                f"Invalid humidity lower bound {lower_bound}. It must be between 1 and 99 %."
+                f"Invalid humidity lower bound {lower_bound}. It must be between 1 and 99%."

Also applies to: 416-419

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a4367c5 and 6e7dc25.

📒 Files selected for processing (2)
  • plugwise_usb/nodes/sense.py (8 hunks)
  • pyproject.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pyproject.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/nodes/sense.py
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (8)
plugwise_usb/api.py (6)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • unload (333-334)
  • name (322-323)
  • save_cache (402-405)
plugwise_usb/exceptions.py (2)
  • MessageError (20-21)
  • NodeError (24-25)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (6)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • _load_from_cache (144-173)
  • dirty (229-231)
  • _run_awake_tasks (424-434)
plugwise_usb/nodes/sed.py (5)
  • initialize (127-137)
  • unload (117-125)
  • _load_from_cache (142-176)
  • dirty (336-338)
  • _run_awake_tasks (540-550)
plugwise_usb/nodes/node.py (9)
  • initialize (412-420)
  • unload (644-651)
  • _load_from_cache (396-410)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Check commit
🔇 Additional comments (2)
plugwise_usb/nodes/sense.py (2)

664-667: Correct disabled sentinels (ACK)

Using 2621 (-1%) and 17099 (-1°C) matches Source behavior. Thanks for aligning with hardware.

Also applies to: 718-721


594-601: Confirm external mapping: group 1→temperature_state and group 2→humidity_state aligns with the SenseStatistics fields in plugwise_usb/api.py; no conflicting references found in the codebase—please validate against the firmware documentation or trace logs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
plugwise_usb/nodes/sense.py (4)

84-85: Docstring nit: this is a Sense node, not Scan.
Change “Initialize Scan Device.” to “Initialize Sense Device.”

-        """Initialize Scan Device."""
+        """Initialize Sense Device."""

54-63: Prefix cache keys to avoid collisions and ease future migrations.
Keys like "humidity_enabled" are very generic. Prefixing them (as you already do for CONFIG_DIRTY) prevents accidental overlap with other features and makes cache files self-descriptive. Backward compatibility is easy now (feature is new).

-CACHE_SENSE_HYSTERESIS_HUMIDITY_ENABLED = "humidity_enabled"
-CACHE_SENSE_HYSTERESIS_HUMIDITY_UPPER_BOUND = "humidity_upper_bound"
-CACHE_SENSE_HYSTERESIS_HUMIDITY_LOWER_BOUND = "humidity_lower_bound"
-CACHE_SENSE_HYSTERESIS_HUMIDITY_DIRECTION = "humidity_direction"
-CACHE_SENSE_HYSTERESIS_TEMPERATURE_ENABLED = "temperature_enabled"
-CACHE_SENSE_HYSTERESIS_TEMPERATURE_UPPER_BOUND = "temperature_upper_bound"
-CACHE_SENSE_HYSTERESIS_TEMPERATURE_LOWER_BOUND = "temperature_lower_bound"
-CACHE_SENSE_HYSTERESIS_TEMPERATURE_DIRECTION = "temperature_direction"
+CACHE_SENSE_HYSTERESIS_HUMIDITY_ENABLED = "sense_hysteresis_humidity_enabled"
+CACHE_SENSE_HYSTERESIS_HUMIDITY_UPPER_BOUND = "sense_hysteresis_humidity_upper_bound"
+CACHE_SENSE_HYSTERESIS_HUMIDITY_LOWER_BOUND = "sense_hysteresis_humidity_lower_bound"
+CACHE_SENSE_HYSTERESIS_HUMIDITY_DIRECTION = "sense_hysteresis_humidity_direction"
+CACHE_SENSE_HYSTERESIS_TEMPERATURE_ENABLED = "sense_hysteresis_temperature_enabled"
+CACHE_SENSE_HYSTERESIS_TEMPERATURE_UPPER_BOUND = "sense_hysteresis_temperature_upper_bound"
+CACHE_SENSE_HYSTERESIS_TEMPERATURE_LOWER_BOUND = "sense_hysteresis_temperature_lower_bound"
+CACHE_SENSE_HYSTERESIS_TEMPERATURE_DIRECTION = "sense_hysteresis_temperature_direction"

37-44: Name the hardware sentinel values and round raw conversions.

  • Replace magic numbers 2621/17099 with named constants for clarity (and to reflect the hardware-specific values you noted).
  • Use round() before int() when converting to raw sensor units to avoid systematic bias.
@@
 SENSE_TEMPERATURE_LIMIT: Final = 65535
+SENSE_HUMIDITY_DISABLED_RAW: Final = 2621     # -1 %
+SENSE_TEMPERATURE_DISABLED_RAW: Final = 17099 # -1 °C
@@
-        # Set value to -1% for 'disabled' (humidity):2621
-        humidity_lower_bound = 2621
-        humidity_upper_bound = 2621
+        # Set value to -1% for 'disabled' (humidity)
+        humidity_lower_bound = SENSE_HUMIDITY_DISABLED_RAW
+        humidity_upper_bound = SENSE_HUMIDITY_DISABLED_RAW
@@
-            humidity_lower_bound = int(
-                (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
-                * 65536
-                / SENSE_HUMIDITY_MULTIPLIER
-            )
-            humidity_upper_bound = int(
-                (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
-                * 65536
-                / SENSE_HUMIDITY_MULTIPLIER
-            )
+            humidity_lower_bound = int(
+                round(
+                    (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
+                    * 65536
+                    / SENSE_HUMIDITY_MULTIPLIER
+                )
+            )
+            humidity_upper_bound = int(
+                round(
+                    (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
+                    * 65536
+                    / SENSE_HUMIDITY_MULTIPLIER
+                )
+            )
@@
-        # Set value to -1 °C for disabled (temperature): 17099
-        temperature_lower_bound = 17099
-        temperature_upper_bound = 17099
+        # Set value to -1 °C for disabled (temperature)
+        temperature_lower_bound = SENSE_TEMPERATURE_DISABLED_RAW
+        temperature_upper_bound = SENSE_TEMPERATURE_DISABLED_RAW
@@
-            temperature_lower_bound = int(
-                (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
-                * 65536
-                / SENSE_TEMPERATURE_MULTIPLIER
-            )
-            temperature_upper_bound = int(
-                (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
-                * 65536
-                / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            temperature_lower_bound = int(
+                round(
+                    (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
+                    * 65536
+                    / SENSE_TEMPERATURE_MULTIPLIER
+                )
+            )
+            temperature_upper_bound = int(
+                round(
+                    (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
+                    * 65536
+                    / SENSE_TEMPERATURE_MULTIPLIER
+                )
+            )

Note: Using round() aligns better with inverse of your decoding math. If the device expects truncation, we can revert. Also, per our stored learning, the disabled sentinels are hardware-specific (17099 and 2621) — good that you’re using those.

Also applies to: 666-667, 671-682, 678-682, 720-721, 727-736, 732-736


135-138: Initialize SenseStatistics with None to reflect “unknown” until first report.
0.0 can be misread as an actual measurement. The API type hints allow None.

-        self._sense_statistics = SenseStatistics(
-            temperature=0.0,
-            humidity=0.0,
-        )
+        self._sense_statistics = SenseStatistics(
+            temperature=None,
+            humidity=None,
+        )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6e7dc25 and c9d6074.

📒 Files selected for processing (1)
  • plugwise_usb/nodes/sense.py (8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/nodes/sense.py
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (7)
plugwise_usb/api.py (6)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (265-292)
  • SenseStatistics (296-302)
  • unload (333-334)
  • name (322-323)
  • save_cache (402-405)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (6)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • _load_from_cache (144-173)
  • dirty (229-231)
  • _run_awake_tasks (424-434)
plugwise_usb/nodes/sed.py (5)
  • initialize (127-137)
  • unload (117-125)
  • _load_from_cache (142-176)
  • dirty (336-338)
  • _run_awake_tasks (540-550)
plugwise_usb/nodes/node.py (9)
  • initialize (412-420)
  • unload (644-651)
  • _load_from_cache (396-410)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (4)
plugwise_usb/nodes/sense.py (4)

769-803: Nice: cache + publish batched via gather; dirty persisted consistently.
Good separation of concerns and avoids excessive writes.


636-647: Awake flow looks solid: both configs awaited; dirty cleared only on full success.
Prevents partial configuration from being marked clean.


352-372: Setters: validation, idempotence, and dirty handling are clean.
Good early returns and centralized cache update trigger.

Also applies to: 458-478


594-601: Verify switch_group mapping
I don’t see any tests or documentation confirming that switch_group 1 maps to temperature_state and 2 to humidity_state. Please verify against the firmware specs or fixtures and consider adding tests to cover these mappings.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
plugwise_usb/api.py (2)

302-303: Document the new hysteresis state fields.

Extend SenseStatistics docstring to describe temperature_state/humidity_state (Boolean switch states derived from hysteresis).

Suggested docstring tweak:

 @dataclass
 class SenseStatistics:
-    """Sense statistics collection."""
+    """Sense statistics collection.
+
+    Attributes:
+        temperature: float | None: Last measured temperature (°C).
+        humidity: float | None: Last measured humidity (%RH).
+        temperature_state: bool | None: Hysteresis switch state driven by temperature (True=ON).
+        humidity_state: bool | None: Hysteresis switch state driven by humidity (True=ON).
+    """

484-490: Tuple annotation consistency for get_state.

Use tuple[NodeFeature, ...] (as elsewhere in this file) to avoid older type-checker issues with tuple[T].

-    async def get_state(self, features: tuple[NodeFeature]) -> dict[NodeFeature, Any]:
+    async def get_state(self, features: tuple[NodeFeature, ...]) -> dict[NodeFeature, Any]:
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c9d6074 and 38488c2.

📒 Files selected for processing (1)
  • plugwise_usb/api.py (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/api.py
🧬 Code graph analysis (1)
plugwise_usb/api.py (3)
plugwise_usb/nodes/sense.py (8)
  • humidity_enabled (288-292)
  • humidity_upper_bound (295-299)
  • humidity_lower_bound (302-306)
  • humidity_direction (309-313)
  • temperature_enabled (316-320)
  • temperature_upper_bound (323-327)
  • temperature_lower_bound (330-334)
  • temperature_direction (337-341)
plugwise_usb/nodes/scan.py (1)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (1)
  • dirty (336-338)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (1)
plugwise_usb/api.py (1)

58-58: New feature flag looks good.

NodeFeature.SENSE_HYSTERESIS addition is clear and consistent with existing naming.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
plugwise_usb/nodes/sense.py (2)

272-286: LGTM: hysteresis_config now reports the correct dirty flag

Uses the internal config’s dirty field (previous bug fixed).


769-796: LGTM: caching now writes the correct dirty flag

Writes CACHE_SENSE_HYSTERESIS_CONFIG_DIRTY using the config’s dirty value and publishes with save_cache in parallel.

🧹 Nitpick comments (7)
plugwise_usb/nodes/sense.py (7)

149-197: Confirm “dirty-on-missing-cache” behavior is intended

When any cache entry is missing, you mark dirty=True and immediately publish via _sense_configure_update(). This will advertise an optimistic config and persist defaults before the device is actually configured. If this is intentional: fine. If not, consider deferring the publish until after a successful awake-cycle apply. Also, the cache-read boilerplate could be reduced with a small helper to fetch-or-default and OR the dirty flag.


566-584: Downgrade noisy “received” log to debug

Switch-group messages are frequent; WARN may be too loud for normal operation.

Apply:

-        _LOGGER.warning("%s received %s", self.name, response)
+        _LOGGER.debug("%s received %s", self.name, response)

607-635: Tiny nit: avoid magic 65536 literal

Consider a named constant (e.g., SENSE_SCALE_16BIT = 65536) to improve readability where used for both temperature and humidity conversions.


636-656: Harden awake task flow against exceptions

If either configure task raises (e.g., unexpected MessageError), gather will bubble and skip the publish. Wrap in try/except and treat as failure so dirty remains set for retry.

Apply:

         if self._hysteresis_config.dirty:
-            configure_result = await gather(
-                self._configure_sense_humidity_task(),
-                self._configure_sense_temperature_task(),
-            )
+            try:
+                configure_result = await gather(
+                    self._configure_sense_humidity_task(),
+                    self._configure_sense_temperature_task(),
+                )
+            except Exception:  # noqa: BLE001
+                _LOGGER.exception(
+                    "Sense hysteresis configuration raised for %s; will retry on next wake.",
+                    self.name,
+                )
+                configure_result = (False, False)

665-683: Round and clamp raw humidity bounds before sending

Using int() truncates; round to nearest and clamp to device limits to avoid off-by-one at boundary values.

Apply:

-            humidity_lower_bound = int(
+            humidity_lower_bound = int(
                 (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
-            humidity_upper_bound = int(
+            )
+            humidity_lower_bound = max(0, min(humidity_lower_bound, SENSE_HUMIDITY_LIMIT))
+            humidity_upper_bound = int(
                 (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            )
+            humidity_upper_bound = max(0, min(humidity_upper_bound, SENSE_HUMIDITY_LIMIT))

Note: Keep the disabled sentinel 2621 as-is (hardware-specific), per prior learning.


719-736: Round and clamp raw temperature bounds before sending

Mirror rounding/clamping for temperature to avoid truncation artifacts.

Apply:

-            temperature_lower_bound = int(
+            temperature_lower_bound = int(
                 (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
-            temperature_upper_bound = int(
+            )
+            temperature_lower_bound = max(0, min(temperature_lower_bound, SENSE_TEMPERATURE_LIMIT))
+            temperature_upper_bound = int(
                 (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            )
+            temperature_upper_bound = max(0, min(temperature_upper_bound, SENSE_TEMPERATURE_LIMIT))

Note: Keep the disabled sentinel 17099 (hardware-specific), as confirmed.


37-44: Name sentinels as constants for clarity

Optional: promote the “disabled” raw values to named constants alongside the other Sense constants.

Apply:

 SENSE_TEMPERATURE_LIMIT: Final = 65535
+
+# Hardware-specific disabled sentinels (as sent by Source)
+SENSE_DISABLED_HUMIDITY_RAW: Final = 2621    # -1 %
+SENSE_DISABLED_TEMPERATURE_RAW: Final = 17099 # -1 °C

Then use these in the configure tasks.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 38488c2 and 40b45bc.

📒 Files selected for processing (2)
  • plugwise_usb/messages/requests.py (1 hunks)
  • plugwise_usb/nodes/sense.py (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugwise_usb/messages/requests.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/nodes/sense.py
🧬 Code graph analysis (1)
plugwise_usb/nodes/sense.py (6)
plugwise_usb/api.py (5)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (266-293)
  • SenseStatistics (297-303)
  • name (323-324)
  • save_cache (403-406)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (4)
  • _switch_group (351-365)
  • initialize (108-118)
  • dirty (229-231)
  • _run_awake_tasks (424-434)
plugwise_usb/nodes/sed.py (3)
  • initialize (127-137)
  • dirty (336-338)
  • _run_awake_tasks (540-550)
plugwise_usb/nodes/node.py (7)
  • initialize (412-420)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (3)
plugwise_usb/nodes/sense.py (3)

7-7: Verify SenseHysteresisConfig is a dataclass (or dataclass-like) for replace() to work

All updates rely on dataclasses.replace; ensure SenseHysteresisConfig is a @DataClass (frozen or not) to avoid runtime errors. If it isn’t, replace() will raise.

Also applies to: 12-18


45-49: Feature gating looks correct

Exposing NodeFeature.SENSE_HYSTERESIS alongside SENSE and INFO is appropriate and consistent with firmware gating.


115-119: Good: switch-group subscription lifecycle handled

Subscribing to NODE_SWITCH_GROUP_ID and properly unsubscribing on unload prevents orphaned callbacks.

Also applies to: 127-128

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
plugwise_usb/api.py (3)

265-297: Expose hysteresis config in the public Protocol.

Sense nodes surface this config; add a typed property on PlugwiseNode for API completeness.

Add near other “configuration properties”:

    @property
    def sense_hysteresis_config(self) -> SenseHysteresisConfig:
        """Sense hysteresis configuration settings.

        Returns:
            SenseHysteresisConfig: Current hysteresis configuration.
                                   When settings are scheduled, returned values are optimistic.
        Raises:
            FeatureError: When this configuration feature is not present.
            NodeError: When the node is not loaded or configuration failed.
        """

269-271: Docstring nit: tighten sentence.

-    Description: Configuration settings for sense hysteresis.
-                 When value is scheduled to be changed the returned value is the optimistic value
+    Description: Configuration settings for sense hysteresis.
+                 When a change is scheduled, returned values are optimistic.

305-306: Clarify state semantics in SenseStatistics.

Briefly document that these booleans reflect current hysteresis output (per direction), e.g., “True means active/output ON.” Helps consumers avoid ambiguity.

 @dataclass
 class SenseStatistics:
-    """Sense statistics collection."""
+    """Sense statistics collection.
+
+    Notes:
+        temperature_state/humidity_state indicate the current hysteresis output (True = ON, False = OFF),
+        taking the configured direction into account.
+    """
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 40b45bc and a888027.

📒 Files selected for processing (1)
  • plugwise_usb/api.py (3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/api.py:90-90
Timestamp: 2025-09-02T11:06:20.103Z
Learning: Plugwise Sense devices push hysteresis state updates through the NODE_SWITCH_GROUP_ID mechanism. When hysteresis thresholds are crossed, the device sends NodeSwitchGroupResponse messages which are processed by the _switch_group method, updating humidity_state and temperature_state, then publishing to subscribers via NodeFeature.SENSE.
📚 Learning: 2025-09-02T11:06:20.103Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/api.py:90-90
Timestamp: 2025-09-02T11:06:20.103Z
Learning: Plugwise Sense devices push hysteresis state updates through the NODE_SWITCH_GROUP_ID mechanism. When hysteresis thresholds are crossed, the device sends NodeSwitchGroupResponse messages which are processed by the _switch_group method, updating humidity_state and temperature_state, then publishing to subscribers via NodeFeature.SENSE.

Applied to files:

  • plugwise_usb/api.py
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/api.py
🧬 Code graph analysis (1)
plugwise_usb/api.py (3)
plugwise_usb/nodes/sense.py (8)
  • humidity_enabled (288-292)
  • humidity_upper_bound (295-299)
  • humidity_lower_bound (302-306)
  • humidity_direction (309-313)
  • temperature_enabled (316-320)
  • temperature_upper_bound (323-327)
  • temperature_lower_bound (330-334)
  • temperature_direction (337-341)
plugwise_usb/nodes/scan.py (1)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (1)
  • dirty (336-338)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Process test coverage
🔇 Additional comments (2)
plugwise_usb/api.py (2)

58-58: Enum addition looks good.

NodeFeature.SENSE_HYSTERESIS name/value is consistent with existing feature nomenclature.


90-90: Verified hysteresis updates published under SENSE_HYSTERESIS. publish_feature_update_to_subscribers is invoked with NodeFeature.SENSE_HYSTERESIS in plugwise_usb/nodes/sense.py, so subscribers will receive hysteresis events.

Copy link
Contributor

@bouwew bouwew left a comment

Choose a reason for hiding this comment

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

Great work @dirixmjm !

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugwise_usb/nodes/scan.py (1)

533-538: Return public MotionConfig everywhere (avoid mixed representations).

get_state now returns the derived public motion_config, but _run_awake_tasks and _scan_configure_update still publish the internal _motion_config. This can surface None/default placeholders to subscribers and produce inconsistent payloads.

Unify by publishing self.motion_config in both places.

@@
         await self.publish_feature_update_to_subscribers(
             NodeFeature.MOTION_CONFIG,
-            self._motion_config,
+            self.motion_config,
         )
@@
         await gather(
             self.publish_feature_update_to_subscribers(
                 NodeFeature.MOTION_CONFIG,
-                self._motion_config,
+                self.motion_config,
             ),
             self.save_cache(),
         )
🧹 Nitpick comments (8)
plugwise_usb/messages/requests.py (2)

1431-1434: Fix copy-paste in init docstring.

Update the docstring to match the class name for clarity.

-        """Initialize ScanConfigureRequest message object."""
+        """Initialize SenseConfigureHysteresisRequest message object."""

1422-1430: Validate bounds early (developer ergonomics, safer defaults).

Defensive check helps catch misconfig before we touch the device.

     def __init__(  # noqa: PLR0913
         self,
         send_fn: Callable[[PlugwiseRequest, bool], Awaitable[PlugwiseResponse | None]],
         mac: bytes,
         temp_hum: bool,
         lower_bound: int,
         upper_bound: int,
         direction: bool,
     ):
+        if lower_bound > upper_bound:
+            raise MessageError(
+                f"Invalid hysteresis bounds: lower_bound({lower_bound}) > upper_bound({upper_bound})"
+            )
tests/test_usb.py (3)

2326-2586: Solid end-to-end Sense hysteresis flow coverage; consider adding switch-group state tests and a small helper to DRY wake/await.

  • Nice validation of ranges, dirty-flag behavior, and awake cycles.
  • Please add a targeted test that injects a NodeSwitchGroupResponse (groups 1=humidity, 2=temperature) to assert humidity_hysteresis_state/temperature_hysteresis_state toggling and SENSE publication.
  • The wake/await pattern repeats many times; factor a tiny helper, e.g. await_awake_and_task(node, resp), to reduce duplication and flakiness.

2581-2583: Redundant double-await of the same delayed_task (flaky and unnecessary).

You await the same _delayed_task twice before sending the next awake. Drop the second await.

Apply:

-        assert test_sense._delayed_task is not None  # pylint: disable=protected-access
-        await asyncio.wait_for(asyncio.shield(test_sense._delayed_task), timeout=2)
+        # already awaited delayed_task for this awake cycle

1995-2015: Good pattern: assert task scheduled then await with timeout.

This keeps the async flow deterministic. Consider extracting a helper to standardize this across SED/SCAN/SENSE tests.

plugwise_usb/nodes/sense.py (3)

135-138: Prefer None over 0.0 for initial Sense statistics.

0.0 can be misread as a real measurement; None signals “unknown until first report.”

Apply:

-        self._sense_statistics = SenseStatistics(
-            temperature=0.0,
-            humidity=0.0,
-        )
+        self._sense_statistics = SenseStatistics(
+            temperature=None,
+            humidity=None,
+        )

673-681: Round raw boundary conversions to nearest integer to avoid off-by-one.

int(...) truncates; round(...) matches typical device math and reduces borderline errors.

Apply:

-            humidity_lower_bound = int(
+            humidity_lower_bound = int(round(
                 (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))
-            humidity_upper_bound = int(
+            humidity_upper_bound = int(round(
                 (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
                 * 65536
                 / SENSE_HUMIDITY_MULTIPLIER
-            )
+            ))
-            temperature_lower_bound = int(
+            temperature_lower_bound = int(round(
                 (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))
-            temperature_upper_bound = int(
+            temperature_upper_bound = int(round(
                 (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
                 * 65536
                 / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            ))

Also applies to: 727-735


575-575: Downgrade noisy switch-group log to debug.

This fires on every hysteresis toggle and can flood logs.

Apply:

-        _LOGGER.warning("%s received %s", self.name, response)
+        _LOGGER.debug("%s received %s", self.name, response)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a888027 and 0222d29.

📒 Files selected for processing (7)
  • plugwise_usb/api.py (3 hunks)
  • plugwise_usb/messages/requests.py (1 hunks)
  • plugwise_usb/nodes/helpers/firmware.py (1 hunks)
  • plugwise_usb/nodes/scan.py (1 hunks)
  • plugwise_usb/nodes/sense.py (8 hunks)
  • pyproject.toml (1 hunks)
  • tests/test_usb.py (12 hunks)
✅ Files skipped from review due to trivial changes (1)
  • pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugwise_usb/nodes/helpers/firmware.py
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/api.py:90-90
Timestamp: 2025-09-02T11:06:20.103Z
Learning: Plugwise Sense devices push hysteresis state updates through the NODE_SWITCH_GROUP_ID mechanism. When hysteresis thresholds are crossed, the device sends NodeSwitchGroupResponse messages which are processed by the _switch_group method, updating humidity_state and temperature_state, then publishing to subscribers via NodeFeature.SENSE.
📚 Learning: 2025-09-02T11:06:20.103Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/api.py:90-90
Timestamp: 2025-09-02T11:06:20.103Z
Learning: Plugwise Sense devices push hysteresis state updates through the NODE_SWITCH_GROUP_ID mechanism. When hysteresis thresholds are crossed, the device sends NodeSwitchGroupResponse messages which are processed by the _switch_group method, updating humidity_state and temperature_state, then publishing to subscribers via NodeFeature.SENSE.

Applied to files:

  • plugwise_usb/api.py
  • plugwise_usb/nodes/sense.py
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/api.py
  • plugwise_usb/nodes/sense.py
📚 Learning: 2025-08-07T17:50:29.188Z
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#302
File: plugwise_usb/nodes/scan.py:132-142
Timestamp: 2025-08-07T17:50:29.188Z
Learning: In the plugwise_usb codebase, the `_load_defaults` method in node classes (scan.py, switch.py, sense.py) is only called once during node initialization, not during runtime. Therefore, flags like `_sed_node_info_update_task_scheduled` don't need to be reset at the start of `_load_defaults` as there's no risk of stale state from previous loads.

Applied to files:

  • plugwise_usb/nodes/scan.py
🧬 Code graph analysis (5)
plugwise_usb/api.py (3)
plugwise_usb/nodes/sense.py (8)
  • humidity_enabled (288-292)
  • humidity_upper_bound (295-299)
  • humidity_lower_bound (302-306)
  • humidity_direction (309-313)
  • temperature_enabled (316-320)
  • temperature_upper_bound (323-327)
  • temperature_lower_bound (330-334)
  • temperature_direction (337-341)
plugwise_usb/nodes/scan.py (1)
  • dirty (229-231)
plugwise_usb/nodes/sed.py (1)
  • dirty (336-338)
plugwise_usb/messages/requests.py (4)
plugwise_usb/messages/responses.py (2)
  • PlugwiseResponse (98-215)
  • NodeAckResponse (902-924)
plugwise_usb/messages/__init__.py (1)
  • mac (53-57)
plugwise_usb/messages/properties.py (1)
  • Int (103-129)
plugwise_usb/exceptions.py (1)
  • MessageError (20-21)
tests/test_usb.py (6)
plugwise_usb/nodes/node.py (8)
  • PlugwiseBaseNode (56-844)
  • mac (235-237)
  • cache_enabled (160-162)
  • cache_enabled (165-167)
  • node_info (225-227)
  • relay_state (298-302)
  • update_node_details (487-523)
  • load (373-375)
plugwise_usb/messages/responses.py (8)
  • NodeAckResponse (902-924)
  • deserialize (137-201)
  • current_logaddress_pointer (638-640)
  • firmware (628-630)
  • hardware (623-625)
  • node_type (633-635)
  • relay_state (643-645)
  • NodeAwakeResponse (801-833)
plugwise_usb/api.py (9)
  • NodeEvent (30-36)
  • mac (354-355)
  • NodeType (62-73)
  • cache_enabled (396-397)
  • cache_enabled (400-401)
  • node_info (330-331)
  • NodeInfoMessage (136-144)
  • relay_state (464-468)
  • load (334-335)
plugwise_usb/nodes/sense.py (19)
  • PlugwiseSense (74-834)
  • load (92-102)
  • hysteresis_config_dirty (344-346)
  • humidity_enabled (288-292)
  • humidity_upper_bound (295-299)
  • humidity_lower_bound (302-306)
  • humidity_direction (309-313)
  • temperature_enabled (316-320)
  • temperature_upper_bound (323-327)
  • temperature_lower_bound (330-334)
  • temperature_direction (337-341)
  • set_hysteresis_humidity_upper_bound (373-403)
  • set_hysteresis_humidity_enabled (352-371)
  • set_hysteresis_humidity_direction (437-456)
  • set_hysteresis_humidity_lower_bound (405-435)
  • set_hysteresis_temperature_upper_bound (479-509)
  • set_hysteresis_temperature_enabled (458-477)
  • set_hysteresis_temperature_direction (543-562)
  • set_hysteresis_temperature_lower_bound (511-541)
plugwise_usb/network/registry.py (2)
  • cache_enabled (58-60)
  • cache_enabled (63-70)
plugwise_usb/nodes/sed.py (2)
  • load (102-115)
  • _awake_response (408-465)
plugwise_usb/nodes/scan.py (2)
plugwise_usb/api.py (2)
  • NodeFeature (39-59)
  • motion_config (536-546)
plugwise_usb/nodes/node.py (1)
  • motion_config (249-255)
plugwise_usb/nodes/sense.py (7)
plugwise_usb/api.py (5)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (266-296)
  • SenseStatistics (300-306)
  • name (326-327)
  • save_cache (406-409)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (4)
  • _switch_group (351-365)
  • initialize (108-118)
  • dirty (229-231)
  • _run_awake_tasks (424-434)
plugwise_usb/nodes/sed.py (3)
  • initialize (127-137)
  • dirty (336-338)
  • _run_awake_tasks (540-550)
plugwise_usb/nodes/node.py (7)
  • initialize (412-420)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Check commit
🔇 Additional comments (10)
plugwise_usb/messages/requests.py (1)

1433-1444: Good: Int wrappers fix .serialize() contract and argument ordering looks consistent.

The switch to Int(...) for all args avoids AttributeError and mirrors existing request patterns.

plugwise_usb/api.py (4)

58-59: Add NodeFeature.SENSE_HYSTERESIS — LGTM.

Enum extension is straightforward and scoped.


80-92: Include SENSE_HYSTERESIS in PUSHING_FEATURES — LGTM.

Matches Sense push via switch-group updates per current implementation.


265-297: SenseHysteresisConfig API surface — solid addition.

Types/units in docstring align with fields; frozen dataclass is appropriate for config snapshots.


305-306: Expose hysteresis states in SenseStatistics — LGTM.

Names avoid collisions with existing fields and clarify scope.

plugwise_usb/nodes/sense.py (5)

115-119: Correctly subscribing to NODE_SWITCH_GROUP_ID.

Matches device behavior (Sense pushes hysteresis state via switch-group). Good placement during initialize.

Note: used your learning about switch-group updates from this PR context.


566-584: Switch-group handler looks good.

Type check, availability update, and routing to hysteresis state update are correct.


585-606: Correct state mapping and publication.

Group 1→humidity, 2→temperature, publish SENSE stats — aligns with device semantics.


636-660: Right place to clear dirty only when both tasks succeed.

The all(configure_result) gate avoids partial-apply issues. Good.


769-803: Cache sync and publication flow is consistent.

Writes all keys and publishes SENSE_HYSTERESIS; relies on save_cache’s guards when uninitialized. Good.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
plugwise_usb/messages/requests.py (1)

1433-1444: LGTM: arguments now use Int(...) objects, fixing serialization.

The earlier raw-int issue is resolved; lengths and ordering look consistent with the protocol.

🧹 Nitpick comments (4)
plugwise_usb/messages/requests.py (1)

1431-1432: Fix constructor docstring naming.

Use the correct class name in the docstring.

-        """Initialize ScanConfigureHysteresisRequest message object."""
+        """Initialize SenseConfigureHysteresisRequest message object."""
plugwise_usb/nodes/sense.py (3)

572-572: Reduce log level for routine switch-group events.

These events are normal device behavior; debug avoids log noise.

-        _LOGGER.warning("%s received %s", self.name, response)
+        _LOGGER.debug("%s received %s", self.name, response)

670-679: Round and clamp register conversions (humidity) to avoid truncation bias/overflow.

Use round() and clamp to [0, SENSE_HUMIDITY_LIMIT] to prevent systematic underestimation and out-of-range values.

-            humidity_lower_bound = int(
-                (self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET)
-                * 65536
-                / SENSE_HUMIDITY_MULTIPLIER
-            )
-            humidity_upper_bound = int(
-                (self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET)
-                * 65536
-                / SENSE_HUMIDITY_MULTIPLIER
-            )
+            humidity_lower_bound = max(
+                0,
+                min(
+                    SENSE_HUMIDITY_LIMIT,
+                    int(round((self.humidity_lower_bound + SENSE_HUMIDITY_OFFSET) * 65536 / SENSE_HUMIDITY_MULTIPLIER)),
+                ),
+            )
+            humidity_upper_bound = max(
+                0,
+                min(
+                    SENSE_HUMIDITY_LIMIT,
+                    int(round((self.humidity_upper_bound + SENSE_HUMIDITY_OFFSET) * 65536 / SENSE_HUMIDITY_MULTIPLIER)),
+                ),
+            )

Also applies to: 675-679


724-733: Round and clamp register conversions (temperature) to avoid truncation bias/overflow.

Same rationale as humidity; keep hardware-specific disabled sentinel (17099) as-is.

-            temperature_lower_bound = int(
-                (self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET)
-                * 65536
-                / SENSE_TEMPERATURE_MULTIPLIER
-            )
-            temperature_upper_bound = int(
-                (self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET)
-                * 65536
-                / SENSE_TEMPERATURE_MULTIPLIER
-            )
+            temperature_lower_bound = max(
+                0,
+                min(
+                    SENSE_TEMPERATURE_LIMIT,
+                    int(round((self.temperature_lower_bound + SENSE_TEMPERATURE_OFFSET) * 65536 / SENSE_TEMPERATURE_MULTIPLIER)),
+                ),
+            )
+            temperature_upper_bound = max(
+                0,
+                min(
+                    SENSE_TEMPERATURE_LIMIT,
+                    int(round((self.temperature_upper_bound + SENSE_TEMPERATURE_OFFSET) * 65536 / SENSE_TEMPERATURE_MULTIPLIER)),
+                ),
+            )

Also applies to: 730-733

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0222d29 and 32464f5.

📒 Files selected for processing (5)
  • CHANGELOG.md (1 hunks)
  • plugwise_usb/messages/requests.py (1 hunks)
  • plugwise_usb/nodes/scan.py (3 hunks)
  • plugwise_usb/nodes/sense.py (8 hunks)
  • pyproject.toml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • pyproject.toml
  • plugwise_usb/nodes/scan.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/api.py:90-90
Timestamp: 2025-09-02T11:06:20.103Z
Learning: Plugwise Sense devices push hysteresis state updates through the NODE_SWITCH_GROUP_ID mechanism. When hysteresis thresholds are crossed, the device sends NodeSwitchGroupResponse messages which are processed by the _switch_group method, updating humidity_state and temperature_state, then publishing to subscribers via NodeFeature.SENSE.
📚 Learning: 2025-09-02T11:06:20.103Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/api.py:90-90
Timestamp: 2025-09-02T11:06:20.103Z
Learning: Plugwise Sense devices push hysteresis state updates through the NODE_SWITCH_GROUP_ID mechanism. When hysteresis thresholds are crossed, the device sends NodeSwitchGroupResponse messages which are processed by the _switch_group method, updating humidity_state and temperature_state, then publishing to subscribers via NodeFeature.SENSE.

Applied to files:

  • plugwise_usb/nodes/sense.py
📚 Learning: 2025-09-01T11:34:30.352Z
Learnt from: dirixmjm
PR: plugwise/python-plugwise-usb#330
File: plugwise_usb/nodes/sense.py:659-661
Timestamp: 2025-09-01T11:34:30.352Z
Learning: In Plugwise Sense hysteresis configuration, the disabled sentinel values are hardware-specific: 17099 for temperature (-1°C) and 2621 for humidity (-1%), as actually sent by the Source device, not calculated values.

Applied to files:

  • plugwise_usb/nodes/sense.py
🧬 Code graph analysis (2)
plugwise_usb/messages/requests.py (5)
plugwise_usb/messages/responses.py (2)
  • PlugwiseResponse (98-215)
  • NodeAckResponse (902-924)
plugwise_usb/messages/__init__.py (1)
  • mac (53-57)
plugwise_usb/messages/properties.py (1)
  • Int (103-129)
plugwise_usb/connection/__init__.py (1)
  • send (227-238)
plugwise_usb/exceptions.py (1)
  • MessageError (20-21)
plugwise_usb/nodes/sense.py (6)
plugwise_usb/api.py (6)
  • NodeFeature (39-59)
  • SenseHysteresisConfig (266-296)
  • SenseStatistics (300-306)
  • unload (337-338)
  • name (326-327)
  • save_cache (406-409)
plugwise_usb/messages/requests.py (18)
  • SenseConfigureHysteresisRequest (1407-1455)
  • response (124-128)
  • send (357-366)
  • send (379-388)
  • send (430-432)
  • send (468-477)
  • send (504-513)
  • send (534-545)
  • send (558-567)
  • send (580-589)
  • send (613-622)
  • send (659-668)
  • send (711-720)
  • send (767-776)
  • send (800-809)
  • send (840-849)
  • send (872-881)
  • send (904-913)
plugwise_usb/messages/responses.py (6)
  • NodeAckResponseType (75-84)
  • NodeSwitchGroupResponse (836-868)
  • PlugwiseResponse (98-215)
  • SenseReportResponse (927-941)
  • switch_state (857-859)
  • switch_group (862-864)
plugwise_usb/nodes/scan.py (4)
  • _switch_group (351-365)
  • initialize (108-118)
  • unload (120-124)
  • dirty (229-231)
plugwise_usb/nodes/node.py (8)
  • initialize (412-420)
  • unload (644-651)
  • _get_cache_as_bool (659-665)
  • _get_cache (653-657)
  • name (218-222)
  • _available_update_state (422-450)
  • _set_cache (690-704)
  • save_cache (706-720)
plugwise_usb/nodes/helpers/subscription.py (1)
  • publish_feature_update_to_subscribers (51-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Prepare
🔇 Additional comments (4)
plugwise_usb/nodes/sense.py (4)

115-119: LGTM: subscribes to switch-group updates for hysteresis state.

Correctly subscribes to NODE_SWITCH_GROUP_ID and passes switch_group onward.


127-129: LGTM: unsubscribes switch-group listener on unload.

Prevents leaks on teardown.


636-644: Clear dirty only after both configs succeed — good.

The success-all gate with gather(...) is correct and avoids the previous early-clear bug.


766-793: Cache writebacks look consistent.

All hysteresis fields, including dirty, are persisted; publishing and save are batched via gather.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 3, 2025

@dirixmjm dirixmjm merged commit 0698cce into main Sep 3, 2025
17 checks passed
@dirixmjm dirixmjm deleted the mdi_sense branch September 3, 2025 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants