-
Notifications
You must be signed in to change notification settings - Fork 2
Alternative scan update #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d9dd756
0f36033
1df1bd5
8ffa0c3
91ead79
68f3a3d
d29c9b7
08911d4
261d41a
c24d8da
4a42522
99dc860
663aba7
4b4a14c
8d7ba2c
a78207d
8072bd3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -233,14 +233,14 @@ class MotionConfig: | |||||||||||||||||||||||||||
| Attributes: | ||||||||||||||||||||||||||||
| reset_timer: int | None: Motion reset timer in minutes before the motion detection is switched off. | ||||||||||||||||||||||||||||
| daylight_mode: bool | None: Motion detection when light level is below threshold. | ||||||||||||||||||||||||||||
| sensitivity_level: int | None: Motion sensitivity level. | ||||||||||||||||||||||||||||
| sensitivity_level: MotionSensitivity | None: Motion sensitivity level. | ||||||||||||||||||||||||||||
| dirty: bool: Settings changed, device update pending | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| daylight_mode: bool | None = None | ||||||||||||||||||||||||||||
| reset_timer: int | None = None | ||||||||||||||||||||||||||||
| sensitivity_level: int | None = None | ||||||||||||||||||||||||||||
| sensitivity_level: MotionSensitivity | None = None | ||||||||||||||||||||||||||||
| dirty: bool = False | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
@@ -678,6 +678,13 @@ async def set_motion_sensitivity_level(self, level: MotionSensitivity) -> bool: | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def scan_calibrate_light(self) -> bool: | ||||||||||||||||||||||||||||
| """Request to calibration light sensitivity of Scan device. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Description: | ||||||||||||||||||||||||||||
| Request to calibration light sensitivity of Scan device. | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+681
to
+687
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion API contract mismatch: scan_calibrate_light return type In this Protocol, scan_calibrate_light() returns bool with semantics “True if successful.” In PlugwiseScan (nodes/scan.py lines 493-496), the method now schedules calibration and returns None, with execution moved to _scan_calibrate_light() during awake tasks. This is a breaking contract between the public API (Protocol and base class) and the concrete implementation. Please align the API to the new scheduling model. Suggested change: make the public method “schedule-only” and return None consistently across API, base class, and implementations; document that actual success/failure will be surfaced via logs/events. Proposed diff for this file: - async def scan_calibrate_light(self) -> bool:
- """Request to calibration light sensitivity of Scan device.
-
- Description:
- Request to calibration light sensitivity of Scan device.
- """
+ async def scan_calibrate_light(self) -> None:
+ """Schedule light sensitivity calibration of a Scan device.
+
+ Description:
+ Schedules a light sensitivity calibration request to be performed
+ when the device is awake for maintenance. No return value.
+ """Also update nodes/node.py to match (see scan.py review for a code block). 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
| async def set_relay_init(self, state: bool) -> bool: | ||||||||||||||||||||||||||||
| """Change the initial state of the relay. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,7 +86,7 @@ def __init__( | |
|
|
||
| self._motion_state = MotionState() | ||
| self._motion_config = MotionConfig() | ||
|
|
||
| self._scan_calibrate_light_scheduled = False | ||
| self._configure_daylight_mode_task: Task[Coroutine[Any, Any, None]] | None = ( | ||
| None | ||
| ) | ||
|
|
@@ -198,7 +198,7 @@ def _reset_timer_from_cache(self) -> int | None: | |
| return int(reset_timer) | ||
| return None | ||
|
|
||
| def _sensitivity_level_from_cache(self) -> int | None: | ||
| def _sensitivity_level_from_cache(self) -> MotionSensitivity | None: | ||
| """Load sensitivity level from cache.""" | ||
| if ( | ||
| sensitivity_level := self._get_cache( | ||
|
|
@@ -274,7 +274,7 @@ def reset_timer(self) -> int: | |
| return DEFAULT_RESET_TIMER | ||
|
|
||
| @property | ||
| def sensitivity_level(self) -> int: | ||
| def sensitivity_level(self) -> MotionSensitivity: | ||
| """Sensitivity level of motion sensor.""" | ||
| if self._motion_config.sensitivity_level is not None: | ||
| return self._motion_config.sensitivity_level | ||
|
|
@@ -326,13 +326,13 @@ async def set_motion_reset_timer(self, minutes: int) -> bool: | |
| await self._scan_configure_update() | ||
| return True | ||
|
|
||
| async def set_motion_sensitivity_level(self, level: int) -> bool: | ||
| async def set_motion_sensitivity_level(self, level: MotionSensitivity) -> bool: | ||
| """Configure the motion sensitivity level.""" | ||
| _LOGGER.debug( | ||
| "set_motion_sensitivity_level | Device %s | %s -> %s", | ||
| self.name, | ||
| self._motion_config.sensitivity_level, | ||
| level, | ||
| self._motion_config.sensitivity_level.name, | ||
| level.name, | ||
| ) | ||
| if self._motion_config.sensitivity_level == level: | ||
| return False | ||
|
|
@@ -426,6 +426,8 @@ async def _run_awake_tasks(self) -> None: | |
| await super()._run_awake_tasks() | ||
| if self._motion_config.dirty: | ||
| await self._configure_scan_task() | ||
| if self._scan_calibrate_light_scheduled: | ||
| await self._scan_calibrate_light() | ||
| await self.publish_feature_update_to_subscribers( | ||
| NodeFeature.MOTION_CONFIG, | ||
| self._motion_config, | ||
|
|
@@ -446,9 +448,9 @@ async def scan_configure(self) -> bool: | |
| request = ScanConfigureRequest( | ||
| self._send, | ||
| self._mac_in_bytes, | ||
| self._motion_config.reset_timer, | ||
| self._motion_config.sensitivity_level, | ||
| self._motion_config.daylight_mode, | ||
| self.reset_timer, | ||
| self.sensitivity_level, | ||
| self.daylight_mode, | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (response := await request.send()) is None: | ||
| _LOGGER.warning( | ||
|
|
@@ -473,17 +475,13 @@ async def scan_configure(self) -> bool: | |
|
|
||
| async def _scan_configure_update(self) -> None: | ||
| """Push scan configuration update to cache.""" | ||
| self._set_cache( | ||
| CACHE_SCAN_CONFIG_RESET_TIMER, str(self._motion_config.reset_timer) | ||
| ) | ||
| self._set_cache(CACHE_SCAN_CONFIG_RESET_TIMER, str(self.reset_timer)) | ||
| self._set_cache( | ||
| CACHE_SCAN_CONFIG_SENSITIVITY, | ||
| str(MotionSensitivity(self._motion_config.sensitivity_level).name), | ||
| ) | ||
| self._set_cache( | ||
| CACHE_SCAN_CONFIG_DAYLIGHT_MODE, str(self._motion_config.daylight_mode) | ||
| self._motion_config.sensitivity_level.name, | ||
| ) | ||
| self._set_cache(CACHE_SCAN_CONFIG_DIRTY, str(self._motion_config.dirty)) | ||
| self._set_cache(CACHE_SCAN_CONFIG_DAYLIGHT_MODE, str(self.daylight_mode)) | ||
| self._set_cache(CACHE_SCAN_CONFIG_DIRTY, str(self.dirty)) | ||
| await gather( | ||
| self.publish_feature_update_to_subscribers( | ||
| NodeFeature.MOTION_CONFIG, | ||
|
|
@@ -492,14 +490,19 @@ async def _scan_configure_update(self) -> None: | |
| self.save_cache(), | ||
| ) | ||
|
|
||
| async def scan_calibrate_light(self) -> bool: | ||
| async def scan_calibrate_light(self) -> None: | ||
| """Schedule light sensitivity calibration of Scan device.""" | ||
| self._scan_calibrate_light_scheduled = True | ||
|
|
||
|
Comment on lines
+493
to
+496
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Public API semantics changed; propagate to base class and Protocol This method now schedules calibration and returns None. Please update:
You can keep _scan_calibrate_light() as the boolean-returning executor. 🤖 Prompt for AI Agents |
||
| async def _scan_calibrate_light(self) -> bool: | ||
| """Request to calibration light sensitivity of Scan device.""" | ||
| request = ScanLightCalibrateRequest(self._send, self._mac_in_bytes) | ||
| if (response := await request.send()) is not None: | ||
| if ( | ||
| response.node_ack_type | ||
| == NodeAckResponseType.SCAN_LIGHT_CALIBRATION_ACCEPTED | ||
| ): | ||
| self._scan_calibrate_light_scheduled = False | ||
| return True | ||
| return False | ||
| raise NodeTimeout( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type contract drift: MotionConfig.sensitivity_level vs implementation
You’ve changed MotionConfig.sensitivity_level to MotionSensitivity | None, which is a good direction. However, PlugwiseScan currently stores and exposes an int for sensitivity_level (see nodes/scan.py line 168 and property at line 276), leading to a mismatch with this public type. This will trip strict type-checking and confuse downstream consumers.
Recommend making the node consistently use MotionSensitivity end-to-end (store the enum in _motion_config, property returns MotionSensitivity) and only convert to the numeric value at the request boundary and when writing to cache. See suggested fixes in scan.py comments.
🤖 Prompt for AI Agents