-
Notifications
You must be signed in to change notification settings - Fork 809
Add temperature and humidity offset to Sonoff SNZB-02D #3989
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
dc10e56
to
e4b396e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #3989 +/- ##
==========================================
+ Coverage 91.20% 91.24% +0.04%
==========================================
Files 335 338 +3
Lines 10889 10919 +30
==========================================
+ Hits 9931 9963 +32
+ Misses 958 956 -2 ☔ 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.
Also, any issues with this PR still, since it's still in draft? Seems like it should work for the most part.
zhaquirks/sonoff/snzb02d.py
Outdated
min_value=-50, | ||
max_value=50, | ||
step=0.1, | ||
unit=TemperatureUnit, |
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.
You should not use the enum TemperatureUnit
for the unit
— it's expecting a str
.
The offset is likely in °C
, so use UnitOfTemperature.CELSIUS
instead.
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.
Updated.
min_value=-50, | ||
max_value=50, | ||
step=0.1, | ||
unit=PERCENTAGE, |
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.
Hmm, technically, the unit for the calibration won't be a percentage, but percentage points.
Don't really think HA uses pp
as a unit for entities like this though. I do see at least one HA entity using "%", so this is likely fine.
Nop, sorry, I just been busy with life. I will try to wrap up this PR this week. |
e2fb02d
to
2464b29
Compare
PTAL. |
93a0a57
to
48164c8
Compare
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.
Hopefully the last thing, I think we're good otherwise.
I can also add it tomorrow or so (hopefully before the beta release) if you don't get around to it.
zhaquirks/sonoff/snzb02d.py
Outdated
.number( | ||
CustomSonoffCluster.AttributeDefs.temperature_calibration.name, | ||
CustomSonoffCluster.cluster_id, | ||
min_value=-50, | ||
max_value=50, | ||
step=0.1, | ||
unit=UnitOfTemperature.CELSIUS, | ||
multiplier=0.01, | ||
translation_key="temperature_calibration", | ||
fallback_name="Temperature calibration", | ||
) | ||
.number( | ||
CustomSonoffCluster.AttributeDefs.humidity_calibration.name, | ||
CustomSonoffCluster.cluster_id, | ||
min_value=-50, | ||
max_value=50, | ||
step=0.1, | ||
unit=PERCENTAGE, | ||
multiplier=0.01, | ||
translation_key="humidity_calibration", | ||
fallback_name="Humidity calibration", | ||
) |
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.
There's also NumberDeviceClass.TEMPERATURE
and NumberDeviceClass.HUMIDITY
for device classes.
I believe using the TEMPERATURE
device class would cause the temperature entity to be converted to °F if using that, instead of °C.
I think we should set these device classes for calibration entities too, though we apparently don't set device classes for all existing ZHA number entities (and Tuya v2 quirks config entities?).
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.
Added with ef69b21
(#3989), for all temperature/humidity entities.
I've also changed the temperature/humidity "calibration" wording to "offset", as that's what we use in a similar Telink quirk. As I've also renamed the attribute name (upon which the unique id is based on by default), you'll likely get new entities if you were using the previous version of this quirk as a custom quirk. |
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.
I think this should be good to go now. Thanks for the PR!
Sorry, I missed your comments and suggestions. Thank you for doing those changes and merging the PR. |
Proposed change
Add temperature and humidity calibration to Sonoff SNZB-02D.
Additional information
N/A.
Checklist
pre-commit
checks pass / the code has been formatted using Black