zwave-js / node-red-contrib-zwave-js

The most powerful, high performing and highly polished Z-Wave node for Node-RED based on Z-Wave JS. If you want a fully featured Z-Wave framework in your Node-RED instance, you have found it.
MIT License
47 stars 6 forks source link

[Bug]: #279

Closed fabnavigator closed 1 year ago

fabnavigator commented 1 year ago

What happened?

The power reporting threshold max value is 50 in the template, but shows as 5 in the node-red-contrib-zwave-js configuration window. See attached template and configuration screen shots.

template

How to reproduce?

Look at node configuration.

Version

8.1.0

Node-RED Version

3.0.2

What hardware are you seeing the problem on?

Raspberry Pi

Relevant Z-Wave-JS log as a text file

N/A

marcus-j-davies commented 1 year ago

Hi @fabnavigator

its not a bug - this config file was only updated last week - so the one your looking at on Github, is the result of it recently being updated.

See:

https://github.com/zwave-js/node-zwave-js/commit/42c081d320768548b72e5e6e533786473b8ec7ed#diff-1abe48fdc015ac052e52578f2d3032e8be38a778e6f4993c39806d3f52151d71

fabnavigator commented 1 year ago

@marcus-j-davies, I don't understand. In node-red, It says the maximum is 5, but it accepted 50. Are you saying that it's going to fix itself?

marcus-j-davies commented 1 year ago

The Min / Max - is only used so that UI's can create numeric inputs that limits the range - I of course don't (but I should really) @AlCalzone - is that right (given the driver didn't enforce the max value)?

@fabnavigator The config DB your install has, is using a config file for this device, that was only recently updated on the main repo (that includes the correct Max/Min), if there is a new DB (with the changes I linked to) - you can (on the controller node) check for an updated DB.

I released v8.2.0 the other day - so I may have just missed the recent DB updates, therefore try updating the DB from the controller (if a DB update is available with these changes)

fabnavigator commented 1 year ago

I updated to 8.2.0. I checked for a DB update, and there are none. This isn't a big deal. I'm more interested in getting 5 more configuration parameters added to this device. I put in a request to do that, and it is in a waiting for someone to help state.

Am I correct in assuming that the manufacturer doesn't normally do any of these updates to zwave js?

marcus-j-davies commented 1 year ago

Am I correct in assuming that the manufacturer doesn't normally do any of these updates to zwave js?

Some do - but a majority of the configuration DB is contributed by the user community.

marcus-j-davies commented 1 year ago

FWIW - if you wanted to test your work in adding them your self.

{
    "#": "9",
    "$if": "firmwareVersion >= 1.2",
    ...
},
{
    "#": "10",
    "$if": "firmwareVersion >= 1.2",
    ...
},
{
    "#": "11",
    "$if": "firmwareVersion >= 1.2",
    ...
},
{
    "#": "12",
    "$if": "firmwareVersion >= 1.2",
    ...
},
{
    "#": "13",
    "$if": "firmwareVersion >= 1.2",
    ...
}

Screenshot 2023-07-31 at 20 02 09

AlCalzone commented 1 year ago

Our min/max values are meant for the UI, e.g. limiting the range on number input fields. If the value reported by the device is out of range, we don't enforce it to be in the documented range.

So as you already figured out, the config file needs to be updated and/or corrected.

AlCalzone commented 1 year ago

The options property of https://zwave-js.github.io/node-zwave-js/#/config-files/file-format?id=paraminformation

You'll probably want to combine it with "allowManualEntry": false to prevent other values from being entered and turn the UI into a dropdown.

fabnavigator commented 1 year ago

Thanks. I found an example of that and deleted my question, but you wer too fast with an answer.

fabnavigator commented 1 year ago

@AlCalzone, I'm seeing a real mixed bag of words starting with an upper case letter. Looks like in this file at least it is usually the case. Would you prefer "Battery report threshold" or "Battery Report Threshold"?

fabnavigator commented 1 year ago

@marcus-j-davies, Not good. I'm getting 2023-07-31T23:08:02.762Z DRIVER Controller identification returned invalid node ID 0. ANd that's even after I removed the new file and tmeplate.

fabnavigator commented 1 year ago

I needed to reboot the computer. I had to reinterview the node, and now I can see the additional configuration items.

marcus-j-davies commented 1 year ago

@marcus-j-davies, Not good. I'm getting 2023-07-31T23:08:02.762Z DRIVER Controller identification returned invalid node ID 0. ANd that's even after I removed the new file and tmeplate.

I have never seen this myself, some strange response with the controller @AlCalzone?

I'll close this issue - given its more a discussion of config updates - that is already addressed with https://github.com/zwave-js/node-zwave-js/issues/6097

AlCalzone commented 1 year ago

Would you prefer "Battery report threshold" or "Battery Report Threshold"?

Parameter labels should be Title Case, option labels Sentence case.

AlCalzone commented 1 year ago

I have never seen this myself, some strange response with the controller @AlCalzone?

This has happened a few times after I used PC controller. The driver typically resets the stick and retries the identification after this, where it should work.

fabnavigator commented 1 year ago

@AlCalzone, is there an easier way to load changes to the drivers, besides rebooting?

marcus-j-davies commented 1 year ago

Restarting the controller node destroys the driver instance and starts it back up.

marcus-j-davies commented 1 year ago

And make sure "Soft Reset USB" is ticked. This causes the stick to power down for a brief moment - before it gets reinitialised

fabnavigator commented 1 year ago

@marcus-j-davies, How do I do that?

fabnavigator commented 1 year ago

@marcus-j-davies , It was not checked, but it is now.

fabnavigator commented 1 year ago

@marcus-j-davies, How does this work "$if": "firmwareVersion >= 1.2",? It looks like 1.2 is treated as a number not a character string, but version numbers aren't numbers (ie., 1.20.1). If these are just treated as character strings "1.2" is less than "1.10". Right?

marcus-j-davies commented 1 year ago

It's all driver side config settings.

The driver manages all this internally - you just need to add the if lines - if a param is only from a Specific firmware version, we do this to hide the entries for firmware that won't support said Params.

https://zwave-js.github.io/node-zwave-js/#/config-files/conditional-settings?id=conditional-configuration-settings

marcus-j-davies commented 1 year ago

Sorry yes (I'm sure) 1.2 is less then 1.10, I re-call some work was done driver side sometime ago around this. @AlCalzone?

AlCalzone commented 1 year ago

"1.2" is less than "1.10"

Correct. The strings in those comparisons are interpreted and compared as versions according to semver.