[SRTS-A01](Aqara Smart Radiator Thermostat E1) Added smart schedule support #4763

artem-sedykh · 2022-10-09T20:59:18Z

No description provided.

artem-sedykh · 2022-10-09T21:24:16Z

devices/xiaomi.js

artem-sedykh · 2022-10-09T21:44:28Z

Start and end time duration at least 4 hours

The end time must be greater than the start time, minimum duration is one hour

devices/xiaomi.js

problem9 · 2022-10-23T06:50:15Z

is this working, will be this merged?

protyposis · 2022-11-08T20:00:41Z

What is missing to get this merged? What is the general PR review process in this repo?

Koenkk · 2022-11-09T16:50:42Z

@artem-sedykh could you add the schedule_settings to the exposes?

artem-sedykh · 2022-11-09T17:18:24Z

I don't know how to do it, now it works via mqtt

Koenkk · 2022-11-09T18:37:30Z

@artem-sedykh can you provide me an example on what payload schedule_setting has?

protyposis

I am wondering whether the Z2M Frontend supports JSON data, or if the schedule_settings should be additionally stringified to allow modification in the UI.

protyposis · 2022-11-15T17:20:06Z

devices/xiaomi.js

+                    const schedule = {
+                        repeat: [],
+                        start: getTime(2),
+                        end: getTime(20),
+                        values: [
+                            {time: getTime(2), temperature: value.readUint16BE(6)/100},
+                            {time: getTime(8), temperature: value.readUint16BE(12)/100},
+                            {time: getTime(14), temperature: value.readUint16BE(18)/100},
+                        ],
+                    };


This structure contains the start time twice, of which schedule.start is ignored in the toZigbee conversion. This feels unintuitive and is a potential (and avoidable) source of user confusion and error. How about using a structure like the following?

Suggested change

const schedule = {

repeat: [],

start: getTime(2),

end: getTime(20),

values: [

{time: getTime(2), temperature: value.readUint16BE(6)/100},

{time: getTime(8), temperature: value.readUint16BE(12)/100},

{time: getTime(14), temperature: value.readUint16BE(18)/100},

],

};

const schedule = {

repeat: [],

values: [

{time: getTime(2), temperature: value.readUint16BE(6)/100},

{time: getTime(8), temperature: value.readUint16BE(12)/100},

{time: getTime(14), temperature: value.readUint16BE(18)/100},

{time: getTime(20)},

],

};

To better fit the Aqara app's naming, values could also be renamed to actingTimes.

protyposis · 2022-11-15T17:35:36Z

devices/xiaomi.js

+                const getTimeValue = (time) => {
+                    let totalMinutes = time.hours * 60 + time.minutes;
+                    if (time.next_day) {
+                        const offset = 1440 - startTime.hours * 60 + startTime.minutes;
+                        totalMinutes = totalMinutes + offset;
+                    }
+                    return totalMinutes;
+                };


Shouldn't this be const offset = 1440 - startTime.hours * 60 - startTime.minutes (or use braces instead)?

Also I'm wondering whether this makes sense at all. According to this calculation, "same day" times are absolute (relative to 00:00), while a "next day" time is calculated relatively to the start time. Is this really desired?

protyposis · 2022-11-15T17:37:31Z

devices/xiaomi.js

+                    throw new Error('"repeat" must have at least one value');
+                }
+
+                const startTime = {...value.values[0].time, next_day: false};


In reference to a previous comment: If you keep the current schedule format, it would be helpful to assert that value.from equals value.values[0].time, or a least log a mismatch warning which says that data is inconsistent and that the start time will be ignored.

protyposis · 2022-11-15T18:00:58Z

devices/xiaomi.js

+                    let repeat = 254;
+                    for (let i = 1; i <= 7; i++) {
+                        if (value.repeat.some((e) => e.toUpperCase() == dayNames[i - 1].toUpperCase()) == false) {
+                            repeat = repeat & ~(1 << i);
+                        }
+                    }


Could be cleaned up to something like this (better names, simplified expressions, zero-based loop index to better match the parsing code in fromZigbee):

Suggested change

let repeat = 254;

for (let i = 1; i <= 7; i++) {

if (value.repeat.some((e) => e.toUpperCase() == dayNames[i - 1].toUpperCase()) == false) {

repeat = repeat & ~(1 << i);

}

}

const normalizedRepeatDays = value.repeat.map(dayName => dayName.toLowerCase());

let repeat = 0;

for (let i = 0; i < dayNames.length; i++) {

if (normalizedRepeatDays.includes(dayNames[i])) {

repeat = repeat | (1 << i+1);

}

}

or even

Suggested change

let repeat = 254;

for (let i = 1; i <= 7; i++) {

if (value.repeat.some((e) => e.toUpperCase() == dayNames[i - 1].toUpperCase()) == false) {

repeat = repeat & ~(1 << i);

}

}

const normalizedRepeatDays = value.repeat.map(dayName => dayName.toLowerCase());

const repeat = dayNames.reduce((repeat, dayName, index) => {

const isDaySelected = normalizedRepeatDays.includes(dayName);

return repeat | isDaySelected << index+1;

}, 0);

0ip · 2022-11-28T16:51:28Z

Not much is happening here anymore, unfortunately, but I think the feature is relatively important to many owners of that thermostat, me included.

@protyposis Would it be ok if you open a new pull request based on the excellent work of @artem-sedykh and your suggestions?

protyposis · 2022-11-29T23:14:36Z

devices/xiaomi.js

+                case 0x027d:
+                    result['schedule'] = {1: 'ON', 0: 'OFF'}[value];
+                    break;


How does this relate to the preset setting? Shouldn't the schedule be active when preset is set to auto, or does auto mean something different? What happens when I set preset to manual or away, and at the same time schedule to ON?

protyposis · 2022-11-29T23:18:54Z

@0ip I'd love to but I'm afraid I'm missing the background to be able to decide what makes sense here. The only knowledge I have of Zigbee messages in general and the Aqara TRV specifically is what I read in this PR, and that unfortunately seems insufficient at this point.

Koenkk · 2022-11-30T15:42:04Z

@0ip feel free to make a new pr

0ip · 2022-12-01T11:37:15Z

@Koenkk I find myself in a similar situation as @protyposis, sorry

protyposis · 2022-12-01T13:08:59Z

Gave it a try anyway: #5058, @Koenkk if you could be so kind and review please :)

github-actions · 2023-01-01T01:17:15Z

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

Koenkk · 2023-01-01T08:54:39Z

I believe this one can be closed in favour of #5058

Added scheduling support

d902a98

artem-sedykh mentioned this pull request Oct 9, 2022

[New device support]: Aqara Smart Radiator Thermostat E1 (SRTS-A01) Koenkk/zigbee2mqtt#13993

Closed

fix

f71d730

refactoring

314a8d4

artem-sedykh changed the title ~~[SRTS-A01](Aqara Smart Radiator Thermostat E1) Added scheduling support~~ [SRTS-A01](Aqara Smart Radiator Thermostat E1) Added smart schedule support Oct 9, 2022

add validation

feaa404

trullock reviewed Oct 9, 2022

View reviewed changes

devices/xiaomi.js Outdated Show resolved Hide resolved

fix

16d05df

Koenkk reviewed Oct 10, 2022

View reviewed changes

devices/xiaomi.js Outdated Show resolved Hide resolved

Koenkk reviewed Oct 10, 2022

View reviewed changes

devices/xiaomi.js Outdated Show resolved Hide resolved

Koenkk reviewed Oct 10, 2022

View reviewed changes

devices/xiaomi.js Show resolved Hide resolved

artem.sedykh added 2 commits October 10, 2022 18:32

add description

9b5371a

refactoring

a7b4352

protyposis reviewed Nov 15, 2022

View reviewed changes

protyposis reviewed Nov 29, 2022

View reviewed changes

protyposis mentioned this pull request Dec 1, 2022

Aqara SRTS-A01 smart schedule #5058

Merged

github-actions bot added the stale label Jan 1, 2023

Koenkk closed this Jan 1, 2023

protyposis mentioned this pull request Jan 18, 2023

Aqara SRTS-A01 heartbeat decoding (fixes battery percentage and firmware version) #5363

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SRTS-A01](Aqara Smart Radiator Thermostat E1) Added smart schedule support #4763

[SRTS-A01](Aqara Smart Radiator Thermostat E1) Added smart schedule support #4763

artem-sedykh commented Oct 9, 2022

artem-sedykh commented Oct 9, 2022

artem-sedykh commented Oct 9, 2022

problem9 commented Oct 23, 2022

protyposis commented Nov 8, 2022

Koenkk commented Nov 9, 2022

artem-sedykh commented Nov 9, 2022

Koenkk commented Nov 9, 2022

protyposis left a comment

protyposis Nov 15, 2022

protyposis Nov 15, 2022

protyposis Nov 15, 2022

protyposis Nov 15, 2022

0ip commented Nov 28, 2022

protyposis Nov 29, 2022

protyposis commented Nov 29, 2022

Koenkk commented Nov 30, 2022

0ip commented Dec 1, 2022

protyposis commented Dec 1, 2022

github-actions bot commented Jan 1, 2023

Koenkk commented Jan 1, 2023

-                    let repeat = 254;
-                    for (let i = 1; i <= 7; i++) {
-                        if (value.repeat.some((e) => e.toUpperCase() == dayNames[i - 1].toUpperCase()) == false) {
-                            repeat = repeat & ~(1 << i);
-                        }
-                    }
+                    const normalizedRepeatDays = value.repeat.map(dayName => dayName.toLowerCase());
+                    let repeat = 0;
+                    for (let i = 0; i < dayNames.length; i++) {
+                        if (normalizedRepeatDays.includes(dayNames[i])) {
+                            repeat = repeat | (1 << i+1);
+                        }
+                    }

[SRTS-A01](Aqara Smart Radiator Thermostat E1) Added smart schedule support #4763

[SRTS-A01](Aqara Smart Radiator Thermostat E1) Added smart schedule support #4763

Conversation

artem-sedykh commented Oct 9, 2022

artem-sedykh commented Oct 9, 2022

artem-sedykh commented Oct 9, 2022

problem9 commented Oct 23, 2022

protyposis commented Nov 8, 2022

Koenkk commented Nov 9, 2022

artem-sedykh commented Nov 9, 2022

Koenkk commented Nov 9, 2022

protyposis left a comment

Choose a reason for hiding this comment

protyposis Nov 15, 2022

Choose a reason for hiding this comment

protyposis Nov 15, 2022

Choose a reason for hiding this comment

protyposis Nov 15, 2022

Choose a reason for hiding this comment

protyposis Nov 15, 2022

Choose a reason for hiding this comment

0ip commented Nov 28, 2022

protyposis Nov 29, 2022

Choose a reason for hiding this comment

protyposis commented Nov 29, 2022

Koenkk commented Nov 30, 2022

0ip commented Dec 1, 2022

protyposis commented Dec 1, 2022

github-actions bot commented Jan 1, 2023

Koenkk commented Jan 1, 2023