Skip to content

Manage Melcloud Horizontal and Vertical Swing Modes from UI#33626

Closed
ollo69 wants to merge 11 commits intohome-assistant:devfrom
ollo69:melcloud-swing-modes-2
Closed

Manage Melcloud Horizontal and Vertical Swing Modes from UI#33626
ollo69 wants to merge 11 commits intohome-assistant:devfrom
ollo69:melcloud-swing-modes-2

Conversation

@ollo69
Copy link
Copy Markdown
Contributor

@ollo69 ollo69 commented Apr 4, 2020

Breaking change

Proposed change

As anticipated on PR #33008, let me propose an alternativa solution for swing modes in Melcloud component, This will provide control of both Horizontal and Vertical swing modes from UI and more "Human Readable" list of states.
This will change the names of the available swing modes.

Type of change

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

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@probot-home-assistant
Copy link
Copy Markdown

Hey there @vilppuvuorinen, mind taking a look at this pull request as its been labeled with a integration (melcloud) you are listed as a codeowner for? Thanks!

Copy link
Copy Markdown
Contributor

@vilppuvuorinen vilppuvuorinen left a comment

Choose a reason for hiding this comment

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

This is a neat trick to encode all of the modes into a single control, but I personally don't like this approach and there are some issues with the implementation.

For example, it looks like only one of the swing modes is returned depending on the device configuration. The position maps prevent reacting to different position combinations.

I'd rather go with the vertical control only to keep things simple.

@ollo69
Copy link
Copy Markdown
Contributor Author

ollo69 commented Apr 4, 2020

it looks like only one of the swing modes is returned depending on the device configuration. The position maps prevent reacting to different position combinations.

Not true. When you set Horizontal, only Horizontal is set, same for Vertical. Changing one mode do not affect the other. and you can manage any combination.

@vilppuvuorinen
Copy link
Copy Markdown
Contributor

I added the comments to the diff.

Best fit solution would still be to just treat the swing mode as vertical vane alias instead of trying to make it do everything.

Most of the Mitsubishi air-to-air heat pumps out there do not seem to offer meaningful horizontal control anyways.

@ollo69
Copy link
Copy Markdown
Contributor Author

ollo69 commented Apr 4, 2020

Anyway, I use this solution to control my vane at all and works fine for me.
Feel free to do what you prefer, my suggestion is to take some time to view user comment with next release and take your decision.

Ciao.

@MuppetOwl
Copy link
Copy Markdown

Also vote for the solution to Ollo69. Both Vertical and Horizontal vane adjust is preferred.

I use both Vertical and horizontal adjustments all the time on Mitsubishi Kaiteki heatpumps.

@MartinHjelmare
Copy link
Copy Markdown
Member

@vilppuvuorinen do you want to accept this PR or should we close? Let me know if you want to accept it and I can make a review. Otherwise we should close it.

@vilppuvuorinen
Copy link
Copy Markdown
Contributor

I'd like to close it. I'm hoping to get something similar cooked up, but I'm not comfortable with this approach.

@MartinHjelmare
Copy link
Copy Markdown
Member

Ok, the I'll close now. Thank you all that participated.

@lock lock Bot locked and limited conversation to collaborators May 6, 2020
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.

5 participants