Skip to content

Away mode temperature fix for generic thermostat#17641

Merged
Danielhiversen merged 6 commits intohome-assistant:devfrom
estevez-dev:dev
Oct 23, 2018
Merged

Away mode temperature fix for generic thermostat#17641
Danielhiversen merged 6 commits intohome-assistant:devfrom
estevez-dev:dev

Conversation

@estevez-dev
Copy link
Copy Markdown

@estevez-dev estevez-dev commented Oct 20, 2018

Description:

Away mode temperature issue fix for generic_thermostat

Related issue (if applicable): fixes #17433

Example entry for configuration.yaml (if applicable):

climate:
  - platform: generic_thermostat
    name: Child room heater
    heater: switch.40000500dc4f22ccb55e
    target_sensor: sensor.unknown_lumisensor_ht_0222819a_1_temperature
    min_temp: 10
    max_temp: 25
    ac_mode: false
    target_temp: 22
    cold_tolerance: 0.2
    hot_tolerance: 0
    initial_operation_mode: "off"
    away_temp: 10

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

Away mode temperature issue fix for generic_thermostat
@homeassistant
Copy link
Copy Markdown
Contributor

Hi @estevez-dev,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@estevez-dev estevez-dev changed the title Resolves /home-assistant/home-assistant#17433 Away mode temperature fix for generic thermostat Oct 20, 2018
@Danielhiversen
Copy link
Copy Markdown
Member

Could you add a test for this?

@estevez-dev
Copy link
Copy Markdown
Author

Could you add a test for this?

Unfortunately I am complete zero in python. It would be very nice if someone more experienced will help me with it.

Test for fix of generic thermostat issue when away_mode was set several times in a row.
@estevez-dev
Copy link
Copy Markdown
Author

Ok, I've added a test for setting away_mode twice. @Danielhiversen please let me know is it acceptable.

Copy link
Copy Markdown
Member

@Danielhiversen Danielhiversen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
Just two small details

@estevez-dev
Copy link
Copy Markdown
Author

Done

@Danielhiversen
Copy link
Copy Markdown
Member

Looks good. Github has some issues, so the tests are not running.
Guess it soon will be fixed.

@ghost ghost removed the in progress label Oct 22, 2018
@ghost ghost assigned Danielhiversen Oct 22, 2018
@ghost ghost added the in progress label Oct 22, 2018
@Danielhiversen Danielhiversen merged commit 324587b into home-assistant:dev Oct 23, 2018
@ghost ghost removed the in progress label Oct 23, 2018
@Danielhiversen
Copy link
Copy Markdown
Member

:thanks

@balloob balloob mentioned this pull request Nov 9, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

climate/generic_thermostat: saved temperature lost on away_mode set multiple times

5 participants