zwave-js / zwave-js-ui

Full featured Z-Wave Control Panel UI and MQTT gateway. Built using Nodejs, and Vue/Vuetify
https://zwave-js.github.io/zwave-js-ui
MIT License
975 stars 203 forks source link

Lastupdate value does not remain as initial node notification time [bug] #391

Open chrish987 opened 3 years ago

chrish987 commented 3 years ago

Before submitting a bug please read: https://zwave-js.github.io/zwavejs2mqtt/#/troubleshooting/bug_report

Version

Build/Run method

Latest :dev image (from yesterday)

Describe the bug

Not sure if this is a bug or working as designed.

When restarting zwavejs2mqtt mqtt messages are sent for all node values. The lastUpdate value in the message JSON is set to the time that the update was sent (i.e. now) rather than the time that the update was received from the node.

To Reproduce

For example a battery temperature sensor that only sends a zwave update every 60 minutes when it wakes up, if I wait until the scheduled wakeup/send then I see the lastupdate is the time the node send the update. If I wait 5 minutes then restart zwavejs2mqtt another message is sent and the lastupdate has updated to current time.

Expected behavior

Ideally this would still be the timestamp from when the node sent the update.

Additional context

This makes determining if messages are old or new on restart, or applying logic at receiving end to handle 'old' retained messages differently difficult (impossible).

I have not attached any logs as they are as expected, as noted above not sure if this is a bug or current behavior is as desired. Can add logs if needed.

robertsLando commented 3 years ago

@chrish987 this is by design, when I receive values updates I don't keep a storage of them so I keep the time of when I read the value. @AlCalzone Any thoughts about adding this on zwave-js side? As you are storing all values in a db

AlCalzone commented 3 years ago

You should distinguish between values read from cache and the one that are emitted as value events from zwave-js during the interview. IMO zwavejs2mqtt shouldn't emit updates for values it reads from cache.

I've solved this in iobroker.zwave2 the following way: On "node ready":

On "value added", "value updated", "value removed":

robertsLando commented 3 years ago

I do this as it could be that for some reason aa user has cleaned all the retained values, in that case sending them on startup helps to keep the state updated

AlCalzone commented 3 years ago

But there must be a way to say: "this value is not fresh, don't act on it!", right?

robertsLando commented 3 years ago

@AlCalzone yep I could add a field for that in the payload, but could be doine also by including node info in the payload and make a check on the node stage (that is already available).

chrish987 commented 3 years ago

A bit more background on what I am trying to achieve. I would like the zwave s/w to be the master of storing values related to zwave nodes, and provide a mechanism to query for these values at any time, this should not only be via a pollValue call which for a battery device could wait until the device wakes up, but by a getValue which could retrieve from cache - in this case the time that the actual update was sent by the device is important.

Can zwave-js not store a timestamp of when the update was received from the node?

I gave the example in the ticket of when zwavejs2mqtt is restarted, but i think there are other use cases, for example the getValue to retrieve a value from cache, here the Interview process could be completed (although my understanding of that is a bit limited - sorry) and a value queried could still be 'old' although valid as it is the most recent update from the device. In this scenario knowing the actual time is very useful for whatever workflow is going to be triggered.

I think sending all the updates on a s/w restart is a useful feature, but could maybe be an option, or even better an option to do by default and a button in the GUI to trigger manually if required. Could also be an API action to trigger, there is a startup message send so an external workflow could trigger on this and then manually trigger rebroadcast of all values by API if required.

There is a value in nodeinfo - lastactive - not sure where this comes from but in my testing it seems to update on a restart to the current time, even with no node activity (interview stage is RestartFromCache). Including the nodeinfo in the payload could be useful but i don't think just the interview stage can be used for all logic - as noted above.

Let me know if this is not the general direction for the s/w, as there are other options, I could for example master values in home assistant, however mastering as close to the source as possible seems architecturally 'better'.. even with the values stored externally a timestamp of when the event happened will be useful.

AlCalzone commented 3 years ago

A bit more background on what I am trying to achieve. I would like the zwave s/w to be the master of storing values related to zwave nodes, and provide a mechanism to query for these values at any time, this should not only be via a pollValue call which for a battery device could wait until the device wakes up, but by a getValue which could retrieve from cache

This is already possible in node-zwave-js - the method is called getValue. I'll leave it up to @robertsLando to decide if it makes sense to expose it through zwavejs2mqtt, since MQTT should have all the cached values anyways if I understand correctly, since any changes in the node-zwave-js cache notify zwavejs2mqtt immediately.

There is a value in nodeinfo - lastactive - not sure where this comes from but in my testing it seems to update on a restart to the current time, even with no node activity

zwavejs2mqtt adds this value - I guess it doesn't distinguish between cached and fresh values here?

robertsLando commented 3 years ago

There is a value in nodeinfo - lastactive - not sure where this comes from but in my testing it seems to update on a restart to the current time, even with no node activity

Like for the values, nothing is stored so it is updated when I receive something from a node (or more in general an event from zwavejs regarding that node).

chrish987 commented 3 years ago

any more thoughts on storing the time the value was received from the node in zwave-js and then passing that through to zwave-js2mqtt as payload?

robertsLando commented 3 years ago

@AlCalzone ?

AlCalzone commented 3 years ago

I don't see the huge benefit, but I guess it could be interesting.

chrish987 commented 3 years ago

When (if) this is implemented in zwavejs2mqtt it will be useful to add as an additional value, not replace the existing value. So having fields for the time that zwavejs2mqtt created the message, and then the field from zwave-js that indicates the time the message was received from the node. Should add some flexibility on applying logic.

I think above might help with logic on trying to avoid double handling messages in certain scenarios - although I am just about to start exploring the QoS settings and see what is possible here (only ever bothered with QoS = 0 up to now).

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

chrish987 commented 3 years ago

Any update on this request. I see it has been in the 'soon' section of Triage for some time.

AlCalzone commented 3 years ago

Nope. There are much more pressing issues to be solved.

Gis70 commented 3 years ago

Hi, just to say i'm interesting too by this function.