feat: multicast/broadcast apis #25

robertsLando · 2020-11-24T15:46:35Z

Fixes #24

coveralls · 2020-11-24T15:50:10Z

Pull Request Test Coverage Report for Build 653435850

15 of 129 (11.63%) changed or added relevant lines in 4 files are covered.
14 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.2%) to 18.201%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/MqttClient.js	0	8	0.0%
types/index.d.ts	0	8	0.0%
lib/Gateway.js	15	57	26.32%
lib/ZwaveClient.js	0	56	0.0%

Files with Coverage Reduction	New Missed Lines	%
lib/Gateway.js	14	19.63%

Totals
Change from base Build 653429998:	-0.2%
Covered Lines:	2025
Relevant Lines:	11410

💛 - Coveralls

chrisns · 2020-11-25T09:48:34Z

lib/Gateway.js

+    for (let i = 0; i < values.length; i++) {
+      const valueId = this.topicValues[values[i]]
+      if (nodes.indexOf(valueId.nodeId) >= 0) {
+        this.zwave.writeValue(valueId, payload.value)
+      }
+    }


i'm never a fan of this syntax.
personally find

values .filter(value => nodes.includes(this.topicValues[value].nodeId)) .forEach(value => this.zwave.writeValue(this.topicValues[value], payload.value))

easier to read, but may just personal choice

chrisns · 2020-11-25T11:14:45Z

README.md

@@ -728,6 +729,30 @@ I will set the Heating setpoint of the node with id `4` located in the `office`

 `zwave/office/nodeID_4/thermostat_setpoint/heating`

+### Multicast
+
+You can send Multicast requests to _all values with a specific suffix_ of a _group_ of nodes in the network.


I'm not really clear on this, maybe provide a use case from the outset such as adjusting all your thermostats/lights?

be clearer that this is just a shorthand, it's not an actual multicast, behind the scenes zj2m is doing the heavy lifting and iterating

zwave-js does support actual multicast (if you use the same command and target the same endpoint of all nodes).

But it doesn't go through the commandClasses API that is being used here. Might be a good idea for a new feature , so you can grab a group of nodes and send a single multicast command to all of them.

I've raised zwave-js/node-zwave-js#1150 to track that.

For reference, what you can do already is this:

const cmd = new BinarySwitchCCSet(driver, { nodeId: [2,3,4,5], targetValue: true, }); await driver.sendCommand(cmd);

I do agree with that i really would like to send a group of nodes the command at the same time using a real multicast if that is possible.

Why, because i found it nice that all nodes respond at the same time, now it takes upto 5 seconds to respond.
Which is also generating errors like #26

My feeling about this is that since all these nodes are generating data (e.g. power reports, closing / dimming duration reports) that it become quite busy on the network, therefore i would like to send it using a real multicast to the nodes so that they all do receive the command before it is getting busy on the network.

If i see it now it looks like this is the same as what i can do within openhab

Shutters_Goup_All.members.forEach[ shutter | shutter.sendCommand(100) ]

I think we should wait for: zwave-js/node-zwave-js#1150 Before going on with this?

For reference, what you can do already is this:

@AlCalzone How could I get the correct command class constructor based on the command class id?

Sorry, I didn't see this...
https://github.com/zwave-js/node-zwave-js/blob/daf0feb9bfe14ae5b9a24c0d0d53206d5bdd41e9/packages/zwave-js/src/lib/commandclass/CommandClass.ts#L1057-L1059

But I'm not sure if that is exported through the main entry point.

ODINServ · 2020-11-25T11:55:23Z

README.md

@@ -728,6 +729,30 @@ I will set the Heating setpoint of the node with id `4` located in the `office`

 `zwave/office/nodeID_4/thermostat_setpoint/heating`

+### Multicast
+
+You can send Multicast requests to _all values with a specific suffix_ of a _group_ of nodes in the network.


I do agree with that i really would like to send a group of nodes the command at the same time using a real multicast if that is possible.

Why, because i found it nice that all nodes respond at the same time, now it takes upto 5 seconds to respond.
Which is also generating errors like #26

My feeling about this is that since all these nodes are generating data (e.g. power reports, closing / dimming duration reports) that it become quite busy on the network, therefore i would like to send it using a real multicast to the nodes so that they all do receive the command before it is getting busy on the network.

If i see it now it looks like this is the same as what i can do within openhab

Shutters_Goup_All.members.forEach[ shutter | shutter.sendCommand(100) ]

README.md

robertsLando · 2020-12-11T08:36:36Z

@ODINServ Could you give this a try?

robertsLando · 2020-12-14T10:30:54Z

@ODINServ @AlCalzone I'm still not 100% sure about using this way as it just works for not secure devices. I think the old way was 'dirty' and slow but at least it works

AlCalzone · 2020-12-14T10:39:27Z

I'm still not 100% sure about using this way as it just works for not secure devices. I think the old way was 'dirty' and slow but at least it works

What about this approach you could extend later when we have S2 support to create and use S2 Multicast groups.

checking which devices are insecure and using multicast on them. If that fails (command not supported), fall back to 2.
doing one-by-one singlecast on the rest

The tricky part about real multicast is that you can only use it for set-type commands, not ones that expect a response.

AlCalzone · 2020-12-14T10:46:10Z

lib/ZwaveClient.js

+    try {
+      const multicastGroup = this.driver.controller.getMulticastGroup(nodes)
+
+      await multicastGroup.setValue(valueId, value)


Like I wrote in my comment, I would check for ZWaveErrorCodes.CC_NotSupported and fall back to iterating singlecast requests.

@AlCalzone Could this cause multiple writes?

No, the checks happen before anything is sent.

zwave-js-assistant · 2021-01-23T09:59:38Z

🚧 It seems like this PR has lint errors 🚧
I should be able to fix them for you. If you want me to, just comment
@zwave-js-bot fix lint

lib/Gateway.js

lib/ZwaveClient.js

robertsLando · 2021-02-01T09:27:22Z

@AlCalzone

AlCalzone · 2021-02-01T10:01:57Z

Multicast SETs don't poll (because that would require one GET for each node, defeating the purpose of a multicast). But we should assume that the command was accepted by all nodes if it succeeds like we already do for singlecasts.

ODINServ · 2021-02-02T10:08:13Z

I was very busy lately and did not have any time to work on my home automation stuff.

So to summarize what i did:

I re-included all nodes unsecured to reduce my strange network issues. (I dont have any now :))
The last thing i did was running my setup "the npm way" just after the new broadcast API became available via MQTT.
- Which i'm using now for 1.5 months without any issues!

From: @blhoward2

This does exactly what I'm looking for and I think it would be heavily used if people knew what it could do

Agreed maby this feature requests will solve this: #70

I'd switched to this branch (without linking the node-zwave-js module) and tested it.

For me with my FGR223 modules this is working as expected with the same results and question that @blhoward2 has.

Node 3 turned off physically, but remained on in HA.

From @AlCalzone

Multicast SETs don't poll (because that would require one GET for each node, defeating the purpose of a multicast). But we should assume that the command was accepted by all nodes if it succeeds like we already do for singlecasts.

So when using the (new) broadcast API, the values are updated when (in my case) the shutters reach there targetValue, not sure if this info helps.

zwave-js-assistant · 2021-03-09T17:02:50Z