fix: restore sh1btabb spec, add a7sghmms #5567

drakhart · 2023-03-11T20:39:13Z

Apply fixes as discussed in #4929

Restore sh1btabb spec using minutes, added conversion from GMT+8 to local timezone.
Add a7sghmms spec using seconds and local timezone.
Small refactor: added constants, removed magic numbers...

drakhart · 2023-03-11T21:09:00Z

devices/giex.js

+                    case dataPoints.giexWaterValve.irrigationEndTime:
+                        return {[keys.giexWaterValve.irrigationEndTime]: timezone ? toLocalTime(value, timezone) : value};
+                    case dataPoints.giexWaterValve.lastIrrigationDuration:
+                        return {[keys.giexWaterValve.lastIrrigationDuration]: value.split(',').shift()}; // Remove meaningless ,0 suffix


Note: for _TZE200_a7sghmms I've always seen ,0 (i.e. 00:02:10,0 instead of a proper fraction of a second, that's why I'm removing this.

devices/giex.js

drakhart · 2023-03-11T23:38:43Z

I'm converting this to draft until we clarify this doubt about model names (the current ones in the PR are quite unfriendly): #4929 (comment)

WhimsySpoon · 2023-03-12T11:07:59Z

@drakhart I've tried the new file as a custom converter and have noticed a couple of issues on a QT06 (sh1btabb):

The device image no longer appears in the Z2M interface
The irrigation_start_time and irrigation_end_time are now GMT+1 (I'm GMT local)

Everything else looks great and it's correctly identified as a model QT06 (sh1btabb) in the UI.

WhimsySpoon · 2023-03-12T11:49:36Z

@drakhart I don't know why, but changing the year in the toLocalTime function from 1970 to 1900 fixes my timezone issue.

So line 18 becomes:

const local = new Date(`1900-01-01T${time}.000${timezone}`);

drakhart · 2023-03-12T12:30:47Z

The device image no longer appears in the Z2M interface

This is most likely related to the change in model names, as well as the broken documentation link I mention in this comment: #5567 (comment)

I guess we'll need to create new markdown files and images into the zigbee2mqtt.io repo.

The irrigation_start_time and irrigation_end_time are now GMT+1 (I'm GMT local)

That's weird. With this conversion I'm getting exactly 7 hours less than the time reported by the device, which is the proper one for me (I'm on CET / GMT+1).

The 1900 fix sounds a bit hacky to me since Unix epoch starts in January 1st 1970, and also feels flaky because setting that I get a weird time difference (i.e. it converts 13:25:08 to 05:10:08 instead of he correct 06:25:08).

Can you please check the system time on the machine you're running Z2M in, and make sure it's set to the proper timezone? Example:

❯ date
Sun Mar 12 13:16:58 CET 2023

WhimsySpoon · 2023-03-12T12:35:06Z

Agreed on the hackiness; I can't explain why it makes a difference. I did some debugging and the timezone is GMT+8 (I'm in GMT) and the time was coming through as 8 hours from now.

I'm running Z2M Add-on in Home Assistant OS and the time is correct:

drakhart · 2023-03-12T12:43:01Z

This feels very, very weird. Would you mind putting a year somewhere in the middle of epoch time, let's say 2000, instead of 1900 or 1970? Just to make sure this is not some edge case caused by the conversion returning a value below epoch's minimum date.

WhimsySpoon · 2023-03-12T12:45:58Z

I changed it to:

const local = new Date(`2000-01-01T${time}.000${timezone}`);

And it's calculating the timezone correctly. I don't understand it. The code as is, with 1970, works perfectly outside of this environment.

drakhart · 2023-03-12T12:49:16Z

I can't explain it either, just wanted to make sure. But seeing that 2000 works properly for both you and me I'll assume that it will also work for people in other timezones, so I'll apply it to the PR.

WhimsySpoon · 2023-03-12T13:00:45Z

Sounds good. Happy to test anything else as needed.

drakhart · 2023-03-12T15:55:20Z

Opened the following PR for the docs & images: Koenkk/zigbee2mqtt.io#1947

devices/giex.js

WhimsySpoon · 2023-03-13T20:29:44Z

I've tested the current changes and the irrigation_end_time no longer changes to --:--:-- when the tap is open.

Instead, when the tap opens, the irrigation_end_time is the irrigation_start_time \ now plus one minute.

drakhart · 2023-03-13T21:21:40Z

I've tested the current changes and the irrigation_end_time no longer changes to --:--:-- when the tap is open.

Instead, when the tap opens, the irrigation_end_time is the irrigation_start_time \ now plus one minute.

It's happening for me too and I didn't notice, thanks for the heads up. It's really weird though, let me look into it.

WhimsySpoon · 2023-03-13T21:24:13Z

I did have a brief go at debugging but didn't have enough time. All I can be sure of is that it's not making into the function that converts the time to local.

drakhart · 2023-03-13T23:10:12Z

I'm getting the following behaviors:

If I turn it on with the physical button on the device end time is --:--:-- until I press it again, as expected.
If I turn it on from Z2M in capacity mode the end time is also --:--:--, as expected (it doesn't know how much time will it take to serve X litres).
If I turn it on from Z2M in duration mode the end time is start time + duration, unless I manually stop it (it then updates to the new end time). I'm not sure, but I'd say this is expected too: it's smart enough to know the end time.

All in all, I don't see anything unusual.

Are you sure it doesn't get into the toLocalTime function? It works fine in my system if I change the model names (I don't own the GMT+8 model, so I need to force it somehow).

WhimsySpoon · 2023-03-14T06:33:05Z

I'll need to double check when I get home tonight, but previous versions definitely had the end time as dashes whilst the tap was open. I'll have a play later and see if I can find out where irrigation_start_time+00:01:00 is getting injected.

Koenkk · 2023-03-14T18:57:46Z

Looks good now, please let me know once this is ready for merge.

WhimsySpoon · 2023-03-15T10:53:37Z

@drakhart Apologies for the delay on this one. I've conducted some thorough testing and was really struggling to get my head around this. I effectively debugged each line of relevant code to see where it was getting overwritten, but could never see where --:--:-- was being lost.

I then decided to roll-back to the previous version of the converter, and it was still doing it! I went as far as pulling the converter entirely. Still doing it. I then wondered if it was something set on the device itself, so I removed and repaired it. Still doing it.

After sleeping on the issue, I then toggled the mode which had at some point been set back to duration. Changing it back to capacity had it working back again, showing --:--:-- in irrigation_end_time. It seems that having mode set to duration and irrigation_target at 0 meant that the irrigation_end_time would behave as I discovered above.

My huge apologies, @drakhart, there was nothing wrong with your code at all. I've just tested the latest code in this PR and it's working perfectly when using mode as capacity.

EDIT: GMT calculation seems to be off again - checking...

WhimsySpoon · 2023-03-15T11:59:31Z

I think I spoke to soon. It looks like the issue is with it not respecting irrigation_target when it's set to 0, regardless of mode, it only opens with the physical button.

When the irrigation_target is set to 0 and the valve is opened via Z2M, something is happening inside the device (a small click can be heard, but it's not the same sound as the valve opening up), it then immediately closes again. Using the physical button works fine.

I think this may be a limitation of this devices firmware which is possibly solved in the later version. The only work-around I can find is setting the irrigation_target to the largest possible value.

Either way, it's not a problem with this code as the issue is present with the current code and this release.

Apologies for the back-and-forth on this one, @drakhart, and thanks again for all of your efforts.

drakhart · 2023-03-15T12:37:24Z

No problem, @WhimsySpoon, better be safe than sorry. Thanks for taking your time double-checking all these things. ❤️

It indeed sounds like a limitation with sh1btabb, because setting the irrigation target to zero on a7sghmms leaves the valve open until manually closed. Let me update sh1btabb description removing this mention to the zero value to avoid misleading the end user and we'll be good to merge.

drakhart · 2023-03-15T12:40:31Z

@Koenkk we're all good, please merge when you have the time. 👍 Thanks.

Koenkk · 2023-03-15T16:15:11Z

Thanks!

fedegrc · 2023-03-16T03:15:04Z

I have tested the _TZE200_a7sghmms several times this week with no problems. Thank @drakhart! Good job

WhimsySpoon · 2023-04-01T11:51:27Z

@drakhart The UK recently moved to British Summer Time, or BST which is GMT+1 and the local times are still showing as GMT, so an hour behind current.

You're not going to believe this, but changing the following back to 1970 fixes this:

const local = new Date(`2000-01-01T${time}.000${timezone}`);

I'm going to have a play with this and try and get to the bottom of it before I open a new PR.

drakhart · 2023-04-01T18:24:02Z

@drakhart The UK recently moved to British Summer Time, or BST which is GMT+1 and the local times are still showing as GMT, so an hour behind current.

You're not going to believe this, but changing the following back to 1970 fixes this:
const local = new Date(`2000-01-01T${time}.000${timezone}`);
I'm going to have a play with this and try and get to the bottom of it before I open a new PR.

Ok, please let me know your findings and let's see if we can get this sorted out once and for all. 🤞

fix: restore sh1btabb spec, add a7sghmms

b0c0717

drakhart mentioned this pull request Mar 11, 2023

Fix GiEX QT06 water valve units #4929

Merged

drakhart commented Mar 11, 2023

View reviewed changes

devices/giex.js Outdated Show resolved Hide resolved

refactor: move timezone into model object

cf38fb2

drakhart marked this pull request as draft March 11, 2023 23:36

fix: replace base year in toLocalTime

5939bbd

fix: give friendlier names to models

9894a90

drakhart marked this pull request as ready for review March 12, 2023 15:40

drakhart requested a review from Koenkk March 12, 2023 15:40

drakhart mentioned this pull request Mar 12, 2023

feat: add QT06 variant Koenkk/zigbee2mqtt.io#1947

Closed

Koenkk reviewed Mar 13, 2023

View reviewed changes

devices/giex.js Outdated Show resolved Hide resolved

drakhart added 3 commits March 13, 2023 18:41

refactor: swap QT06_1 & QT06_2 model names

321fa37

feat: custom fz model converters

2495db5

feat: add safety min secs for a7sghmms

5756212

drakhart requested a review from Koenkk March 13, 2023 19:02

style: fix comment typo

c0d120b

fix: irrigation target desc for sh1btabb

a71fa20

Koenkk merged commit 2e2383c into Koenkk:master Mar 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: restore sh1btabb spec, add a7sghmms #5567

fix: restore sh1btabb spec, add a7sghmms #5567

drakhart commented Mar 11, 2023

drakhart Mar 11, 2023

drakhart commented Mar 11, 2023

WhimsySpoon commented Mar 12, 2023 •

edited

Loading

WhimsySpoon commented Mar 12, 2023 •

edited

Loading

drakhart commented Mar 12, 2023 •

edited

Loading

WhimsySpoon commented Mar 12, 2023

drakhart commented Mar 12, 2023

WhimsySpoon commented Mar 12, 2023

drakhart commented Mar 12, 2023

WhimsySpoon commented Mar 12, 2023

drakhart commented Mar 12, 2023

WhimsySpoon commented Mar 13, 2023

drakhart commented Mar 13, 2023

WhimsySpoon commented Mar 13, 2023

drakhart commented Mar 13, 2023 •

edited

Loading

WhimsySpoon commented Mar 14, 2023 •

edited

Loading

Koenkk commented Mar 14, 2023

WhimsySpoon commented Mar 15, 2023 •

edited

Loading

WhimsySpoon commented Mar 15, 2023

drakhart commented Mar 15, 2023

drakhart commented Mar 15, 2023

Koenkk commented Mar 15, 2023

fedegrc commented Mar 16, 2023

WhimsySpoon commented Apr 1, 2023

drakhart commented Apr 1, 2023

fix: restore sh1btabb spec, add a7sghmms #5567

fix: restore sh1btabb spec, add a7sghmms #5567

Conversation

drakhart commented Mar 11, 2023

drakhart Mar 11, 2023

Choose a reason for hiding this comment

drakhart commented Mar 11, 2023

WhimsySpoon commented Mar 12, 2023 • edited Loading

WhimsySpoon commented Mar 12, 2023 • edited Loading

drakhart commented Mar 12, 2023 • edited Loading

WhimsySpoon commented Mar 12, 2023

drakhart commented Mar 12, 2023

WhimsySpoon commented Mar 12, 2023

drakhart commented Mar 12, 2023

WhimsySpoon commented Mar 12, 2023

drakhart commented Mar 12, 2023

WhimsySpoon commented Mar 13, 2023

drakhart commented Mar 13, 2023

WhimsySpoon commented Mar 13, 2023

drakhart commented Mar 13, 2023 • edited Loading

WhimsySpoon commented Mar 14, 2023 • edited Loading

Koenkk commented Mar 14, 2023

WhimsySpoon commented Mar 15, 2023 • edited Loading

WhimsySpoon commented Mar 15, 2023

drakhart commented Mar 15, 2023

drakhart commented Mar 15, 2023

Koenkk commented Mar 15, 2023

fedegrc commented Mar 16, 2023

WhimsySpoon commented Apr 1, 2023

drakhart commented Apr 1, 2023

WhimsySpoon commented Mar 12, 2023 •

edited

Loading

WhimsySpoon commented Mar 12, 2023 •

edited

Loading

drakhart commented Mar 12, 2023 •

edited

Loading

drakhart commented Mar 13, 2023 •

edited

Loading

WhimsySpoon commented Mar 14, 2023 •

edited

Loading

WhimsySpoon commented Mar 15, 2023 •

edited

Loading