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

Introduce PRESET_NONE for climate #25360

Merged
merged 4 commits into from Jul 21, 2019

Conversation

@balloob
Copy link
Member

commented Jul 21, 2019

Description:

The frontend used to automatically add PRESET_NONE to the preset dropdown. However, this is not something that all devices support. So it is dropped in the new frontend, and we need to be explicit about integrations that support PRESET_NONE.

I went through all integrations that allow changing preset, and looked into if they supported None as a value.

balloob added some commits Jul 21, 2019

@balloob balloob requested review from awarecan, frenck, rytilahti and home-assistant/core as code owners Jul 21, 2019

@balloob balloob added this to the 0.96.3 milestone Jul 21, 2019

@fredrike

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

What is the purpose of PRESET_NONE? Should I add support for it on Daikin too (I don't think any Daikin devices have Home but it defaults to it when I select None)?
Screenshot 2019-07-21 at 9 19 49

@cgtobi

This comment has been minimized.

Copy link
Collaborator

commented Jul 21, 2019

@fredrike if you are not sure if it supports None it likely doesn’t. What is the default mode the device is in?

@pvizeli
Copy link
Member

left a comment

Think it's fine

@fredrike

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

@fredrike if you are not sure if it supports None it likely doesn’t. What is the default mode the device is in?

Holiday is a toggle so the default should probably be None in hass.

Screenshot_20190721-103132
Screenshot_20190721-103101
Screenshot_20190721-103044
Screenshot_20190721-103015

@cgtobi

This comment has been minimized.

Copy link
Collaborator

commented Jul 21, 2019

@fredrike I would guess that if you don’t tell it otherwise the device follows its schedule, no?

@fredrike

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

@fredrike I would guess that if you don’t tell it otherwise the device follows its schedule, no?

Not sure what you mean but Holiday-mode disables built-in schedules. Neither of powerful or eco is implemented as the API for those calls have not been discovered.

My comment is that I don't think that Home mode should be visible for Daikin.

@cgtobi

This comment has been minimized.

Copy link
Collaborator

commented Jul 21, 2019

I probably misunderstood that. In that case Home (and None) should be removed.

@balloob

This comment has been minimized.

Copy link
Member Author

commented Jul 21, 2019

@fredrike can you open a new PR for that ?

@balloob

This comment has been minimized.

Copy link
Member Author

commented Jul 21, 2019

@balloob balloob merged commit 50d4921 into dev Jul 21, 2019

5 of 11 checks passed

CI (FullCheck Pylint) FullCheck Pylint failed
Details
codecov/patch 83.33% of diff hit (target 94.01%)
Details
CI Build #20190721.40 was canceled
Details
CI (Tests PyTest Python35) Tests PyTest Python35 was canceled
Details
CI (Tests PyTest Python36) Tests PyTest Python36 was canceled
Details
CI (Tests PyTest Python37) Tests PyTest Python37 was canceled
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/project 94.01% (target 90%)
Details

@delete-merged-branch delete-merged-branch bot deleted the explicit-preset-none branch Jul 21, 2019

balloob added a commit that referenced this pull request Jul 21, 2019

Introduce PRESET_NONE for climate (#25360)
* Introduce PRESET_NONE for climate

* Require preset mode to be a string

* Lint

* Fix tests
@balloob balloob referenced this pull request Jul 21, 2019
@fredrike

This comment has been minimized.

Copy link
Contributor

commented Jul 21, 2019

@fredrike can you open a new PR for that ?

Yes, in a couple of days..

Tuncay-Ayhan referenced this pull request in Tuncay-Ayhan/toon_climate Jul 22, 2019

@lock lock bot locked and limited conversation to collaborators Jul 22, 2019

@elupus

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2019

Seems something odd is going on here. #25424

A casual look tells me something is translating the "none" string into an actual None value. But the constant PRESET_NONE is the string "none".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.