Zwave panel api#7456
Conversation
balloob
left a comment
balloob
left a comment
There was a problem hiding this comment.
Awesome start! Got some minor comments but it looks great.
There was a problem hiding this comment.
This should be debug at best, probably better to just remove it?
There was a problem hiding this comment.
No need to use keys(), that's the default. Just use for key in groupdata:
There was a problem hiding this comment.
Are you sure you need to do this? What are the values in groupdata ?
Also, if you need to cast node_id to int, please do it once at the top.
There was a problem hiding this comment.
It's not the desired output. It's missing the max_associations so it needs to be done to get that.
There was a problem hiding this comment.
Change it to {node_id:\d+} to only match numbers.
There was a problem hiding this comment.
This shouldn't be logged as info.
There was a problem hiding this comment.
Use a guard clause
if not node.has_command_class(…):
return self.json_message(…)
for value in …
There was a problem hiding this comment.
'tests.common.mock_http_component_app' imported but unused
There was a problem hiding this comment.
local variable 'mock_network' is assigned to but never used
There was a problem hiding this comment.
local variable 'mock_network' is assigned to but never used
There was a problem hiding this comment.
local variable 'mock_network' is assigned to but never used
There was a problem hiding this comment.
'tests.mock.zwave.MockNode' imported but unused
There was a problem hiding this comment.
'homeassistant.setup.async_setup_component' imported but unused
There was a problem hiding this comment.
'unittest.mock.patch' imported but unused
There was a problem hiding this comment.
local variable 'ZWaveGroup' is assigned to but never used
|
So this is a good start for tests, however, the tests don't test anything yet?? It expects an empty dictionary? |
There was a problem hiding this comment.
Why fetch the groups as dict, then iterate over the keys and construct group objects again?
Seems like node.groups will give you a list of groups too? https://github.com/OpenZWave/python-openzwave/blob/master/src-api/openzwave/node.py#L289
|
Updated one of your tests to show how to fetch data. |
| result = yield from resp.json() | ||
|
|
||
| assert result == {'message': 'Node not found'} | ||
|
|
There was a problem hiding this comment.
local variable 'network' is assigned to but never used
There was a problem hiding this comment.
local variable 'value' is assigned to but never used
There was a problem hiding this comment.
closing bracket does not match visual indentation
There was a problem hiding this comment.
closing bracket does not match visual indentation
There was a problem hiding this comment.
local variable 'value' is assigned to but never used
There was a problem hiding this comment.
continuation line under-indented for visual indent
There was a problem hiding this comment.
continuation line under-indented for visual indent
There was a problem hiding this comment.
continuation line under-indented for visual indent
There was a problem hiding this comment.
continuation line under-indented for visual indent
There was a problem hiding this comment.
continuation line under-indented for visual indent
There was a problem hiding this comment.
redefinition of unused 'test_get_groups' from line 11
There was a problem hiding this comment.
redefinition of unused 'test_get_groups' from line 11
|
🎉 🐬 ❤️ |
5 participants
Description:
API to the zwave-panel
Related issue (if applicable): fixes #
home-assistant/frontend#260