cgarwood / homeassistant-zwave_mqtt

Limited Pre-Release of the new OZW1.6 Z-Wave component. Currently has limited platform support. Check the README for more details.
72 stars 8 forks source link

scene_activated service require actual node id rather than implied entity #73

Open kevinkahn opened 4 years ago

kevinkahn commented 4 years ago

In setting up an automation based on service.scene_activated you seem to need to supply the actual node id that the zwave system supplies. This has already been mapped in setup to the HA entity corresponding to it so it would be nicer to be able to specify the entity_id for the device. It would avoid having 2 references to the same thing that can get out of sync.

kpine commented 4 years ago

That looks like the behavior for the current zwave integration, so it was probably just missed here.

marcelveldt commented 4 years ago

You're absolutely right. Shouldn't be too hard to add the entity_id in the event. Fix will be there asap

marcelveldt commented 4 years ago

Ok, had a look at this. In the old/current implementation each node itself has an entity. This is not the case with the new implementation where you have the device with it's entities. Scene events happen at the device level so no entity id and we wil not be able to provide it in the scene_activated event unless we bring back the entity for the node itself.

kevinkahn commented 4 years ago

Not sure I understand. Scene_activated currently needs a "node_id" to match on. E.g., I have an automation that triggers on zwave_mqtt.scene_activated for: node_id: 6 scene_id: 2 scene_value_label: Pressed 2 Times

To get the node_id I need to look at an entity (in this case) switch.patio_lights which in this case has a node_id of 6. There should be some way to retrieve this value from the states for use in the trigger (e.g., {{ states.switch.patio_lights.attributes.node_id }} returns 6 but I haven't found a syntax that lets me substitute this for the literal 6 in the yaml - but I'm not that familiar with the template options). In any case, if we are catering to the naive user this would not be particularly friendly. So I guess that what I was asking for was some straightforward way to build a trigger where the node_id value was extracted easily from the related entity_id. It would seem that if I can do that manually we should be able to do it in a simple user friendly way.

I understand that a single device maps to potentially multiple entities but it each of those entities maps back to the device and it that mapping that I'd like to see exposed. So if a switch exposes a switch entity as well as a sensor entity for, e.g., kw hours both have the node id of the underlying device. It might be a bit weird that you could create a trigger for an automation that referenced the sensor entity's node id (which would likely be a side effect of this) but the win would be that a naive user would get a way to easily say "when that switch is pressed twice on" which currently she can't do.

marcelveldt commented 4 years ago

Hi Kevin, a Z-wave device has multiple entities, each feature/function has it's own entity. We will solve this by providing device automations for you to make it user friendly.

BTW: your syntax was correct. We set the node_id as an attribute for each entity, so {{ states.switch.patio_lights.attributes.node_id }} should work.

I'll let you know once the device automations are implemented, that will make it really easy to automate with scene events.

kevinkahn commented 4 years ago

Thanks - wasn't trying to cause extra work (especially since I can certainly just look up the node ids) but it does seem in the long run our desire to make HA very user friendly will benefit from making scenes easier.

I did try just replacing: node_id: 6 with node_id: " {{ states.switch.patio_lights.attributes.node_id }} " and it did not appear to match on the event. Looks like maybe it was searching for the literal quoted string. But without the quotes the system changes it to some weird [Object object] string. I must be doing something wrong.

marcelveldt commented 4 years ago

Ah well, I get what you mean. You're trying to use a template in the trigger. Not sure if that's supported in core yet. One thing to try is use data_template instead of data.

kevinkahn commented 4 years ago

Figured that was the case. Unfortunately data_template doesn't seem to be implemented in the trigger nor is event_data_template. They only work in the actions. Not a big deal for now. Thanks for the quick responses. (now hope Justin gets around to looking at the refresh/target value stuff soon - that's actually a more serious issue and I agree with you, one that should get addressed upstream).

marcelveldt commented 4 years ago

One solution that just popped up in my mind: drop the node_id from the event trigger and add it as a condition, as conditions can be templated.

So in your condition you check the trigger.service_data.node_id

kevinkahn commented 4 years ago

Yup - that approach does work. I had tried to do that earlier but didn't have the trigger reference correct. It doesn't seem to be in the documentation anywhere that I could find (specifically, using service_data). But perhaps I just missed it. It's rather awkward looking but it does document in the automation which switch is being used so that's good.

BTW, out of curiosity I took a look at the code and now I understand your previous comment. At the time of generating the event you only have the node_id and at the time that the event is matched by the automation, that work is being done by the core and not you. Interesting conundrum. It does seem that the cleanest solution might be if the core simply permitted a reference like switch.myswitch.node_id in the trigger. I suspect that they don't want to possibly do dynamic evaluation of the trigger conditions there since while in this case node_id is completely static, that notation could be referencing fields that were not. I wonder if this suggests a more general issue since it idea of using a symbolic reference in a trigger seems pretty reasonable if it is static at least.

Clearly a device automation trigger is the simplest. (I just need to rename all my devices since I hadn't bothered so they all are named the same. Duh!) If you did do a device trigger though it should ultimately be possible to actually make the trigger clearly on e.g., Scene 1 Double Tap by pulling the names of the scenes and the value ids from the corresponding device info. That would be a real benefit to non-advanced users compared to having to know the scene ids and operation ids.

marcelveldt commented 4 years ago

Yes, that's indeed the idea. Great that the temporary workaround with the condition is working.