Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request increases the EXTERNAL_CHANGE_TOLERANCE and replaces the dynamic pending_request_tolerance with a fixed target_tolerance in the realignment logic. Review feedback highlights that the new tolerance of 0.55 is too high for Celsius thermostats with 0.5-degree increments, which could lead to manual changes being ignored. Furthermore, reverting from a dynamic tolerance to a hardcoded one may cause issues with high-precision hardware by incorrectly skipping necessary updates.
| MAX_TRACKED_REAL_TARGET_REQUESTS = 5 | ||
| PENDING_REQUEST_TIMEOUT = 30.0 # Seconds before a pending request expires | ||
| EXTERNAL_CHANGE_TOLERANCE = 0.2 # Degrees to ignore as noise/rounding for external detection | ||
| EXTERNAL_CHANGE_TOLERANCE = 0.55 # Degrees to ignore as noise/rounding for external detection |
There was a problem hiding this comment.
Increasing EXTERNAL_CHANGE_TOLERANCE to 0.55 will cause manual adjustments of 0.5 degrees (common in Celsius thermostats) to be ignored by the proxy's external change detection. This means the proxy will not switch to the "Physical Entity" preset when a user manually adjusts the thermostat by a single step, and it will likely overwrite the user's change during the next realignment cycle. Consider using a value slightly smaller than the minimum expected step (e.g., 0.4) or making this tolerance dynamic based on the device's precision.
|
|
||
| pending_tolerance = self._pending_request_tolerance() | ||
| if self._has_pending_real_target_request(desired_real_target, pending_tolerance): | ||
| if self._has_pending_real_target_request(desired_real_target, target_tolerance): |
There was a problem hiding this comment.
Replacing the dynamic pending_tolerance with a hardcoded target_tolerance (0.1) may lead to issues with high-precision thermostats (e.g., those with 0.01 or 0.05 precision). A 0.1 tolerance is too loose for such devices and could cause the proxy to incorrectly match different target temperatures as being the same, resulting in skipped updates. The previous use of self._pending_request_tolerance() (which scales as precision / 2) is more robust for varying hardware.
| if self._has_pending_real_target_request(desired_real_target, target_tolerance): | |
| pending_tolerance = self._pending_request_tolerance() | |
| if self._has_pending_real_target_request(desired_real_target, pending_tolerance): |
1 participant
No description provided.