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

Unmanaged GetValue operation returns value only #15

Closed hufftheweevil closed 3 years ago

hufftheweevil commented 3 years ago

Perhaps this was intended, but either way, I think it's worth discussion.

When using the Unmanaged GetValue operation, it outputs the value using the VALUE_UPDATED event, but the object property is just the raw value itself. Whereas when the controller sends an update, the object property is an object that contains the entire ValueID along with the value.

So when it's just a value, the downline node reading the VALUE_UPDATED event needs to somehow determine what class/property/propertyKey the value was for. Honestly, I'm not even sure how I'm going to do that yet.

https://github.com/zwave-js/node-red-contrib-zwave-js/blob/90bf06fd7d624a310b12d4bc99c9ca24a3a26eaf/zwave-js/zwave-js.js#L306

marcus-j-davies commented 3 years ago

Yup,

The getValue method within the z-wave js lib only returns the value, as identified by the ValueID. I think the premise here, is that the caller would already know to what CC, Endpoint etc etc, it come from (as they would have called it).

What we can do, is attach a copy of the orignal request on its return to red? This way, it can be associated to the request making it easy to identify its origin.

EDIT: The origin property will only be included for Unmanaged -> GetValue and will be a copy of the request. this would allow the user to understand where the value come from, or more helpfully, the CC end point etc etc.

I would also think updating the event is wise also

{
  node: 13,
  object: "value",
  event: "GET_VALUE_RESULT",
  origin: {}
}
marcus-j-davies commented 3 years ago

Extending the send method, to support this, should be easy also.

function Send(Node, Subject, Value, send, OriginalMessage) {

   let PL = { "node": Node.id, "event": Subject, "timestamp": new Date() }

   if(OriginalMessage != null) {
      PL.origin = OriginalMessage;
   }
   ...
}
marcus-j-davies commented 3 years ago

Have classified this as an enhancement if you don't mind.

hufftheweevil commented 3 years ago

Agreed with all. Sounds good!

marcus-j-davies commented 3 years ago

Latest commit in dev which provides the changes discussed - which I haven't actually tested yet :disappointed:

Object contains:

{
  response: Return Value,
  valueId: Orignal ValueID (params[0])
}