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

[max] Setting temperature can inadvertently change automatic/manual mode #4347

Closed
cubistico opened this issue Dec 10, 2018 · 4 comments
Closed
Assignees

Comments

@cubistico
Copy link

cubistico commented Dec 10, 2018

Expected Behavior

I'm using my thermostats in automatic mode by default. In order to set something like an "away mode", I've set up a rule that sets the thermostat to mode MANUAL, then sets a lower temperature. Example:

    allThermostats_ControlMode.sendCommand("MANUAL")
    allThermostats_SetTemperature.sendCommand(18)

This should result in the thermostat being set to a set temperature of 18° C and manual mode.

Current Behavior

What happens is that the thermostat is set to manual mode at first, but when the temperature is set, it switches back to automatic mode.

2018-12-10 18:18:26.666 [ome.event.ItemCommandEvent] - Item 'LivingRoomThermostat_ControlMode' received command MANUAL
2018-12-10 18:18:26.675 [ome.event.ItemCommandEvent] - Item 'LivingRoomThermostat_SetTemperature' received command 19.0
2018-12-10 18:18:26.679 [vent.ItemStateChangedEvent] - LivingRoomThermostat_ControlMode changed from AUTOMATIC to MANUAL
2018-12-10 18:18:26.688 [vent.ItemStateChangedEvent] - LivingRoomThermostat_SetTemperature changed from 20.5 to 19.0
2018-12-10 18:18:33.503 [vent.ItemStateChangedEvent] - MAXCubeLANGateway_DutyCycle changed from 49 to 52
2018-12-10 18:18:37.173 [vent.ItemStateChangedEvent] - LivingRoomThermostat_ControlMode changed from MANUAL to AUTOMATIC

See also this forum thread.

Possible Solution(s)

I'm asking for your assessment regarding the fix I propose!

Approach 1: Update internal status when sending commands

I've been trying to dig into the source code, and I believe this is the problem:

  • The protocol requires the setpoint temperature to be sent in combination with the control mode.
  • When the control mode MANUAL is set, a command is sent to the cube, but the changed control mode is not reflected in the device object until it gets an update from the cube.
  • When next the setpoint temperature is updated, the command is constructed using the outdated information and sends a combination of the temperature value with the old AUTOMATIC mode.

The obvious solution is to update the internal value of the device object at the same time when sending out the command. What are the implications? It would mean that the data of the internal device object may not reflect the actual device data, if, for unknown reasons, the command fails. (When could that happen?) On the other hand, now that the internal status is not updated, the device object definitely holds incorrect data for a while until it's been updated.

In the worst case, with the proposed fix applied, the command would be sent but fails to update the thermostat for some reason. The device object status would then stay wrong for an undefined time period. On the other hand, I see no reason why the command would fail other than the device (or cube) being unreachable. In this case, the problems are on a more basic level, and the device status would be unknown anyway.

Pros

  • allows to use the binding in the standard way
  • meets the user's expectations
  • allows for simple rules

Cons

  • may lead to incorrect control mode representation, if the command fails

Approach 2: Allow to set control mode with setpoint temperature

Another approach would be to reflect the way the protocol is designed by allowing to set the control mode with the setpoint temperature. Practically, this could be done by allowing String type parameters that add the control mode to the temperature, like

LivingRoomThermostat_SetTemperature.sendCommand("18°C MANUAL")

Pros

  • allows to use the protocol in the way it actually works
  • saves an update (reduced duty cycle)
  • no side effects
  • fully backward compatible

Cons

  • exposes internals of the protocol
  • works different from other binding
  • would not work with standard items

Approach 3: Delay and combine commands

A more complicated, but from the users' view elegant solution would be to delay the sending of the commands and combine them into one. For example, if a change is received over the control mode channel, the binding could wait for a short (configurable) time, for example in the range of 50-500 ms. If during this delay, the setpoint temperature is changed, both settings would be combined and sent in one command.

Pros

  • allows to use the protocol in the way it actually works
  • saves an update (reduced duty cycle)
  • no side effects
  • fully backward compatible, if the default value for the delay would be 0 ms (= none)

Cons

  • increases complexity of the implementation

Alternative Work-around
A work-around (outside the binding implementation) would be to set up the rule in a way that, after sending the control mode command, it waits until the cube has sent back an update about the control mode change before sending out the new setpoint temperature. According to my observations, the update back takes around 30 seconds to arrive. To be on the safe side, one could add a delay of 60 seconds. A cleaner - yet more complicated - work-around would to wait for the specific expected update. However, both variants can lead to additional complications, like how to handle updates of the control mode and/or setpoint temperature that occur in between the two commands?

The downside is that you always need a quite complex rule to handle switching temperature and control mode at the same time. If you change control mode and temperature manually (using two different widgets in the UI), you won't be able to handle it without using proxy items and rules. Quite a complex approach for such a simple and basic requirement. I consider this not acceptable for the user.

Steps to Reproduce (for Bugs)

Example items:

Number LivingRoomThermostat_SetTemperature (LivingRoomThermostat, allThermostats_SetTemperature) [ "TargetTemperature" ] {channel="max:wallthermostat:KEQ0644073:MKF0043919:set_temp"}
String LivingRoomThermostat_ControlMode "Steuermodus" (LivingRoomThermostat, allThermostats_ControlMode) ["homekit:HeatingCoolingMode"] {channel="max:wallthermostat:KEQ0644073:MKF0043919:mode"}

Switch AwaySwitch "Away mode" <switch>

Example rule:

rule "Away mode"
when
	Item AwaySwitch received command ON
then
       LivingRoomThermostat_ControlMode.sendCommand("MANUAL")
       LivingRoomThermostat_SetTemperature.sendCommand(18)
end

Context

I'm trying to set up a rule that detects when no one is home and if so, lowers the temperature. I usually run all my thermostats in automatic mode. I know that some prefer not to use automatic mode at all, and that I wouldn't experience this issue if I used only manual mode, but I find the automatic mode very convenient and do no want to do without it.

Your Environment

  • openHAB 2.3.0
  • openHABIAN
  • Raspberry Pi
@cubistico
Copy link
Author

I'm planning to go with approach 3 (Delay and combine commands). It looks like I can simply steal much of the code from the Homematic binding.

marcelrv added a commit that referenced this issue Jan 29, 2019
Addresses issue Setting temperature can inadvertently change
automatic/manual mode #4347

Signed-off-by: Marcel Verpaalen <[email protected]>
@cweitkamp cweitkamp added the PR pending There is a pull request for resolving the issue label Jan 29, 2019
@Alestrix
Copy link

I'm planning to go with approach 3 (Delay and combine commands). It looks like I can simply steal much of the code from the Homematic binding.

Seems like #22a8575 follows approach 1. :-)

@cubistico
Copy link
Author

Uh, okay... Thanks for updating me!

@cubistico
Copy link
Author

Can someone assign this issue to him then to reduce confusion? :-)

@cweitkamp cweitkamp removed the PR pending There is a pull request for resolving the issue label Jan 30, 2019
cweitkamp pushed a commit that referenced this issue Jan 30, 2019
* Fix inadvertently change automatic/manual mode

Addresses issue Setting temperature can inadvertently change
automatic/manual mode

Fixes #4347

Signed-off-by: Marcel Verpaalen <[email protected]>
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this issue Jun 19, 2019
…enhab#4755)

* Fix inadvertently change automatic/manual mode

Addresses issue Setting temperature can inadvertently change
automatic/manual mode

Fixes openhab#4347

Signed-off-by: Marcel Verpaalen <[email protected]>
Signed-off-by: Pshatsillo <[email protected]>
ne0h pushed a commit to ne0h/openhab-addons that referenced this issue Sep 15, 2019
…enhab#4755)

* Fix inadvertently change automatic/manual mode

Addresses issue Setting temperature can inadvertently change
automatic/manual mode

Fixes openhab#4347

Signed-off-by: Marcel Verpaalen <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
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

No branches or pull requests

4 participants