Optional persistence in generic_thermostat #10636

ziotibia81 · 2017-11-17T13:22:39Z

Description:

Generic thermostat can, optionally, hold target_temp and state if changed in UI

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4003

Example entry for `configuration.yaml` (if applicable):

- platform: generic_thermostat
  name: Test
  heater: switch.Test
  target_sensor: sensor.test
  min_temp: 4
  max_temp: 60
  target_temp: 20
  min_cycle_duration:
    seconds: 1
  keep_alive:
    minutes: 5
  cold_tolerance: 0.5
  hot_tolerance: 0.5
  persistence: both

Checklist:

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass

houndci-bot · 2017-11-17T13:22:48Z

homeassistant/components/climate/generic_thermostat.py

+        if self._persistence in [ATTR_BOTH, CONF_TARGET_TEMP]:
+            self._target_temp=state.attributes[ATTR_TEMPERATURE]
+        if (self._persistence in [ATTR_BOTH, ATTR_OPERATION_MODE] and
+            state.attributes[ATTR_OPERATION_MODE] == STATE_OFF):


visually indented line with same indent as next logical line

houndci-bot · 2017-11-17T13:22:48Z

homeassistant/components/climate/generic_thermostat.py

+        if not state:
+            return
+        if self._persistence in [ATTR_BOTH, CONF_TARGET_TEMP]:
+            self._target_temp=state.attributes[ATTR_TEMPERATURE]


missing whitespace around operator

houndci-bot · 2017-11-17T13:22:48Z

homeassistant/components/climate/generic_thermostat.py

@@ -213,6 +222,19 @@ def _async_sensor_changed(self, entity_id, old_state, new_state):
        self._async_update_temp(new_state)
        self._async_control_heating()
        yield from self.async_update_ha_state()
+


blank line contains whitespace

happyleavesaoc · 2017-11-17T16:19:55Z

homeassistant/components/climate/generic_thermostat.py


+ATTR_NONE = 'none'
+ATTR_BOTH = 'both'
+ATTR_OPERATION_MODE = 'operation_mode'


This is available in homeassistant.components.climate

Fix ATTR_OPERATION_MODE from homeassistant.components.climate

houndci-bot · 2017-11-17T17:59:21Z

tests/components/climate/test_generic_thermostat.py

+        }})
+
+    state = hass.states.get('climate.test_thermostat')
+    assert(state.attributes[ATTR_TEMPERATURE] == 20)


undefined name 'ATTR_TEMPERATURE'

houndci-bot · 2017-11-17T17:59:21Z

tests/components/climate/test_generic_thermostat.py

+        State('climate.test_thermostat', '0', {'operation_mode': STATE_OFF}),
+    ))
+
+    hass.state = CoreState.starting


undefined name 'CoreState'

houndci-bot · 2017-11-17T17:59:21Z

tests/components/climate/test_generic_thermostat.py

+    """Ensure states are restored on startup."""
+    mock_restore_cache(hass, (
+        State('climate.test_thermostat', '0', {ATTR_TEMPERATURE: "20"}),
+        State('climate.test_thermostat', '0', {'operation_mode': STATE_OFF}),


undefined name 'State'

houndci-bot · 2017-11-17T17:59:21Z

tests/components/climate/test_generic_thermostat.py

+def test_restore_state(hass):
+    """Ensure states are restored on startup."""
+    mock_restore_cache(hass, (
+        State('climate.test_thermostat', '0', {ATTR_TEMPERATURE: "20"}),


undefined name 'State'
undefined name 'ATTR_TEMPERATURE'

houndci-bot · 2017-11-17T17:59:21Z

tests/components/climate/test_generic_thermostat.py

+@asyncio.coroutine
+def test_restore_state(hass):
+    """Ensure states are restored on startup."""
+    mock_restore_cache(hass, (


undefined name 'mock_restore_cache'

houndci-bot · 2017-11-17T18:12:17Z

tests/components/climate/test_generic_thermostat.py

        }})

    state = hass.states.get('climate.test_thermostat')
    assert(state.attributes[ATTR_TEMPERATURE] == 20)
+    assert(state.attributes['operation_mode'] == STATE_OFF)


no newline at end of file

houndci-bot · 2017-11-17T18:12:17Z

homeassistant/components/climate/generic_thermostat.py

@@ -226,6 +234,19 @@ def _async_sensor_changed(self, entity_id, old_state, new_state):
        self._async_control_heating()
        yield from self.async_update_ha_state()

+    @asyncio.coroutine


redefinition of unused 'async_added_to_hass' from line 129

tinloaf · 2017-11-17T18:16:14Z

Sorry, I think I pushed my PR literally a minute before you pushed yours. If you want, you can certainly rebase on top of the current dev branch - since my PR doesn't restore the operation mode, it would actually add functionality. However, I don't think that this should be configurable. If a user doesn't want the target temperature to be restored on startup, he / she can set the target_temperature option.

ziotibia81 · 2017-11-17T18:21:28Z

My patch work even if target_temperature is set: if there are no prevous states temperature is set from configuration.

houndci-bot · 2017-11-17T18:24:12Z

tests/components/climate/test_generic_thermostat.py

        }})

    state = hass.states.get('climate.test_thermostat')
    assert(state.attributes[ATTR_TEMPERATURE] == 20)
+    assert(state.attributes['operation_mode'] == STATE_OFF)
+


blank line at end of file
blank line contains whitespace

tinloaf · 2017-11-17T18:43:03Z

@ziotibia81 Then you should change that. 😉 The usual semantics with initial values being set in the configuration is: Restore the state only if no such value is set.

Danielhiversen · 2017-11-17T18:50:13Z

I agree with @tinloaf, I don't think it should be configurable

ziotibia81 · 2017-11-17T19:17:19Z

Ok, but we have to restore also 'operation mode' (configurabile if restore or not, or with initial value). Before my patch at restat the state is always auto. So if user want turn off thermostat and be sure that sta off at restart he have to cange setpoint. Is more confortable to change state in off.

tinloaf · 2017-11-18T11:42:44Z

Yes, I agree, we should also restore the operation mode. And if you want to do it 100 percent, you could add an optional initial_operation_mode config, which overrides the restore value if set.

ziotibia81 · 2017-11-18T16:50:06Z

Ok! I will work on it tomorrow or Monday.

ziotibia81 · 2017-11-20T09:02:05Z

Outdated by #10690

Update generic_thermostat.py

43ae96b

homeassistant added platform: climate.generic_thermostat cla-signed labels Nov 17, 2017

houndci-bot reviewed Nov 17, 2017

View reviewed changes

ziotibia81 added 2 commits November 17, 2017 14:28

Fix code syle

79b90a2

Fix spaces

9ff4021

ziotibia81 mentioned this pull request Nov 17, 2017

Generic thermostat persistence home-assistant/home-assistant.io#4003

Closed

2 tasks

ziotibia81 closed this Nov 17, 2017

ziotibia81 reopened this Nov 17, 2017

happyleavesaoc reviewed Nov 17, 2017

View reviewed changes

ziotibia81 changed the title ~~Optional persistence in generi_thermostat~~ Optional persistence in generic_thermostat Nov 17, 2017

Fix ATTR_OPERATION_MODE

ad51e4b

Fix ATTR_OPERATION_MODE from homeassistant.components.climate

homeassistant added the cla-signed label Nov 17, 2017

balloobbot added the platform label Nov 17, 2017

Fix Tests

106c50b

homeassistant added the cla-signed label Nov 17, 2017

houndci-bot reviewed Nov 17, 2017

View reviewed changes

ziotibia81 added 3 commits November 17, 2017 19:01

More fix to tests

b9cb475

Fix conflicts

30a11ec

Merge branch 'dev' into generic_thermostat_persistence

81f9121

houndci-bot reviewed Nov 17, 2017

View reviewed changes

Merge branch 'dev' into generic_thermostat_persistence

0d9c404

Fix new line at the end of file

ef8db94

houndci-bot reviewed Nov 17, 2017

View reviewed changes

ziotibia81 added 2 commits November 17, 2017 19:25

Fix blank space

56471bd

FIX redefinition of unused 'async_added_to_has

2b9625c

ziotibia81 closed this Nov 20, 2017

ziotibia81 mentioned this pull request Nov 20, 2017

Generic thermostat initial_operation_mode #10690

Merged

2 tasks

ziotibia81 deleted the generic_thermostat_persistence branch December 5, 2017 15:01

home-assistant locked and limited conversation to collaborators Mar 30, 2018

ghost added integration: generic_thermostat and removed platform: climate.generic_thermostat labels Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optional persistence in generic_thermostat #10636

Optional persistence in generic_thermostat #10636

ziotibia81 commented Nov 17, 2017 •

edited

Loading

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

happyleavesaoc Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

houndci-bot Nov 17, 2017

tinloaf commented Nov 17, 2017

ziotibia81 commented Nov 17, 2017

houndci-bot Nov 17, 2017

tinloaf commented Nov 17, 2017

Danielhiversen commented Nov 17, 2017

ziotibia81 commented Nov 17, 2017

tinloaf commented Nov 18, 2017

ziotibia81 commented Nov 18, 2017

ziotibia81 commented Nov 20, 2017

Optional persistence in generic_thermostat #10636

Optional persistence in generic_thermostat #10636

Conversation

ziotibia81 commented Nov 17, 2017 • edited Loading

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tinloaf commented Nov 17, 2017

ziotibia81 commented Nov 17, 2017

Choose a reason for hiding this comment

tinloaf commented Nov 17, 2017

Danielhiversen commented Nov 17, 2017

ziotibia81 commented Nov 17, 2017

tinloaf commented Nov 18, 2017

ziotibia81 commented Nov 18, 2017

ziotibia81 commented Nov 20, 2017

ziotibia81 commented Nov 17, 2017 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):