8000 Final tweaks for Zwave panel by turbokongen · Pull Request #7652 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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.

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
merged 12 commits into from
May 19, 2017
Merged

Conversation

turbokongen
Copy link
Contributor

Description:

Modifies the set_config parameter to allow setting the string of the parameter instead of the index.
This will resolve #7608
In case of wrong value nothing is set for the parameter. Internal checks in OZW handles this. The behaviour is the following: Node does not respond at all to the command.
This is the last tweaks to get the zwave panel fully operational.

Related issue (if applicable): fixes #7608

# The first commit's message is:
Add seperate zwave panel

# The 2nd commit message will be skipped:

#	unused import

# The 3rd commit message will be skipped:

#	Use get for config
@mention-bot
Copy link

@turbokongen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @balloob and @andrey-git to be potential reviewers.

_LOGGER.info("Setting config parameter %s on Node %s "
"with selection %s and size=%s", param, node_id,
selection, size)
if value.index != param:
Copy link
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.

Copy link
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.

Copy link
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) ?

Copy link
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"
],

Copy link
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?

@turbokongen
Copy link
Contributor Author

Is this a viable solution @andrey-git ? 75b29ba

_LOGGER.info("Setting config list parameter %s on Node %s "
"with selection %s", param, node_id,
selection)
break
Copy link
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

@balloob balloob added this to the 0.45 milestone May 18, 2017
@balloob balloob merged commit 88ffe39 into home-assistant:dev May 19, 2017
@balloob
Copy link
Member
balloob commented May 19, 2017

Cherry picked for 0.45

balloob pushed a commit that referenced this pull request May 19, 2017
* # This is a combination of 3 commits.
# The first commit's message is:
Add seperate zwave panel

# The 2nd commit message will be skipped:

#	unused import

# The 3rd commit message will be skipped:

#	Use get for config

* Add seperate zwave panel

* Modify set_config_parameter to accept setting string values

* descriptions

* Tweaks

* Tweaks

* Tweaks

* Tweaks

* lint

* Fallback if no config parameteres are available

* Update services.yaml

* review changes
@balloob balloob mentioned this pull request Jun 2, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Z-wave service component sends wrong value for parameter
5 participants
0