Skip to content
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

Blacklist some (but not all) HA config discoveries #3558

Closed
apella12 opened this issue Jan 26, 2024 · 10 comments · Fixed by #3569
Closed

Blacklist some (but not all) HA config discoveries #3558

apella12 opened this issue Jan 26, 2024 · 10 comments · Fixed by #3569
Assignees
Labels
enhancement New feature or request

Comments

@apella12
Copy link
Contributor

apella12 commented Jan 26, 2024

Is your feature request related to a problem? Please describe.
I have highlighted in the OpenHAB (OH) community that ZUI can be used with OH in place of the OH Z-wave binding (and have implemented that myself). I think this would provide the OH community with additional features and ZUI with additional customers.
However, I implemented ZUI without the HA discovery because during testing several HA configs do not work within OH (The details are TL;DR for this request). Instead I used generic MQTT things (in OH lingo) directly from the zwave section of MQTT. This is okay for many OH users, but for less technical ones I had hoped for HA config discovery.

Describe the solution you'd like
Ability in the Node HASS tab to disable some of the config IDs, but not all. (This may be possible already, but all I have managed to do is disable all or none.) I'd also add that recently added configuration CC configs (number and switch) be disabled by default. Those are in the "static" category and IMO the ratio of clutter to usefulness is high. (I was changing two configurations (112 params) in a 35 node network and noticed within the HA config test environment have 600 new HA configs

Describe alternatives you've considered
Before the recent change (112 parameters) I registered and an issue with jinjava about using numbers as keys. Jinjava is what OH uses to decode the HA configs. There is also a PR to address, but it hasn't been merged for several months (don't know why, except the developers do not see as a problem). The other prior problem is with scenes (91) a null value creates a warning in a number channel, but works as a string item. Since the recent change there are numerous WARN messages because the device config files are not always right for the version of the device. (again tl;dr regarding examples). Rather than fix potentially dozens of device configs I'm suggesting the blacklist idea and that those be disabled by default.

Additional context
These probably won't mean much, but are from the OH logs.

Original issue.

Executing the JINJA-transformation failed: An error occurred while transformation. InterpretException: Error resolving expression [{22: "Window/door is open",5632: "Window/door is open in regular position",5633: "Window/door is open in tilt position"}[value_json.value] | default(value_json.value)]: TemplateStateException: Dict key must be a string or identifier, was: 22

Configuration issues (Configs 9 & 10) do not exist for my device (tried to change in ZUI, but returns "0"

Command '0' from channel 'mqtt:homeassistant_zwavejs2mqtt_5F0xf1c43721_5Fnode9:6035d9536c:zwavejs2mqtt_5F0xf1c43721_5Fnode9:zwavejs2mqtt_5F0xf1c43721_5F9_2D112_2D0_2D9#number' not supported by type 'NumberValue': 0 is out of range
Command '0' from channel 'mqtt:homeassistant_zwavejs2mqtt_5F0xf1c43721_5Fnode9:6035d9536c:zwavejs2mqtt_5F0xf1c43721_5Fnode9:zwavejs2mqtt_5F0xf1c43721_5F9_2D112_2D0_2D10#number' not supported by type 'NumberValue': 0 is out of range
@apella12 apella12 added the enhancement New feature or request label Jan 26, 2024
@robertsLando
Copy link
Member

robertsLando commented Jan 29, 2024

Hi @apella12!

I could suggest to add a setting in order to enable/disable Configuration CC discovery. @ccutrer what do you think?

About the possibility to enable/disable discoery of specific device entities, did you tried to manually set the ignoreDiscovery parameter in the json on the right side and then pressing save?

@robertsLando
Copy link
Member

See #3569 (comment)

@ccutrer
Copy link
Contributor

ccutrer commented Jan 29, 2024

@apella12 : two questions.

First, which version of openHAB are you running? The new configuration entities are "enabled_by_default": false. In Home Assistant, that means they are just kind of hidden until you go explicitly enable them. In (recent versions of) openHAB, they are created as Advanced channels, so you won't see them in the UI unless you click the button to show advanced channels.

Second, as per your log (Command '0' from channel 'mqtt:homeassistant_zwavejs2mqtt_5F0xf1c43721_5Fnode9:6035d9536c:zwavejs2mqtt_5F0xf1c43721_5Fnode9:zwavejs2mqtt_5F0xf1c43721_5F9_2D112_2D0_2D10#number' not supported by type 'NumberValue': 0 is out of range, the actual problem isn't about strings or numbers. It's that the range for this number entity in the home assistant config is defined, but the ZWave value is outside that range (0, as noted in the log). All of my devices (my network is 145 nodes, but fairly homogenous with only a handful of device types) seem to properly advertise their supported ranges. My suspicion is you have a device whose min and max are null or undefined, meaning there is no set range. At https://github.com/zwave-js/zwave-js-ui/blob/master/api/lib/Gateway.ts#L1699 that basically means the Home Assistant config will pass through the null, and per the Home Assistant spec (https://www.home-assistant.io/integrations/number.mqtt), that will default to 1-100. Can you please post the final Home Assistant config JSON that's getting published to MQTT for this entity? There could be two things to fix here... on the ZWave side, if it's null, we should probably publish a min/max of -254-255 (IIRC all config values are single byte, but they may be interpreted as signed or unsigned - @robertsLando can you confirm that?). Second, on the openHAB side we may not be properly interpreting an explicit null as not having a minimum. I'd need to check that that's how Home Assistant operates before attempting to replicate that behavior. But that would be an openHAB issue, not a zwave-js issue.

@robertsLando
Copy link
Member

IIRC all config values are single byte, but they may be interpreted as signed or unsigned - @robertsLando can you confirm that?

Yeah exactly

@apella12
Copy link
Contributor Author

apella12 commented Jan 29, 2024

@ccutrer @robertsLando
1a) OH4.1.1 and the configurations did show up as advanced.
1b) The configs did show up by default. It doesn't seem "enabled_by_default": false, keeps them from appearing in ZUI. I have only been playing with this today (so could just not understand- never have used HA) They were written to MQTT topics and picked up by OH on startup (in my test environment). I needed to set the persistence flag on the disabled items to keep them from coming back on restart. For the future I will use the manual discovery option, set disabled, persist that setting then allow auto discovery again.
2a) I already have fixed the device issue with a custom json in the store/config folder. The broader issue is, unlike the CCs which are "advertised" by the device, the configs are queried by Zwave-js during startup based on the json config for that device. If the particular device does not have the config because it was added later by the manufacturer (the case here) what will be reported is a "crap shoot" and could fall outside the range. I had three configs over 2 device types (and had two of each) out of 35 nodes. Scaling this up to the total number of zwave devices, I was concerned about potentially hundreds of config issues from the config channels, hence my suggestion that they be disabled by default. In relation to the total number of configs I only see a few being used (IMO)
2b) Although most config params are 1 byte, however, the ones regarding intervals are typically 2 bytes (report temp every hour =3600). I do not think there is a ZWA specification restricting the config byte length, but could be wrong. Mostly only the 1 byte params use signed.

@ccutrer
Copy link
Contributor

ccutrer commented Jan 29, 2024

1b: Indeed, "enabled_by_default": false will have no impact on zwave-js-ui, just in how Home Assistant and openHAB display them.
2. I'm not super familiar with the internals of zwave-js, or even the ZWave protocol itself. If we can't know the proper range, we need to be able to advertise that appropriately. But I can't write code for that if I don't have a device that behaves that way, unless someone can confirm to me that min and max can legitimately be null on a ZUIValueId for a configuration param. It seems like it can be from the definition of ZUIValueId.

@apella12
Copy link
Contributor Author

1b) good to know
2) I'm probably not explaining well but agree you will not be able to write code for this category of problems. A custom (or changed) config.json is needed without the parameters since they do not exist within that device. Unlike the rest of the device interview there is no guarantee the paramInformation in the json exists in a particular device. When queried about a non-existent parameter the device may respond with "0" or with the value of another parameter of the device at random.

tl;dr
Using the error message (reporting "0"): the standard json device config was (modified to include the minValue from the template)

	"paramInformation": [
		{
			"#": "1",
			"$import": "templates/config_template.json#init_brightness"
		},
		{
			"#": "9",
			"$import": "templates/config_template.json#dim_step_zwave",
			"minValue": 1
		},
		{
			"#": "10",
			"$import": "templates/config_template.json#dim_speed",
			"minValue": 1
		}
	],

What works now is

	"paramInformation": [
		{
			"#": "1",
			"$import": "templates/config_template.json#init_brightness"
		}
	],

Because 9 & 10 are not present on my devices.

We can change the json to use firmware versions "$if": "firmwareVersion >= 3.0", (and many are setup that way), but now disabling is an option too as I have no need for those params in OH and never noticed their values.

Hope that helps.

@ccutrer
Copy link
Contributor

ccutrer commented Jan 29, 2024

Ah, okay. So you have custom device config to specify new params (with a minValue), but not all of your devices actually support those params since they're on older firmware versions. So you are actually getting params with a current value (which is essentially garbage, since the device doesn't support that value) less than minValue. Yup, okay, got it. Nothing I can do about that.

@robertsLando
Copy link
Member

@ccutrer @apella12 TL;DR Should I do something else here?

@apella12
Copy link
Contributor Author

@robertsLando No you are good. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants