-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
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
added smappee component #11491
added smappee component #11491
Conversation
… credentials Added coverage omit
Fixed pylint errors and a few use cases when starting up with invalid…
Added a few more sensors Added more error handling Better parsing and debug message
Added support to run only locally
* dev: upgrade schiene to 0.20 (#11504) Upgrade pywebpush to 1.5.0 (#11497) Fix time functions would throw errors in python scripts (#11414) Clean up Alexa.intent and DialogFlow.intent (#11492) Add missing configuration variables (#11390) Add new iGlo component (#11171) Upgrade pysnmp to 4.4.4 (#11485)
* dev: Address missed review comments for Dark Sky weather (#11520) Fix canary flaky test (#11519) Lazy loading of service descriptions (#11479) Add Dark Sky weather component (#11435) Snips (new) added speech response, parse snips/duration (#11513) More tolerant KNX component if gateway cant be connected (#11511) timer: include the remaining time in the state attributes (#11510)
* Merged with local version * Updated smappy library with the patched one Fixed lint, added merge missing param Fixed missing run for requirements_all.txt Fixed lint * Fixed on/off based on library. Reverted change used for testing stacktrace * Fixed switches to work with both remote and local active Fixed lint Fixed switches Fixed lint
homeassistant/components/smappee.py
Outdated
@@ -0,0 +1,290 @@ | |||
""" | |||
Support for Digital Ocean. |
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.
Shouldn't this be Smappee
* dev: (85 commits) Bump dev to 0.62.0.dev0 (#11652) Add Deconz support for Zigbee green power devices like Hue Tap (#11455) Update Pyarlo to 0.1.2 (#11626) Use kelvin/mireds correctly for setting iglo white (#11622) patch stop command (#11612) Upgrade yarl to 0.18.0 (#11609) Xiaomi lib upgrade (#11603) Bugfix and cleanup for Rfxtrx (#11600) Concord232 alarm arm away fix (#11597) Snips add say and say_actions services (new) (#11596) Fix state for trigger with forced updates (#11595) Pushbullet email support (fix) (#11590) Fix Tahoma stop command for 2 types of shutters (#11588) Core support for hass.io calls & Bugfix check_config (#11571) Avoid returning empty media_image_url string (#11557) Upgrade coinmarketcap to 4.1.2 (#11634) Update CODEOWNERS Add templates to MQTT climate (#11623) upgrade xiaomi lib (#11629) Fixes and enhancements for the Tahoma platform (#11547) ...
+1 |
def update(self): | ||
"""Get the latest data from Smappee and update the state.""" | ||
if self._sensor == 'alwaysOn': | ||
data = self._smappee.get_consumption( |
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.
This is doing a call to the API per sensor per update (every 30 seconds!). Any way you can fetch all data at once?
self._state = round(power, 2) | ||
self._timestamp = datetime.now().strftime("%Y-%m-%d %H:%M:%S") | ||
else: | ||
return None |
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.
Remove this. Does nothing.
homeassistant/components/smappee.py
Outdated
"""Get switches from local Smappee.""" | ||
if self.is_local_active: | ||
return self._localsmappy.load_command_control_config() | ||
return False |
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.
Inconsistent return type
homeassistant/components/smappee.py
Outdated
_LOGGER.error( | ||
"Error getting comsumption from Smappee cloud. (%s)", | ||
error) | ||
return False |
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.
Inconsistent return type
self._timestamp = datetime.now().strftime("%Y-%m-%d %H:%M:%S") | ||
self._state = round(consumption.get('solar') / 1000, 2) | ||
elif self._sensor == 'power_today': | ||
data = self._smappee.get_consumption( |
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 is no error handling being done here. This method can return False.
homeassistant/components/smappee.py
Outdated
# 3 = daily values, | ||
# 4 = monthly values, | ||
# 5 = quarterly values | ||
if self.is_remote_active: |
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.
Use guard clause
if not self.is_remote_active:
return
end = …
homeassistant/components/smappee.py
Outdated
# 3 = daily values, | ||
# 4 = monthly values, | ||
# 5 = quarterly values | ||
if self.is_remote_active: |
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.
guard clause
homeassistant/components/smappee.py
Outdated
error) | ||
return False | ||
|
||
return False |
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.
These error values are never handled. Probably better to propagate the error to the update method of the sensor.
homeassistant/components/smappee.py
Outdated
self._smappy.actuator_on(location_id, actuator_id, duration) | ||
else: | ||
self._localsmappy.on_command_control(actuator_id) | ||
return True |
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.
Inconsistent return
homeassistant/components/smappee.py
Outdated
|
||
def instantaneous_values(self): | ||
"""ReportInstantaneousValues.""" | ||
if self.is_local_active: |
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.
guard clause
# Skip local switches if remote available | ||
if not smappee.is_remote_active: | ||
local_devices = smappee.local_devices | ||
if smappee.is_local_active: |
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.
Merge with parent if statement.
|
||
def turn_on(self, **kwargs): | ||
"""Turn on Comport Plug.""" | ||
self._smappee.actuator_on(self._location_id, self._switch_id, |
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.
Why do this twice?
self._remoteswitch) | ||
self._smappee.actuator_on(self._location_id, self._switch_id, | ||
self._remoteswitch) | ||
self._state = True |
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.
Why are you setting state to True? You can only do that if you check the command result and make sure the command was successful.
Also, Home Assistant calls your update method after calling a service, which will also query your device
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.
The plugs don't have any way of communicating with the rest of the smappee system, they work by just sending a signal out and then hoping it is received.
It's not really very nice so setting the state seemed to be the only way I could handle them, if theres a better way to handle switches where you can't get the status just point me in that direction and I'll look into it
Please include a link to the docs PR. |
Docs PR is here home-assistant/home-assistant.io#4354 |
- update smappy module - cache cloud api requests - added actuator info - updated return states
Thanks for al of the feedback, I tried fixing all of it, but I'm not exactly a python expert, so I hope it's better :) |
Awesome work! 🎉 |
* added smappee component * Fixed pylint errors and a few use cases when starting up with invalid credentials Added coverage omit * Added support to run only locally Added a few more sensors Added more error handling Better parsing and debug message * fixed smappee switch after local/remote support was added * Smappee - update switches for local support (#3) * Merged with local version * Updated smappy library with the patched one Fixed lint, added merge missing param Fixed missing run for requirements_all.txt Fixed lint * Fixed on/off based on library. Reverted change used for testing stacktrace * Fixed switches to work with both remote and local active Fixed lint Fixed switches Fixed lint * nothing to update per switch as the states are not saved by smappee system * added better error handling for communication errors with smappee * fixed lint errors * fixed comment * fixed lint error * fixed lint error * update smappee module with reviewer comments - update smappy module - cache cloud api requests - added actuator info - updated return states
Description:
Added components for https://www.smappee.com/
Example entry for
configuration.yaml
(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTS
variable ([example][ex-requir]).requirements_all.txt
by runningscript/gen_requirements_all.py
.