Adds energy monitoring capabilities to the TP-Link HS110 #39 8000 17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Energy monitoring works only on the HS110 model
|
@kirichkov, thanks for your PR! By analyzing the history of the files in this pull request, we identified @GadgetReactor, @robbiet480 and @balloob to be potential reviewers. |
|
Two notes to make:
|
|
Your fork introduces alot of mixed indentation and other style issues. |
|
It looks like the original author has responded to your PR. Let's await his merging of your PR before we continue with this PR. @GadgetReactor, since you are familiar with this code, would you mind reviewing this PR? |
|
My updates were merged into GadgetReactor's and most of the styling errors in the module should be resolved. @mweinelt can you tell me what lint settings you're using, because I got different output when using the HASS pylintrc against the pyHS100/pyHS100.py codebase. |
|
From my end, the code looks good and tested with the HS100 with no issues. |
|
@kirichkov thats just the flake8 default. |
|
@balloob any possibility of getting this in for 0.31? |
balloob
requested changes
Oct 21, 2016
| """Return the state attributes of the device.""" | ||
| _LOGGER.debug("Updating TP-Link energy meter data") | ||
|
|
||
| emeter_readings = self.smartplug.get_emeter_realtime() |
There was a problem hiding this comment.
You are not allowed to fetch data from an entity inside a property. Instead you should cache this data inside the update method.
There was a problem hiding this comment.
I am not sure if self.smartplug.state is also fetching data, but if it is, it too should be moved to update.
There was a problem hiding this comment.
Indeed, self.smartplug.state is used to fetch data from the plug. I moved everything to update and the state and the meter's values are now stored as class variables.
balloob
approved these changes
Oct 22, 2016
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description:
Adds energy monitoring to the TP-Link HS110 Smart switches
Related issue (if applicable):
Pull request in home-assistant.github.io with documentation (if applicable):
Example entry for
configuration.yaml(if applicable):No changes to the configuration are needed. The updated module should automatically detect whether the switch's model and determine whether energy monitoring is available (HS110) or not (HS100)
Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests passEnergy monitoring works only on the HS110 model