Report states #11973
Merged
Report states #11973
Conversation
This file contains 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
houndci-bot
reviewed
Jan 27, 2018
| assert msg['header']['name'] == 'Discover.Response' | ||
| assert msg['header']['namespace'] == 'Alexa.Discovery' | ||
|
|
||
| endpoint_ids = set(appliance['endpointId'] for appliance in msg['payload']['endpoints']) |
There was a problem hiding this comment.
line too long (92 > 79 characters)
| if not self.properties_retrievable(): | ||
| return | ||
|
|
||
| properties = [] |
There was a problem hiding this comment.
local variable 'properties' is assigned to but never used
bitglue
force-pushed
the
report_states
branch
from
6777b2b
to
f86ba58
Compare
bitglue
force-pushed
the
report_states
branch
from
f86ba58
to
74a5f17
Compare
bitglue
force-pushed
the
report_states
branch
from
74a5f17
to
7061e39
Compare
bitglue
force-pushed
the
report_states
branch
from
7061e39
to
cc11ae0
Compare
bitglue
force-pushed
the
report_states
branch
from
cc11ae0
to
ea14e5f
Compare
|
No pylint, that function doesn't only return None. If you're so good at programming, why don't you open the PRs? |
Having an object per interface will make it easier to support properties. Ideally, properties are reported in context in all responses. However current implementation reports them only in response to a ReportState request. This seems to work sufficiently. As long as the device is opened in the Alexa app, Amazon will poll the device state every few seconds with a ReportState request.
bitglue
force-pushed
the
report_states
branch
from
ea14e5f
to
58ed62a
Compare
3 tasks
Fixes (mostly) home-assistant#11874. Other interfaces will need properties implemented as well. Implementing properties for just PowerController seems sufficient to eliminate the "There was a problem." error for any device that supports it, even if other interfaces are supported. Of course the additional properties will be reported incorrectly in the Alexa app. Includes a minor bugfix: `reportable` was previously placed incorrectly in the responses, so Amazon was ignoring it.
bitglue
force-pushed
the
report_states
branch
from
58ed62a
to
5eae9d1
Compare
balloob
approved these changes
Jan 28, 2018
Merged
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:
This depends on #12016. For review purposes the first commit in this PR can be ignored, since it's in #12016.
This reports properties for PowerController, LockController, and BrightnessController. That should cover most use cases. It covers all the devices I have, at least 馃槃
I did some fairly extensive refactoring to make the implementation of properties more readable. Suggest reviewing commits individually. I'd like to refactor the tests some more as well since there's a lot of copy pasta there, but that can wait for another day.
Related issue (if applicable): fixes #11874
Checklist:
toxrun successfully. Your PR cannot be merged unless tests pass