-
Notifications
You must be signed in to change notification settings - Fork 3k
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
danfoss: Add thermostat schedule support #3499
Conversation
a9372fd
to
99d738b
Compare
bc617b1
to
e1eeb05
Compare
devices/popp.js
Outdated
@@ -66,7 +67,9 @@ module.exports = [ | |||
.withDescription('Mean radiator load for room calculated by gateway for load balancing purposes (-8000=undefined)') | |||
.withValueMin(-8000).withValueMax(100), | |||
exposes.numeric('load_estimate', ea.STATE_GET) | |||
.withDescription('Load estimate on this radiator')], | |||
.withDescription('Load estimate on this radiator'), | |||
exposes.enum('programming_operation_mode', ea.ALL, ['setpoint', 'schedule']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this expose to exposes.js
, could you also improve the description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this expose to exposes.js
As its own thing or as another withXyz
on the climate expose?
could you also improve the description
Sure, but it will be a little wordy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As its own thing or as another withXyz on the climate expose?
Own thing
Sure, but it will be a little wordy.
No problem, no need to keep things short there.
1c30ecb
to
4195d18
Compare
Looks good to me, if you can fix the merge conflict I can merge it. |
4195d18
to
19f702f
Compare
Sorry for the delay, rebased! |
Is this issue still open? I 'm using Zigbee2MQTT version: 1.22.2 commit: 414c51f |
It is merged and works. You set weekly_schedule over mqtt as an object that looks something like: {
days: 0b1111111, // bitmask of days
mode: 0x1, // heat mode
transitions: [ // times are minutes since midnight
{transitionTime: 660, heatSetpoint: 21.5},
{transitionTime: 1200, heatSetpoint: 18.0},
]
} Remember to change the programming operation mode to schedule for it to work. |
@kennylevinsen , thank you for the answer.
Payoload:
Tested also without the comma before
I also tryed adding |
For reference, this is the code snippet I used: const setSchedule = (schedule, days, radiator) => {
console.log("Setting schedule" + (radiator ? " for " + radiator.key : ""), schedule);
if (!days) {
return;
}
for (let room_type in schedule) {
let transitions = schedule[room_type];
let payload = {
dayofweek: days, // Already a bitmask
mode: 0b01, // Heating
transitions: transitions.map(t => ({
transitionTime: timeFromMidnight(t.time),
heatSetpoint: t.temp,
})),
};
payload.numoftrans = payload.transitions.length; // This should just be deduced by z2m...
radiatorsForType(room_type)
.filter(r => !radiator || radiator.key === r)
.forEach(r => client.publish(r + "/set", JSON.stringify({
weekly_schedule: payload,
programming_operation_mode: "schedule",
})));
};
}; The error you are getting would be from here, and seems to suggest that either there is no field named "transition" in what you sent, or it is not an array. If you are sure that you are sending a well-formed JSON serialized message (remember to JSON.stringify), maybe add a log there to see if what the payload contains? |
I publish MQTT with mosquitto.
I do not use java, so I do not use stringify. in the whole log file, all the entries about weekly schedule look like:
|
If that is what z2m is logging, then what you are sending is not valid JSON. What I quoted is JavaScript, and you need to serialize it as JSON first. It should be enough to convert the binary and hex notated numbers to decimal and remove the trailing comma. |
Oooops, you are right. I just realized that in the test with decimal I made a typo ... corrected. No errors anymore. Now, I still miss 2 points:
|
|
I made some tests and I could not find a way to read back the weekly schedule. I suppose I do not use the correct payload or Z2M doesn't get correct data or doesn't expose it at all. I noticed that I cannot set more than 6 transitions: Z2M doesn't return any error but the TRV simply doesn't apply any transition.
So there is no chance to use 7 or more transistions. I'm going to test separate weekly schedule for today and tomorrow and see if it works. |
The numberOfXYZTrans fields tell you what the device limitation is - they are not something you can change. Doing more than 6 transitions in a day does seem aggressive for a thermostat considering the thermal mass of a room/home. As for getting the weekly schedule, that may be a device bug. I recall someone saying that Danfoss thermostats only reply with their weekly schedule when asked solely for heat schedule, instead of all schedules as z2m asks. Not something I have tested. Would require a Danfoss quirk. |
Note that I have not tested the popp/hive side of things.