zwave-js / node-zwave-js

Z-Wave driver written entirely in JavaScript/TypeScript
https://zwave-js.github.io/node-zwave-js/
MIT License
755 stars 614 forks source link

setting clock: timezone applied twice #6386

Closed chrand closed 1 year ago

chrand commented 1 year ago

Checklist

Deploy method

Docker

Z-Wave JS UI version

9.0.3.c37f5a4

ZwaveJS version

12.0.2

Describe the bug

Z-Wave JS sends clock to nodes, applying its own timezone, also to devices which already have a timezone setting. Therefore, nodes will show time = UTC + timezone_controler + timezone_node (one timezone too much).

To Reproduce

How to reproduce: in Z-WaveJS UI -> [node] -> advanced -> set clock, I push clock from server to nodes. Operation completes successfully, and nodes get updated. However, device shows wrong clock, off by yet-another timezone setting. In my case: Z-WaveJS UI is deployed on docker, setting timezone (UTC+1). Nodes are thermostats, with a configured setting of the same timezone (UTC+1), among their node configuration.

Expected behavior

Server could optionally send clock, without timezone correction (i.e. in UTC). Having nodes in UTC would not be the best workaround, because of automation (programmed on node).

Additional context

Controller is an usb, Silicon Labs, Aeotec 7-Stick, f/w. 7.19.1

robertsLando commented 1 year ago

Z-WaveJS UI is deployed on docker, setting timezone (UTC+1). Nodes are thermostats, with a configured setting of the same timezone (UTC+1), among their node configuration.

Are you using the TZ env var on your docker-compose file?

chrand commented 1 year ago

Are you using the TZ env var on your docker-compose file?

Yes, the docker-compose is:

zwave-js-ui:
  image: zwavejs/zwave-js-ui:latest
  ...
  environment:
    - TZ=...
robertsLando commented 1 year ago

@AlCalzone any clue?

Please make a driver log, loglevel debug and attach it here as a file (drag & drop into the text field). Do that so it include a set clock call

AlCalzone commented 1 year ago

Yeah I'll need to see the driver log of a re-interview. Setting time is complex as there are about 4 ways to do that - some support timezone, some don't. And it's not impossible that the device has a bug or there's some incorrect assumption involved.

AlCalzone commented 1 year ago

@chrand FYI I moved the issue

chrand commented 1 year ago

@AlCalzone @robertsLando Thank you for the replies. Logs are attached. The portion about setting and acknowledging the clock is zwavejs_current.log, [Node 003], lines 478706 and onwards. In the message, Time is sent as UTC+server_timezone. The node then displays it by adding its own timezone :/

Apologies if file is a bit big (these days I always run with debug on file, to check for fw bugs and topology shifts), I kept it uncut because maybe some earlier messages about the same node could be useful. Network is 1 controller + 11 thermostats, all same product/vendor id's.

zwave-js-ui-store.zip

AlCalzone commented 1 year ago

Thanks - that confirms that the time is indeed sent as local time. This is done using Time Parameters CC, which is meant to take UTC, but some devices don't support any way to configure the time zone, and interpret UTC as local time, resulting in the opposite problem.

Z-Wave JS tries to detect whether local time needs to be used instead and this check triggers for your device. I'm not sure why, so I really need a re-interview of the node in the log.

chrand commented 1 year ago

Quick question: what about a UI setting (checkbox?) in Node/Advanced/Set Time, to send time as UTC? It can be pre-filled with the default from the check triggers, and still allow the user to manually override if needed.

Re-interview is ongoing (has been for more than 1 hour), I'll post what I have, seems there have been errors in parsing thermostat fields.

chrand commented 1 year ago

Log with re-interview is attached. Line 507365 starts the interview for [Node 003]; interview seems to stop around line 510503, with an error in parsing the returned field. From the UI, the spinning marker for the interview is still ongoing. Maybe it didn't complete because of the above error.

zwave-js-ui-store-node_interview.zip

AlCalzone commented 1 year ago

Looks like you have some connectivity issues, which causes commands to be repeated and/or fail right at the moment when Z-Wave JS tries to determine how to interpret the thermostat setpoints. You'll have to repeat the re-interview to hopefully get that resolved. Also might be worth taking a look at this and this

As for the original issue: The device actually supports no other time-related CCs, so it shouldn't know the timezone at least from the Z-Wave perspective. I think the way forward here is to add a compat flag, so we can mark this device as requiring UTC.

chrand commented 1 year ago

@AlCalzone Thank you for the suggestions, appreciated!