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

atse · 2023-08-07T14:10:03+08:00

atse commented

2023-08-07 14:10:03 +08:00

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.

atse added 1 commit 2023-08-07 14:10:04 +08:00

add5120f8c Turn off LED L3 only when all channels have no PID

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

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

Poster

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

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.

No reviewers

No Label

No Milestone

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