OpenZWave / Zwave2Mqtt

Fully configurable Zwave to MQTT gateway and Control Panel using NodeJS and Vue
MIT License
353 stars 93 forks source link

[bug] missing configuration values with hass integration #445

Closed travisghansen closed 3 years ago

travisghansen commented 4 years ago

Version

Build/Run method

Zwave2Mqtt version: 3.1.0 (on an rpi) Openzwave Version:

Describe the bug

I've got 3 devices which either don't configure the hass settings at all or only configure (seemingly) read-only data points for integration with hass.

Examples:

Expected behavior

I would generally assume all 'user', 'configuration' and 'system' values create respective hass devices. Let me know if I'm missing something in how I've got it setup.

Thanks!

robertsLando commented 4 years ago

@travisghansen That's not a bug. Z2M is intended to be used as control panel to configure your devices and hass to be used as dashboard so that's why only user values are mapped

travisghansen commented 4 years ago

@robertsLando ok, but what about the user values that are not exposed?

robertsLando commented 4 years ago

@travisghansen Could you tell me the valueid of those values?

travisghansen commented 4 years ago

@robertsLando sure

I should note the only hass devices created for user editable values are for a couple of dimmer plugs I have. Of the above 3 devices, none of the editable user values have associated hass devices..or said differently, all of the hass devices are associated with read-only values.

robertsLando commented 4 years ago

in poor words i need to make the support of

The others are thermostat command classes, if you check the readme those need to be configured manually by creating a configuration in hass/devices.js

travisghansen commented 4 years ago

I missed that in the readme I’ll go have a look. What about 102 from the garage door? Same thing? Manually add?

travisghansen commented 4 years ago

Ok, so I've added the following:

cat store/customDevices.json 
{
  "335-21553-21570": [
    {
      "type": "climate",
      "object_id": "thermostat",
      "values": [
        "49-1-1",
        "64-1-0",
        "66-1-0",
        "67-1-1",
        "67-1-2",
        "68-1-0",
        "69-1-0"
      ],
      "fan_mode_map": {
        "on": "On",
        "auto": "Auto"
      },
      "mode_map": {
        "off": "Off",
        "heat": "Heat",
        "cool": "Cool",
        "auto": "Auto"
      },
      "setpoint_topic": {
        "Heat": "67-1-1",
        "Cool": "67-1-2"
      },
      "default_setpoint": "67-1-1",
      "discovery_payload": {
        "min_temp": 61,
        "max_temp": 90,
        "modes": [
          "off",
          "heat",
          "cool",
          "auto"
        ],
        "fan_modes": [
          "on",
          "auto"
        ],
        "action_topic": "66-1-0",
        "mode_state_topic": "64-1-0",
        "mode_command_topic": true,
        "current_temperature_topic": "49-1-1",
        "current_temperature_template": "{{ value_json.value }}",
        "temperature_state_template": "{{ value_json.value }}",
        "temperature_low_command_topic": true,
        "temperature_low_state_template": "{{ value_json.value }}",
        "temperature_high_command_topic": true,
        "temperature_high_state_template": "{{ value_json.value }}",
        "fan_mode_command_topic": true,
        "fan_mode_state_topic": "68-1-0"
      }
    }
  ]
}

But I'm not seeing anything additional in the UI...what should I be doing now?

On a related note, I've tried adding the block to the UI (hass device json) (without device id key/etc) directly which validates it until the ADD button turns blue. However clicking the ADD button does nothing. Nothing going on with the browser on the network tab nor the console while 'inspecting'.

robertsLando commented 4 years ago

@travisghansen When you start zwave2mqtt do you see a log that says once custom device has been added?

Also in order to make this work you need to restart the application or press on rediscover button

If nothing shows up check hass logs and check if there are errors with the discovery configuration sent

travisghansen commented 4 years ago

I’ll check for sure but when I say ui here I mean the z2m ui not the hass ui. Nothing will show up in hass unless it first appears in z2m right?

robertsLando commented 4 years ago

@travisghansen Sure, if it doesn't show up so something is wrong with the config. Do you see any error in z2m logs? Do you se that log I mentioned that says a custom component has been loaded?

travisghansen commented 4 years ago

I’ll check in just a few minutes and send over what I’ve got.

travisghansen commented 4 years ago

I'm not seeing anything but it could be burried in the mountain of zwave output...I know the file is at least being read because I initially entered the data as an object instead of an array of objects and z2m puked due to a forEach error.

Got a more specific line I can grep the output for to check what you're after?

travisghansen commented 4 years ago

The error I got with a misformed file:

/usr/src/app/lib/Gateway.js:255
    nodeDevices.forEach(device => this.discoverDevice(node, device))
                ^

TypeError: nodeDevices.forEach is not a function
    at Gateway.onNodeStatus (/usr/src/app/lib/Gateway.js:255:17)
    at ZwaveClient.emit (events.js:310:20)
    at ZwaveClient.nodeReady (/usr/src/app/lib/ZwaveClient.js:465:10)
    at OZW.emit (events.js:322:22)
travisghansen commented 4 years ago

Scratch that (sorry for all the confusion). I just noticed each thermostat in z2m still has 2 devices only but one of them is a thermostat instead of the previous temperature reading...

I subsequently noticed this in the hass logs:

2020-05-07 08:21:40 INFO (MainThread) [homeassistant.components.mqtt] Got update for entity with hash: ('climate', 'nodeID_23 thermostat') '{'min_temp': 61, 'max_temp': 90, 'modes': ['off', 'heat', 'cool', 'auto'], 'fan_modes': ['on', 'auto'], 'action_topic': 'z2m/23/66/1/0', 'mode_state_topic': 'z2m/23/64/1/0', 'mode_command_topic': 'z2m/23/64/1/0/set', 'current_temperature_topic': 'z2m/23/49/1/1', 'current_temperature_template': '{{ value_json.value }}', 'temperature_state_template': '{{ value_json.value }}', 'temperature_low_command_topic': True, 'temperature_low_state_template': '{{ value_json.value }}', 'temperature_high_command_topic': True, 'temperature_high_state_template': '{{ value_json.value }}', 'fan_mode_command_topic': 'z2m/23/68/1/0/set', 'fan_mode_state_topic': 'z2m/23/68/1/0', 'mode_state_template': '{{ {"Off":"off","Heat":"heat","Cool":"cool","Auto":"auto"}[value_json.value] | default(\'off\') }}', 'temperature_state_topic': 'z2m/23/67/1/1', 'temperature_command_topic': 'z2m/23/67/1/1/set', 'fan_mode_state_template': '{{ {"On":"on","Auto":"auto"}[value_json.value] | default(\'auto\') }}', 'precision': 0.1, 'device': {'identifiers': ['zwave2mqtt_0xc6115232_node23'], 'manufacturer': 'Linear (Nortek Security Control LLC)', 'model': 'GC-TBZ48 Battery Powered Z-Wave Thermostat (0x5431)', 'name': 'nodeID_23', 'sw_version': '1.02'}, 'name': 'nodeID_23_thermostat', 'unique_id': 'zwave2mqtt_0xc6115232_Node23_thermostat', 'platform': 'mqtt'}'

I'll do some more digging around now to see if anything seems off. So far though looks great!

Would it be possible to dynamically set the min/max temps using the configuration values instead of hard-coding?

robertsLando commented 4 years ago

@travisghansen Sure but for that I need to write some code :)

Check out here: https://github.com/OpenZWave/Zwave2Mqtt/blob/master/lib/Gateway.js#L715

You could simply add this lines inside climate if statement:

if(hassDevice.setpoint_max && hassDevice.setpoint_min) {
payload.max_temp = hassDevice.setpoint_max
payload.min_temp = hassDevice.setpoint_min
}
travisghansen commented 4 years ago

Yeah, mostly just thinking about getting the details committed for others to use. In any case I've still got something off as trying to set the target temps is behaving strangely..

2020-05-07 09:14:53 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 21.0
2020-05-07 09:14:53 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 90.0
2020-05-07 09:14:56 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 21.0
2020-05-07 09:14:56 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 89.0
2020-05-07 09:15:08 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 71.0
2020-05-07 09:15:08 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 89.0
robertsLando commented 4 years ago

@travisghansen Sure once you have a working configuration please send a PR and add it to hass/devices :) Fot that fix about the custom min/max setpoints I can make it for you now

travisghansen commented 4 years ago

What's the connection between these:

      "setpoint_topic": {
        "Heat": "67-1-1",
        "Cool": "67-1-2"
      },

and

        "temperature_low_command_topic": true,
        "temperature_high_command_topic": true,

I've explicitly set the latter and now things are working as expected..

robertsLando commented 4 years ago

I don't support temperature_low_command_topic and temperature_high_command_topic so they doesn't make any difference to me. There is no connection between the two things

The setpoint topic is used to automatically rediscover the climate component once the mode changes as zwave2mqtt sends the values on differents topics (as they are two differents valueids)

travisghansen commented 4 years ago

I see. This is relatively complex stuff obviously trying to map :(

I have an 'auto' setting and the hass UI has target high and low temps (in addition to the generic 'target temp')...seems to make sense to map those to the setpoint_topic as appropriate?

If not if I replace true with 67-1-1 and/or 67-1-2 will that be translated appropriately?

travisghansen commented 4 years ago

That's what these are from (setting the low/high without a translation to proper topics) FYI:

2020-05-07 10:24:13 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 21.0
2020-05-07 10:24:13 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 73.0
2020-05-07 10:24:32 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 70.0
2020-05-07 10:24:32 DEBUG (MainThread) [homeassistant.components.mqtt] Transmitting message on True: 73.0
robertsLando commented 4 years ago

If not if I replace true with 67-1-1 and/or 67-1-2 will that be translated appropriately?

Nope I need to make the support in the gateway to check for this files and translate the valueids to the corrisponding topics

travisghansen commented 4 years ago

Ok cool. Looks like climate devices support some sort of bitwise features flag which for me was set to 11 so my UI is showing those elements. Not sure how that value is getting set but may be beneficial to somehow allow overriding it.

Been using it for several hours now and seems ok but somehow ha got into a situation where it had an unknown state for the thermostats even though it was not restarted and I've got the retain flags set. I’ll keep an eye on it for another couple days to see how it goes.

Any recommendations for the garage door opener?

robertsLando commented 4 years ago

Any recommendations for the garage door opener

What kind of problem do you have with that?

travisghansen commented 4 years ago

The most important attribute isn’t mapping (the state/position) to be able to actually control it. I just looked and saw you have an example custom entry for a cover so maybe I’ll try that out. 102-1-1

robertsLando commented 4 years ago

just looked and saw you have an example custom entry for a cover so maybe I’ll try that out

Yep try with that and let me know :)

Remember to submit a PR if you have created some custom device, could help other users :)

travisghansen commented 4 years ago

Yeah for sure will do. Or at least pass them on to you.

travisghansen commented 4 years ago

This seems to work like a charm:

  "335-12336-18244": [
    {
      "type": "cover",
      "object_id": "barrier_state",
      "values": [
        "102-1-1"
      ],
      "discovery_payload": {
        "command_topic": "102-1-1",
        "state_topic": "102-1-1",
        "value_template": "{{ value_json.value }}",
        "device_class": "garage",
        "payload_open": "Opened",
        "payload_close": "Closed",
        "payload_stop": "Stopped",
        "state_open": "Opened",
        "state_opening": "Opening",
        "state_closed": "Closed",
        "state_closing": "Closing"
      }
    }
  ]

I'm pretty sure the device doesn't support setting Stopped so I'm not sure if there's a way to tell hass to not show that feature for this device (just getting started with hass so I have a lot to digest). It looks like covers have the same sort of bitwise feature flags that can be set like climate controls so maybe...

robertsLando commented 4 years ago

Will add it to devices. Thanks @travisghansen

robertsLando commented 4 years ago

@travisghansen Just opened a PR for the support of this with all devices. Check it out once it's merged

travisghansen commented 4 years ago

OK I wondered about barrier_state as the object_id. I just sort of picked that since that's what the UI showed. If I'm reading the PR correctly it looks like it will generically handle barrier state devices without anything special in hass/devices.js which is cool. I'm not sure if locks use barrier state (I don't have any myself).

The thermostat has generally been functioning but as I mentioned hass seems to lose track of the state of the thing (operating mode, and enabled features) over time...I'm not entirely sure of that's a problem with z2m or hass itself. I'm inclined to say hass itself as z2m is publishing with retain flags so I'm not sure how/why hass would not have some value for the operating state etc. If you have any input on this one let me know.

Do you still intend to do something with scene activation? Want me to try something out locally? I know this minimote device has 2 operating modes. Here's my note about it from years ago:

 - to put into scene mode parameter 250 should be 1 (0 is group mode)

I'm also not sure if/how that would change the interaction of the device with z2m in terms of auto-detection etc.

robertsLando commented 4 years ago

I'm inclined to say hass itself as z2m is publishing with retain flags so I'm not sure how/why hass would not have some value for the operating state etc

I suggest you to open a question on hass forum, as you said as long as messages are retained hass should always know the last status of them

Do you still intend to do something with scene activation? Want me to try something out locally? I know this minimote device has 2 operating modes

What do you mean with scene activation? Activate scenes using mqtt or what else?

travisghansen commented 4 years ago

Yeah something really weird. Not sure if it’s connected to the upper/lower values being weird. Definitely a bad experience but still not sure exactly where the issue is honestly.

I’m referring to your earlier comment about support for command classes 43 and 129. Do you still intend to implement those?

robertsLando commented 4 years ago

@travisghansen Sure! I would just ask to you if you can find a correct configuration like you did for the barrier state so I can do the same for them :)

travisghansen commented 4 years ago

I can try but still really new to hass. I’m guessing the scene activation should be a sensor? For me the value changes from 1-8 temporarily and then back to 0 after a second or so. Does a sensor make sense for that kind of thing or am I way off here?

robertsLando commented 4 years ago

I'm not an hass user, I just used hass for few weeks to develop hass integration, for what I know it should be a sensor yes. Everything I have used is in this docs:

https://www.home-assistant.io/docs/mqtt/discovery/

travisghansen commented 4 years ago

@robertsLando so I've added the following:

  "134-3-1": [
    {
      "type": "sensor",
      "object_id": "scene",
      "discovery_payload": {
        "state_topic": "43-1-0"
      }
    }
  ],

But both before and after adding that snippet I don't see the hass section in the z2m UI...what needs to be done to get it to show up?

robertsLando commented 4 years ago

You are missing

"values": [
        "43-1-0"
      ],
travisghansen commented 4 years ago

Yeah, just getting it going now actually. Not really sure how to handle this sanely to be honest. Technically zwave registers an event every button push (which, if you push it fast enough the state doesn't actually change).

Is this off?: https://github.com/OpenZWave/Zwave2Mqtt/blob/master/hass/configurations.js#L135

Did you intend for that to be sensor_scene?

travisghansen commented 4 years ago

If I press the button quickly this is what comes through to hass

2020-05-13 08:33:52 DEBUG (MainThread) [homeassistant.components.mqtt] Received message on z2m/5/43/1/0: b'{"value_id":"5-43-1-0","node_id":5,"class_id":43,"type":"int","genre":"user","instance":1,"index":0,"label":"Scene","units":"","help":"","read_only":true,"write_only":false,"min":-2147483648,"max":2147483647,"is_polled":false,"value":1,"lastUpdate":1589380432510}'
2020-05-13 08:33:52 DEBUG (MainThread) [homeassistant.components.mqtt] Received message on z2m/5/43/1/0: b'{"value_id":"5-43-1-0","node_id":5,"class_id":43,"type":"int","genre":"user","instance":1,"index":0,"label":"Scene","units":"","help":"","read_only":true,"write_only":false,"min":-2147483648,"max":2147483647,"is_polled":false,"value":1,"lastUpdate":1589380432736}'
2020-05-13 08:33:53 DEBUG (MainThread) [homeassistant.components.mqtt] Received message on z2m/5/43/1/0: b'{"value_id":"5-43-1-0","node_id":5,"class_id":43,"type":"int","genre":"user","instance":1,"index":0,"label":"Scene","units":"","help":"","read_only":true,"write_only":false,"min":-2147483648,"max":2147483647,"is_polled":false,"value":0,"lastUpdate":1589380433507}'
2020-05-13 08:33:53 DEBUG (MainThread) [homeassistant.components.mqtt] Received message on z2m/5/43/1/0: b'{"value_id":"5-43-1-0","node_id":5,"class_id":43,"type":"int","genre":"user","instance":1,"index":0,"label":"Scene","units":"","help":"","read_only":true,"write_only":false,"min":-2147483648,"max":2147483647,"is_polled":false,"value":0,"lastUpdate":1589380433733}'

Not even sure if it's sane but looking at this and how it may related: https://www.home-assistant.io/integrations/device_trigger.mqtt/

travisghansen commented 4 years ago

Magical combination:

  "134-3-1": [
    {
      "type": "sensor",
      "object_id": "scene_state",
      "values": [
        "43-1-0"
      ],
      "discovery_payload": {
        "state_topic": "43-1-0",
        "value_template": "{{ value_json.value}}"
      }
    }
  ]
robertsLando commented 4 years ago

@travisghansen PR ready

travisghansen commented 4 years ago

@robertsLando very cool. Where did things land for adding support to expand temperature_{high,low}_command_topic?

robertsLando commented 4 years ago

Are those properties of climate discovery payyloads?

If yes I just need to add them inside here in order to check that the payload supports them

travisghansen commented 4 years ago

Yeah seen here: https://www.home-assistant.io/integrations/climate.mqtt/

I'm also guessing that optionally we could set the ‘away’ mode stuff which likely maps to (for me) the heating/cooling econ attributes mentioned above. That would essentially be 100% coverage for the device I have. Especially if the scene mode is implemented which is the same class as the minimote (no idea how/if that would ever be used in a thermostat but hey, it’s there).

travisghansen commented 4 years ago

Actually scratch the econ comment, that’s a mode in the discovery stuff not a high/low setting so probably not a proper mapping.

travisghansen commented 4 years ago

Just like we discussed dynamically sending the min/max temps by using configuration values it may make sense to do the same for precision which can be determined using the celsius/fahrenheit configuration value as well? For me its 112-1-7 for example.

robertsLando commented 4 years ago

Just like we discussed dynamically sending the min/max temps by using configuration values

IMO configuration values should remain in z2m, I mean you use the control panel to configure your device and hass to use them (so only 'user' values there)

travisghansen commented 4 years ago

I'm not suggesting we send those values as items the can be set/changed via hass. I'm suggesting that dynamically setting the values sent to hass based off the internal options scales better particularly since these have to be hard-coded and manually injected into the code base anyhow. My settings for min/max temp will likely not necessarily match someone else' settings.

Just trying to make it more dynamic for other users is all..

robertsLando commented 4 years ago

@travisghansen Could you better explain your idea? I don't fully understand what you want