Fix decode_reservation_hex docstring: param is half-°C, not 20°F offset - #115
Merged
Conversation
…fset The docstring claimed byte 5 was "temperature offset by 20°F", which contradicts the half-degree-Celsius encoding used consistently elsewhere (models/schedule.py, models/_converters.py reservation_param_to_preferred). Fixes #113
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the decode_reservation_hex documentation in src/nwp500/encoding.py to correctly describe the reservation param byte as a half-degrees Celsius value (°C = param/2), aligning the docstring with the rest of the schedule/model/protocol documentation and avoiding misleading “20°F offset” guidance.
Changes:
- Corrected the
decode_reservation_hexdocstring to describeparamas half-degrees Celsius (°C = param/2).
Comment on lines
344
to
+348
| - Byte 1: week bitfield (days of week) | ||
| - Byte 2: hour (0-23) | ||
| - Byte 3: minute (0-59) | ||
| - Byte 4: mode (operation mode ID) | ||
| - Byte 5: param (temperature offset by 20°F) | ||
| - Byte 5: param (target temperature, half-degrees Celsius; °C = param/2) |
Owner
Author
There was a problem hiding this comment.
Fixed in 7c4e7c7 — the example now shows 'min' instead of 'minute', matching the actual dict key.
Review feedback on #115: the Examples section still showed the decoded dict using a 'minute' key, but the function actually returns 'min' (see the dict construction below and every schedule model). Corrected the doctest-style example to match the real return value.
eman
added a commit
that referenced
this pull request
Jul 23, 2026
Resolves conflicts with #115 (docstring fix) and #116 (freeze protection validation), both merged to main after this branch was cut. Conflicts were purely additive: dr_setting_use and freeze_protection_use both added to _CAPABILITY_MAP, and their respective test classes both added to test_protocol_correctness.py / test_device_capabilities.py. Combined capability count is now 10.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
decode_reservation_hexdocstring insrc/nwp500/encoding.pysaid byte 5 (param) was "temperature offset by 20°F", which is stale/incorrect.paramis documented and treated as half-degrees Celsius (°C = param/2):models/schedule.py,models/_converters.py(reservation_param_to_preferred→HalfCelsius(param)), and the protocol docs.Fixes #113
Test plan
encoding.py.