Skip to content

Final tweaks for Zwave panel #7652

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.

Sign up for GitHub

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

Merged
balloob merged 12 commits into from
May 19, 2017
Merged

Final tweaks for Zwave panel #7652

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d70ab95
# This is a combination of 3 commits.
turbokongen Apr 13, 2017
f9215b9
Add seperate zwave panel
turbokongen Apr 13, 2017
2e57aec
Modify set_config_parameter to accept setting string values
turbokongen May 16, 2017
b38ca84
descriptions
turbokongen May 18, 2017
aee711e
Tweaks
turbokongen May 18, 2017
8684e1c
Tweaks
turbokongen May 18, 2017
4813613
Tweaks
turbokongen May 18, 2017
089b7e8
Tweaks
turbokongen May 18, 2017
7f4680f
lint
turbokongen May 18, 2017
75b29ba
Fallback if no config parameteres are available
turbokongen May 18, 2017
59d6012
Update services.yaml
turbokongen May 18, 2017
29f7d25
review changes
turbokongen May 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 15 additions & 21 deletions homeassistant/components/zwave/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@
SET_CONFIG_PARAMETER_SCHEMA = vol.Schema({
vol.Required(const.ATTR_NODE_ID): vol.Coerce(int),
vol.Required(const.ATTR_CONFIG_PARAMETER): vol.Coerce(int),
vol.Required(const.ATTR_CONFIG_VALUE): vol.Coerce(int),
vol.Optional(const.ATTR_CONFIG_SIZE): vol.Coerce(int)
vol.Required(const.ATTR_CONFIG_VALUE): vol.Any(vol.Coerce(int), cv.string),
})
PRINT_CONFIG_PARAMETER_SCHEMA = vol.Schema({
vol.Required(const.ATTR_NODE_ID): vol.Coerce(int),
Expand Down Expand Up @@ -410,28 +409,23 @@ def set_config_parameter(service):
node = network.nodes[node_id]
param = service.data.get(const.ATTR_CONFIG_PARAMETER)
selection = service.data.get(const.ATTR_CONFIG_VALUE)
size = service.data.get(const.ATTR_CONFIG_SIZE, 2)
i = 0
for value in (
node.get_values(class_id=const.COMMAND_CLASS_CONFIGURATION)
.values()):
if value.index == param and value.type == const.TYPE_LIST:
_LOGGER.debug("Values for parameter %s: %s", param,
value.data_items)
i = len(value.data_items) - 1
if i == 0:
node.set_config_param(param, selection, size)
else:
if selection > i:
_LOGGER.error("Config parameter selection does not exist! "
"Please check zwcfg_[home_id].xml in "
"your homeassistant config directory. "
"Available selections are 0 to %s", i)
return
node.set_config_param(param, selection, size)
_LOGGER.info("Setting config parameter %s on Node %s "
"with selection %s and size=%s", param, node_id,
selection, size)
if value.index != param:

@andrey-git andrey-git May 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why switch to direct value manipulation instead of node.set_config_param ?
node.set_config_param supports setting params not "known" to ozw via the device config.

While this option is not needed for UI panel it is still useful as a service call.

@turbokongen turbokongen May 18, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It might be a usefull service call to have, but my opinion is:
The device should rather be added to OZW than we supporting making changes on an unknown device.

@andrey-git andrey-git May 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is there an upside of calling value.data = selection as opposed to node.set_config_param(param, selection, size) ?

@turbokongen turbokongen May 18, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes readability:
Take issue #7608
The provided "value" by the user, does not correspond to the actual value in OZW.
OZW value always corresponds to index within the list of options. But there is no way to retrieve that value from ozw. It always respond with the data or data_as_string.
When retrieving the data_items, it is not sorted in any way, so:
from zwcfg file:

				<Value type="list" genre="config" instance="1" index="25" label="Indicate a location for IR code learning and start learning" units="" read_only="false" write_only="true" verify_changes="false" poll_intensity="0" min="0" max="22" vindex="0" size="1">
					<Help>In case none of the code on the code list works for the targeted air conditioner, user can use IR code learning function. See manual at section &quot;IR Code Learning&quot; for a description of the procedure. Value 0-22</Help>
					<Item label="OFF" value="0" />
					<Item label="ON (resume)" value="1" />
					<Item label="cool 19 C" value="2" />
					<Item label="cool 20 C" value="3" />
					<Item label="cool 21 C" value="4" />
					<Item label="cool 22 C" value="5" />
					<Item label="cool 23 C" value="6" />
					<Item label="cool 24 C" value="7" />
					<Item label="cool 25 C" value="8" />
					<Item label="cool 26 C" value="9" />
					<Item label="cool 27 C" value="10" />
					<Item label="cool 28 C" value="11" />
					<Item label="heat 19 C" value="12" />
					<Item label="heat 20 C" value="13" />
					<Item label="heat 21 C" value="14" />
					<Item label="heat 22 C" value="15" />
					<Item label="heat 23 C" value="16" />
					<Item label="heat 24 C" value="17" />
					<Item label="heat 25 C" value="18" />
					<Item label="heat 26 C" value="19" />
					<Item label="heat 27 C" value="20" />
					<Item label="heat 28 C" value="21" />

And from the values:

25: {
data: "OFF",
data_items: [
"heat 21 C",
"cool 23 C",
"cool 25 C",
"cool 27 C",
"heat 22 C",
"cool 21 C",
"heat 25 C",
"ON (resume)",
"heat 27 C",
"cool 20 C",
"heat 20 C",
"heat 28 C",
"heat 26 C",
"cool 22 C",
"cool 26 C",
"cool 28 C",
"heat 24 C",
"cool 19 C",
"heat 23 C",
"cool 24 C",
"heat 19 C",
"OFF"
],

@andrey-git andrey-git May 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sound good.

Though could you bring back node.set_config_param(param, selection, size) in case the for loop didn't find a match?

continue
if value.type in [const.TYPE_LIST, const.TYPE_BOOL]:
value.data = selection
_LOGGER.info("Setting config list parameter %s on Node %s "
"with selection %s", param, node_id,
selection)
break

@andrey-git andrey-git May 18, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just return here (and below) instead of break. Then you won't need param_set

else:
value.data = int(selection)
_LOGGER.info("Setting config parameter %s on Node %s "
"with selection %s", param, node_id,
selection)
break

def print_config_parameter(service):
"""Print a config parameter from a node."""
Expand Down
4 changes: 1 addition & 3 deletions homeassistant/components/zwave/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ set_config_parameter:
parameter:
description: Parameter number to set (integer).
value:
description: Value to set on parameter. (integer).
size:
description: (Optional) The size of the value. Defaults to 2.
description: Value to set for parameter. (String value for list and bool parameters, integer for others).

print_config_parameter:
description: Prints a Z-Wave node config parameter value to log.
Expand Down
29 changes: 5 additions & 24 deletions tests/components/zwave/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,7 @@ def test_set_config_parameter(self):
value = MockValue(
index=12,
command_class=const.COMMAND_CLASS_CONFIGURATION,
type=const.TYPE_BYTE,
)
value_list = MockValue(
index=13,
Expand All @@ -911,41 +912,21 @@ def test_set_config_parameter(self):
self.hass.services.call('zwave', 'set_config_parameter', {
const.ATTR_NODE_ID: 14,
const.ATTR_CONFIG_PARAMETER: 13,
const.ATTR_CONFIG_VALUE: 1,
const.ATTR_CONFIG_VALUE: 'item3',
})
self.hass.block_till_done()

assert node.set_config_param.called
assert len(node.set_config_param.mock_calls) == 1
assert node.set_config_param.mock_calls[0][1][0] == 13
assert node.set_config_param.mock_calls[0][1][1] == 1
assert node.set_config_param.mock_calls[0][1][2] == 2
node.set_config_param.reset_mock()
assert value_list.data == 'item3'

self.hass.services.call('zwave', 'set_config_parameter', {
const.ATTR_NODE_ID: 14,
const.ATTR_CONFIG_PARAMETER: 13,
const.ATTR_CONFIG_PARAMETER: 12,
const.ATTR_CONFIG_VALUE: 7,
})
self.hass.block_till_done()

assert not node.set_config_param.called
node.set_config_param.reset_mock()

self.hass.services.call('zwave', 'set_config_parameter', {
const.ATTR_NODE_ID: 14,
const.ATTR_CONFIG_PARAMETER: 12,
const.ATTR_CONFIG_VALUE: 0x01020304,
const.ATTR_CONFIG_SIZE: 4,
})
self.hass.block_till_done()
assert value.data == 7

assert node.set_config_param.called
assert len(node.set_config_param.mock_calls) == 1
assert node.set_config_param.mock_calls[0][1][0] == 12
assert node.set_config_param.mock_calls[0][1][1] == 0x01020304
assert node.set_config_param.mock_calls[0][1][2] == 4
node.set_config_param.reset_mock()

def test_print_config_parameter(self):
"""Test zwave print_config_parameter service."""
Expand Down