home-assistant / addons

:heavy_plus_sign: Docker add-ons for Home Assistant
https://home-assistant.io/hassio/
Apache License 2.0
1.51k stars 1.46k forks source link

OZW: set_config_parameter not working when node has no manufacturer specfic xml? #1549

Closed wauswaus closed 4 years ago

wauswaus commented 4 years ago

The problem

I am new to Home Assistant and therefore playing a lot with. I installed the OpenZWave (beta) v0.5.2 integration and it works great. I also have a new Neo Coolcam PIR motion sensor for which no config xml exists yet (manufacturer id=0258, type=0200, id=1031). In OZWAdmin the tab 'Config values' is empty. No problem, as there is the set_config_parameter service i thought which I tried using from the Developer tools page with data: node_id: 20 (the PIR sensor node ID) parameter: 2 value: 10

And then.... nothing happens. I looked in the OZWAdmin and i don't see any commands being send. Looked in the logs of HA, nothing there. When I do the same for a node which has valid config values in OZWAdmin (tried for a Fibaro dimmer 2, force the calibration which is parameter 13, value 1) I get the following in the HA log:

2020-08-26 15:03:53 ERROR (MainThread) [homeassistant.components.ozw.services] Value 1 out of range for parameter 13 (Min: 0 Max: 0)

I was hoping that this service would allow me to change values even if there is no xml file for the device.

When I use ozw.add_node, the controller goes into inclusion mode, so all good there.

Environment

Problem-relevant configuration

If you need any I will provide, no idea what to add here.

Traceback/Error logs

Additional information

MartinHjelmare commented 4 years ago

We're you able to complete the inclusion of this node?

wauswaus commented 4 years ago

I did not actually add a node. However, the USB z-wave controller did start blinking its LED like it normally does when I add a node via OZWAdmin.

kpine commented 4 years ago

I was hoping that this service would allow me to change values even if there is no xml file for the device.

ozwdaemon does not provide an API to set unknown configuration parameters, so there is nothing HA can do for this.

So an XML file is required at this time. If you require this capability you would need to submit a request to the qt-openzwave project, and if implemented there, then get support added in HA core.

2020-08-26 15:03:53 ERROR (MainThread) [homeassistant.components.ozw.services] Value 1 out of range for parameter 13 (Min: 0 Max: 0)

This is an unrelated problem. Is this for a FGD-212?

If yes, then first off you can probably workaround the problem by specifying a label instead of the list index.

node: <node id>
parameter: 13
value: "Start auto-calibration of the load without Fibaro Bypass"

Let us know if that works.

The problem here is that parameter 13 is a list parameter. In HA, a list parameter can be set by either the list index (e.g. 1 as you were trying) or the list value label string. When using the index, HA will validate it against the min and max values. Well, there's arguably a bug in the XML definition for this parameter:

    <Value genre="config" index="13" instance="1" label="Force auto-calibration" size="1" type="list" value="0">

If you notice, there is no max or min value defined. I am assuming that what happens is the min and max are therefore defaulted to 0 (you could confirm by posting the MQTT data for the config param). If both min and max are 0, the range check would fail for any value not 0.

The OZW wiki page for creating config parameters lists min/max as a required (not really) parameter. If you want to help yourself and others, you could correct that XML file and add min/max values, and that would fix your problem.

The addon does not make it easy to add custom XML files though, so you're kind of screwed there.

Here is an example of a properly defined list config parameter:

    <Value genre="config" index="1" label="Temperature Reporting Threshold" max="4" min="0" size="1" type="list" units="" value="2">

Notice min and max are valid values.

The OZW integration though could easily figure out whether the index value is valid or not by comparing to the list of values it knows about, instead of relying on the min/max values. This also assumes ozwdaemon is not checking min/max itself (which you would confirm by using the label instead).

I would say there are two possible fixes for this one:

  1. The root problem should be fixed by updating the XML file upstream at OpenZWave.
  2. The OZW integration could not check min/max and search the known list index values for the given index.

Neither of these issues are related to the addon (unless upstream code is changed, requiring a version update).

wauswaus commented 4 years ago

Thanks for your elaborate explanation. I might send a request to the OpenZWave project for this as you proposed. As you already mentioned, it is not easy to make custom XML files and test them (to be honest, I don't even know what and where if I wanted to, no Linux hero here). So being able to change config parameters without a file would be really nice to have, else you are indeed screwed and you end up with a device of which you cannot change settings :-( However, It might be a good idea to mention this behaviour on the OpenZWave integration page (https://www.home-assistant.io/integrations/ozw/) so noobs like me are aware of this.

With regards to the FGD-212, I just noticed this when testing the set_config_parameter service so for me it is not an issue. It all works fine when you change the values from the OZWAdmin. I wonder if a lot of people change settings for this device dynamically from an automation. I will leave it up to you guys if you want todo something with this, Else I think the issue can be closed.

MartinHjelmare commented 4 years ago

We want the device specific code to go as much upstream as possible, ie this device support should be fixed in OpenZWave.

Thanks for the report!

kpine commented 4 years ago

Good luck with that.

$ git grep 'Value.*genre=\"config\"' -- '*.xml' | grep 'type=\"list\"' | wc -l
2499

$ git grep 'Value.*genre=\"config\"' -- '*.xml' | grep 'type=\"list\"' | grep -E -v '(min.*max|max.*min)' | wc -l
688
MartinHjelmare commented 4 years ago

@kpine Sorry, I'm not following your latest post. What do you mean?

kpine commented 4 years ago

My concern was that 27% (688 out of 2499) of the list configuration parameters don't set a min and max, which is a considerable amount.

Fortunately it's a moot point now as the requirement that list values are within in the range of min/max was removed in a (loosely related) PR slated for 0.115.

https://github.com/home-assistant/core/commit/597d1e2799b398b7f2df7f5bb28104c5a7cf3476#diff-bf5e3c44633c200ae03702929633c83a