-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
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
Conversation
# 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
@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: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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 "IR Code Learning" 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"
],
There was a problem hiding this comment.
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?
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 |
There was a problem hiding this comment.
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
Cherry picked for 0.45 |
* # 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
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