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.
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
Add initial support for SwitchBot relay switch #130863
Add initial support for SwitchBot relay switch #130863
Changes from 3 commits
e581d25
60f1e83
4b1cb67
eb2b5de
87a706d
9f49927
b29a7e4
74bf688
dab6e8c
d74f14f
239bb00
6dd242a
8fabc07
082a64e
4a750d7
6101335
690dbfe
54a2689
ffd1196
efb6ff9
56478df
6c46cde
7aaa1a6
062634f
fb5e040
9c4bab3
45e30ba
47ddf59
9904851
ca5bae7
c42faf7
cd9bbfa
5edb5e7
07bca62
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 you can use
return self.async_abort(
instead hereThere 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.
Thanks,already changed to
return self.async_abort(
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 your feedback. I have made the requested changes and addressed the issues. Please review again.
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 bump the dependency in a preliminary PR
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 have added a link to the changelog in the PR description.
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.
But can you please open a prelimary PR where we bump the version? Check point 7 at https://developers.home-assistant.io/docs/review-process#creating-the-perfect-pr for the explanation behind it :)
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.
Sorry, I opened a prelimary PR here. #130869
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 your feedback. I have made the requested changes and addressed the issues. Please review again.
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.
In what unit of measurement is this the most usable? Because if this always measures 100V or 230V, we should not have the shown unit of measurement be millivolts. Rather we should set the
suggested_unit_of_measurement
toVOLT
, so it will automatically come up like thatThere 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 your valuable advice, I have fixed this problem. I also created a PR to bump the PySwitchbot at #131783. Could you please review the updated code at your earliest convenience?