Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Lidl / Parkside watering timer #5710

Merged
merged 16 commits into from
May 8, 2023

Conversation

zinserjan
Copy link
Contributor

This adds full support for the lidl watering timer as requested in Koenkk/zigbee2mqtt#7695. Tuya dp converter format is used when possible.

Tested locally with npm link against the latest zigbee2mqtt:

  • on/off
  • timer
  • reporting of time left (manual watering + scheduled watering)
  • battery
  • time sync
  • scheduling / time slot config (even partial configs are possible to simplify creation of custom mqtt entities in HA)
  • frost lock

Open tasks (won't fix):

  • prevention of overlapping time slot configurations (e.g. due to iterations + pause)
    • device skips/ignores overlapping time slots, only the first one wins
    • reporting will be correct for the active slot (on/off, time left)

This it how it looks like:
image

Btw while testing I discovered that the initial reporting of the device was unreliable in my production setup, but worked pretty good in my new dev setup. In my previous converter I got around this with a delay of one second before sending the tuya magic, but this doesn't help always.
The only real difference in my setup between dev and production was the used firmware. Since I updated the firmware in my production environment as well, the initial reporting seems to be working pretty solid, too.

  • bumped cc2652p coordinator from 20220103 to 20221226
  • bumped cc2652p router from 20220125 to 20221102
Info: data restrictions by the original gateway

Note: Device may accepts higher values or invalid for some of these, but the following are the officially used ones.

  • Timer: 1 - 599 min
  • Scheduler: weekday (monday to sunday), periodic (every 1 -7 days) or disabled
  • Timeslot (1 - 6):
    • start hour: 0 - 23 (255 seems to be reported for never used slots, but it's technically invalid)
    • start minute: 0 - 59 (255 seems to be reported for never used slots, but it's technically invalid)
    • timer: 1 - 599 min (0 seems to be reported for never used slots, but it's technically invalid)
    • pause: 0 - 599
    • iterations: 1 - 9 (0 seems to be reported for never used slots, but it's technically invalid)

lib/tuya.js Show resolved Hide resolved
@zinserjan
Copy link
Contributor Author

Ah linting fails against the merged state. Seems that utils was removed in the mean time. I'll fix this.

devices/lidl.js Outdated
@@ -140,6 +141,96 @@ const fzLocal = {
}
},
},
watering_time_left: {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you refactor this to a valueconverter? (such that we only use tuya.tz.datapoints for this device)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this should be possible, because this relies only on the ON/OFF DP and the current/previous state.
But then I need to replace the continues reporting of the remaining time by a JS interval. Is it possible to report state changes programmatically?

The current approach uses the continuous reporting of the battery level (every minute) to update the remaining time. But if there is way to do it programmatically, I can refactor this.

What I mean is something like that

var timer;
function  valudeContverter() {
	if (ON) {
	  timer = setInterval(() => {
		// report state programmatically --> how?
		device.updateState({time_left: xxx});
		}, 1000 * 60);
	} else {
		clearInterval(timer);
		timer = undefined;
	}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a publish method, which is used in another converter to publish the state programmatically. This should work here too, if we pass it down to the DP.from converter.

@Koenkk does it makes sense this way?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zinserjan yes, feel free to pass the publish method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now 😊

devices/lidl.js Outdated
[1, null, valueConverterLocal.wateringState],
// disable optimistic state reporting (device may not turn on when battery is low)
[1, 'state', tuya.valueConverter.onOff, {optimistic: false}],
[5, 'timer', tuya.valueConverter.range(1, 599)],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The range is already specified in the exposes, therefore there is no need to do it again here. I suggest to use tuya.valueConverter.raw here and remove tuya.valueConverter.range

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I used this before but I noticed there isn't any validation. At least I was able to set unsupported values via frontend and MQTT messages. But that's a different story.

devices/lidl.js Outdated
toZigbee: [
{
...tuya.tz.datapoints,
key: ['state', 'timer', 'reset_frost_lock', 'schedule_periodic', 'schedule_weekday',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add these keys directly to tuya.tz.datapoints, this line should just be toZigbee: [tuya.tz.datapoints]

@Koenkk Koenkk merged commit 272df49 into Koenkk:master May 8, 2023
1 check passed
@Koenkk
Copy link
Owner

Koenkk commented May 8, 2023

Many thanks!

@BDV77
Copy link

BDV77 commented May 13, 2023

Wow this looks great, can't wait to get this in the next release and test it out.

Many thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants