-
Notifications
You must be signed in to change notification settings - Fork 2
fix sign bug in humidity #261
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
Conversation
WalkthroughThe constructor of the Changes
Poem
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #261 +/- ##
==========================================
- Coverage 81.13% 81.12% -0.02%
==========================================
Files 36 36
Lines 7502 7502
==========================================
- Hits 6087 6086 -1
- Misses 1415 1416 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise_usb/messages/responses.py (1)
933-935: Switch to unsignedIntfixes negative-humidity bug – use keywords for clarityGood catch making humidity unsigned:
negative=Falseprevents bogus negative values.
Tiny readability nit: pass the attributes as keywords to avoid guessing what the two positional4, Falsestand for.- self.humidity = Int(0, 4, False) + self.humidity = Int(0, length=4, negative=False)(Optional) you may also clamp/validate the deserialized value (0–100 %) when exposing it downstream.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugwise_usb/messages/responses.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/messages/responses.py (1)
plugwise_usb/messages/properties.py (1)
Int(100-126)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Check commit
- GitHub Check: Check commit
bouwew
left a comment
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.
Please update the CHANGELOG as suggested on Discord
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CHANGELOG.md (1)
3-4: Inconsistent heading casingThe new section heading uses lowercase ("ongoing") while all other version headers are in title case.
Change it to title case to match the rest of the changelog:- ## ongoing + ## Ongoing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
5-5: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Ruff check and force
bouwew
left a comment
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.
LGTM!
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CHANGELOG.md (4)
3-4: Consider standardizing the unreleased section header
Using a common changelog convention like## Unreleased(instead of lowercaseongoing) may improve clarity.
Apply this diff:-## ongoing +## Unreleased
5-5: Fix casing and grammar in the new entry
The term “bugFix” is awkwardly capitalized and the description reads better with “as unsigned int.”- - PR [#261](https://github.com/plugwise/python-plugwise-usb/pull/261): Sense: bugFix parsing of humidity value should be unsigned int + - PR [#261](https://github.com/plugwise/python-plugwise-usb/pull/261): Sense: Bugfix parsing of humidity value as unsigned int
17-17: Add missing comma before “and”
These are two independent clauses; a comma improves readability:- - PR [#258](https://github.com/plugwise/python-plugwise-usb/pull/258): Sense: make sure NodeFeature.BATTERY is called and configuration parameters are propagated properly + - PR [#258](https://github.com/plugwise/python-plugwise-usb/pull/258): Sense: make sure NodeFeature.BATTERY is called, and configuration parameters are propagated properly
21-21: Align product name casing with project convention
The repo uses “Plugwise” (lowercase “w”), so update for consistency:- - PR [#256](https://github.com/plugwise/python-plugwise-usb/pull/256): Implement PlugWise Sense product + - PR [#256](https://github.com/plugwise/python-plugwise-usb/pull/256): Implement Plugwise Sense product
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~17-~17: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... make sure NodeFeature.BATTERY is called and configuration parameters are propagated...
(COMMA_COMPOUND_SENTENCE_2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Check commit
- GitHub Check: Check commit
🔇 Additional comments (1)
CHANGELOG.md (1)
9-9: Formatting is consistent here
The v0.44.3 entry follows the established pattern with a colon after the PR reference.
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
17-17: Add comma before “and” for clarity
This improves readability by separating two independent clauses.- PR [#258](https://github.com/plugwise/python-plugwise-usb/pull/258): Sense: make sure NodeFeature.BATTERY is called and configuration parameters are propagated properly + PR [#258](https://github.com/plugwise/python-plugwise-usb/pull/258): Sense: make sure NodeFeature.BATTERY is called, and configuration parameters are propagated properly
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~17-~17: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... make sure NodeFeature.BATTERY is called and configuration parameters are propagated...
(COMMA_COMPOUND_SENTENCE_2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check commit
🔇 Additional comments (3)
CHANGELOG.md (3)
3-6: Add an “Ongoing” section for PR #261
Good addition to highlight the in-progress humidity sign bugfix. Formatting and link look correct.
9-9: Consistent formatting for v0.44.3 entry
The PR reference and description follow the existing pattern.
21-21: Verify v0.44.0 entry consistency
The entry matches the format used for other PR listings.



Summary by CodeRabbit