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

Separate fan speeds into percentages and presets modes #8216

Merged
merged 3 commits into from
Jan 28, 2021

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Jan 24, 2021

Breaking change

Fans now use a percentage slider for speeds and a dropdown for preset modes.

Screen Shot 2021-01-24 at 7 33 40 PM

Screen Shot 2021-01-23 at 6 00 44 PM

Screen Shot 2021-01-23 at 6 00 38 PM

Proposed change

Core: home-assistant/core#45407

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

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

@Jc2k
Copy link
Member

Jc2k commented Jan 24, 2021

Having seen the UI - wondering if we need to support some kind of "step size"? Otherwise this could be an annoying UI for fans that genuinely only do have 3 speeds. For dyson we could set this to 10, and for homekit_controller we could read it off the characteristic metadata too. Is that something thats possible?

@bdraco
Copy link
Member Author

bdraco commented Jan 24, 2021

Having seen the UI - wondering if we need to support some kind of "step size"? Otherwise this could be an annoying UI for fans that genuinely only do have 3 speeds. For dyson we could set this to 10, and for homekit_controller we could read it off the characteristic metadata too. Is that something thats possible?

I think it's doable but I'm not sure what to do with the ones with 7 speeds and a step size of 14.28. Also for zwave and isy we wouldn't know the step size in advance. Maybe something we can add later if we find we really need it since it won't require any refactoring in the entity model, just a new property? (I'm starting to get a bit nervous about scope and the size of the core PR)

Edit: I did some testing and step size should be relatively clear to do this later, and it should make the homekit experience a bit cleaner. For backwards compat we would default to a step size of 1

@bdraco bdraco marked this pull request as draft January 24, 2021 23:10
@bdraco
Copy link
Member Author

bdraco commented Jan 24, 2021

Marking as draft until the preset_mode concern is addressed

@bdraco bdraco marked this pull request as ready for review January 25, 2021 01:34
@bdraco bdraco changed the title Convert fans to use percentage for speed Separate fan speeds into percentages and presets modes Jan 25, 2021
@bdraco
Copy link
Member Author

bdraco commented Jan 28, 2021

Retested. All seems ok 👍

@bdraco bdraco merged commit afdb369 into home-assistant:dev Jan 28, 2021
@bdraco bdraco deleted the fan_percentage branch January 28, 2021 15:24
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2021
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.

4 participants