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
BedJet: expose the outlet temperature on the climate and as a sensor #6633
Conversation
Hey there @jhansche, mind taking a look at this pull request as it has been labeled with an integration ( |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #6633 +/- ##
==========================================
+ Coverage 53.70% 54.21% +0.50%
==========================================
Files 50 50
Lines 9408 9594 +186
Branches 1654 1691 +37
==========================================
+ Hits 5053 5201 +148
- Misses 4056 4069 +13
- Partials 299 324 +25 ☔ View full report in Codecov by Sentry. |
a118af2
to
f6bada9
Compare
This is still a draft - I accidentally hit the button early. Sorry for the noise! |
18fe40a
to
971a04c
Compare
Alright this is ready for review! |
Hey there @jhansche, mind taking a look at this pull request as it has been labeled with an integration ( |
891e76a
to
bcef3cf
Compare
bcef3cf
to
b651022
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.
Thank you for this! The code looks good - with a minor suggestion for reducing code duplication.
5bf37e4
to
077bd0e
Compare
@jhansche - responded and pushed changes, let me know if you have any more questions! |
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.
Nice, and thanks for the PR!
Just FYI, I can't approve it for merge, but looks good
No worries @jhansche, I assume @jesserockz will be along soon enough to click the button. Thanks for taking a look at this! (And thanks for writing this awesome component in the first place 😄) |
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Hey there @jhansche, mind taking a look at this pull request as it has been labeled with an integration ( |
b310cc9
to
89ddcd9
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.
LGTM
@javawizard not sure if Jesse filters his view, but it might help to dismiss/re-request review to show that you've made the changes
f4fb6ed
to
a98fbb4
Compare
Hey there @jhansche, mind taking a look at this pull request as it has been labeled with an integration ( |
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
What does this implement/fix?
bedjet
climate platform now takes atemperature_source
option that can be used to tell it whether to use the temperature of the air being discharged by the BedJet or the ambient temperature of the room it's in as the source of the climate entity's current temperaturesensor
platform entitiesbedjet_id:
option is now optional on the climate entity (and the newly-introduced sensor entities) when only onebedjet:
hub is definedTypes of changes
Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#3799
Test Environment
Example entry for
config.yaml
:Checklist:
tests/
folder).If user exposed functionality or configuration variables are added/changed: