M-Labs/thermostat
9
0
Fork 5
Code Issues 9 Pull Requests 5 Releases Wiki Activity

Turn off LED L3 only when all channels have no PID #78

Merged
sb10q merged 1 commits from atse/thermostat:l3-led-fix into master 2023-08-07 16:15:25 +08:00
Conversation 1 Commits 1 Files Changed 2 +13 -1
atse commented 2023-08-07 14:10:03 +08:00
Contributor
Copy Link

Change the behaviour of LED L3 to turn off only when all channels have PID disengaged, as opposed to when any channel disengages PID.

Otherwise, when disengaging PID on a Thermostat that has had both channels engaged in PID, the LED would turn off, even when PID is still engaged on the other channel.

This lets the LED better reflect the status of the Thermostat as a whole, as it would stay on as long as PID is engaged on at least one channel.

Change the behaviour of LED L3 to turn off only when all channels have PID disengaged, as opposed to when any channel disengages PID. Otherwise, when disengaging PID on a Thermostat that has had both channels engaged in PID, the LED would turn off, even when PID is still engaged on the other channel. This lets the LED better reflect the status of the Thermostat as a whole, as it would stay on as long as PID is engaged on at least one channel.
mwojcik reviewed 2023-08-07 14:25:55 +08:00
src/command_handler.rs Outdated
@ -189,2 +189,3 @@
channels.channel_state(channel).pid_engaged = false;
leds.g3.off();
// Only turn off LED when PID is disengaged on all channels
if !channels.channel_state(channel ^ 1).pid_engaged {
mwojcik commented 2023-08-07 14:25:55 +08:00
Owner
Copy Link

channel ^ 1 while quick, would cause weird behavior if e.g. we create a new version of the thermostat with more than two channels and re-used the code.

My suggestion would be make a function in Channels for checking the PID state of all channels.

``channel ^ 1`` while quick, would cause weird behavior if e.g. we create a new version of the thermostat with more than two channels and re-used the code. My suggestion would be make a function in ``Channels`` for checking the PID state of all channels.
👍 1
atse commented 2023-08-07 16:14:28 +08:00
Author
Contributor
Copy Link

Thanks for the suggestion, the latest force push loops through all channels to check their PID state.

Thanks for the suggestion, the latest force push loops through all channels to check their PID state.
atse marked this conversation as resolved
sb10q commented 2023-08-07 14:41:46 +08:00
Owner
Copy Link

Yes, we'll probably want to port this to the 4-channel EEM version. I'm not totally convinced with MQTT.

Yes, we'll probably want to port this to the 4-channel EEM version. I'm not totally convinced with MQTT.
atse force-pushed l3-led-fix from add5120f8c to b04a61c414 2023-08-07 16:10:27 +08:00 Compare
sb10q merged commit b04a61c414 into master 2023-08-07 16:15:25 +08:00
atse deleted branch l3-led-fix 2023-08-07 16:18:35 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/thermostat#78
No description provided.
Delete Branch "atse/thermostat:l3-led-fix"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?