jeatheak / Mitsubishi-WF-RAC-Integration

WF-RAC homeassistant integration
MIT License
101 stars 19 forks source link

Add horizontal swing mode service #34

Closed bjorne closed 1 year ago

bjorne commented 1 year ago

This change is a WIP towards adding a custom service to the climate entity exposing the possibility to set the horizontal swing mode. This would address #12. Opening this PR to begin some discussion and get some feedback. Let me mention the major things:

Renaming the domain (and custom component directory)

When I first added the custom service it couldn't actually be called, since the domain uses dashes rather than underscores, i.e. mitsubishi-wf-rac instead of mitsubishi_wf_rac. Looking at the components list in core I can't see any other integrations with dashes, so maybe it's just not allowed and supported. I assume that this will be a major breaking change and that all existing users will need to re-configure the integration, unless there's a hack for the entity registry or something. (I tried to hack it on my dev instance without success.) But these are my first steps to HA development so I might be missing something.

Relation to vertical swing mode and 3D Auto

When setting the 3D Auto mode, it seems like both the horizontal and vertical directions are disabled in the M-Air app. Since the 3D Auto mode is available under the existing (vertical) swing mode service, I decided to not expose it also in the horizontal swing mode service added her. Does that make sense?

No attribute for horizontal swing mode

I played around quickly with adding attributes similar to _attr_swing_mode and _attr_swing_modes as defined in the climate platform, but didn't get it to work. I've left these additions as comments. Is there a way to define custom attributes for an entity? Alternatively, the current horizontal swing mode could be exposed with an additional sensor entity, perhaps?

Naming the horizontal modes

The values 1-7 for the horizontal mode seems to set the left and right fins to left-center-right in various combinations. I tried some of them and compared to the app, and then established some kind of pattern describing the position of the two fins with the values "Left-Left", "Left-Center", "Right-Right", "Right-Left", etc. Does this make sense?

Also, I'm not sure if the "Left/Right Auto" should be called that, just went with something similar to the vertical modes.

Finally, a screenshot to show how it works

2023-01-10-22:47:30-screenshot

SanderKo85 commented 1 year ago

Nice work! As i understand its difficult (one of things) to understand the entities that would explain the position of the blades/blowing direction of the two(!) different blowing sections; left section with its own blowing path, and right section with its own blowing path to control.

As for the 3D option: my unit directionmotors will blow up/down/left/right and so on to on steady basis to reach most basis (almost circular).

As for naming: take for example [Center-Right], I assume that when I am in the front of the unit looking at it, the left section will only blow straight forward/middle and the right section will blow to the right (of me).

Sander.

SanderKo85 commented 1 year ago

Ah, apart from 3D-Auto (overrules all current blowing direction settings), I see for up/down a sort of "auto"; going up and down, "swinging" all the time. And the same for left/right.

I think that would be exactly the same as 3D if both are in Auto. 20230111_005040

bjorne commented 1 year ago

Thanks for chipping in @SanderKo85

As for naming: take for example [Center-Right], I assume that when I am in the front of the unit looking at it, the left section will only blow straight forward/middle and the right section will blow to the right (of me).

That is exactly how I also think about it and the pattern used currently matches this. :+1:

Ah, apart from 3D-Auto (overrules all current blowing direction settings), I see for up/down a sort of "auto"; going up and down, "swinging" all the time. And the same for left/right.

Yes, this is what I call "Left/Right Auto". For the existing swing mode there is the corresponding "Up/Down Auto".

I think that would be exactly the same as 3D if both are in Auto.

Right, I agree the result is likely the same. But it seems that when the 3D Auto mode is set, the horizontal swing mode doesn't matter. I tried setting the horizontal mode when 3D Auto was enabled, and it still kept alternating horizontally. So maybe this should also be set to "3D Auto" when it's enabled, just like the existing swing mode? And when calling this service with any other value then 3D Auto should be disabled before setting the horizontal swing mode?

Edit: the existing swing mode service already enabled/disables 3D Auto like this:

https://github.com/jeatheak/Mitsubishi-WF-RAC-Integration/blob/38e6dc4af8859e9aa3e440f8eae9a9861be4cff0/custom_components/mitsubishi-wf-rac/climate.py#L98-L102

bjorne commented 1 year ago

Following up on the domain change I found this in the dev docs:

The domain is a short name consisting of characters and underscores

So most likely it needs to be changed in order to conform. I imagine it's best to just do that change as early as possible and bump the major version?

jeatheak commented 1 year ago

Hey,

regarding the domain name. that is indeed wrong (I think) how I did this. I'm also new in developing for Hassio. Till now it did not give any problems. but it needs to be changed. The problem is indeed that users are already using it and need to check the impact.

I think it is indeed good to not also add the 3d auto to the horizontal swing because it is already a long dropdown list.

The attributes part: I don't know much about that one.

I think the naming is ok and will be good to understand if the users have used the original app.

Nice work! I will check the code in detail soon. (my dev environment has some issues atm)

SanderKo85 commented 1 year ago
  • I think it is indeed good to not also add the 3d auto to the horizontal swing because it is already a long dropdown list.

Indeed, the [Left-Right Auto] is already the "swing all the time horizontaly"

EDIT: haha, was looking for somebody else his Panasonic, came across this: afbeelding

SanderKo85 commented 1 year ago

Hey guys, can I test the update? Is the dev environment already straighten up? :)

bjorne commented 1 year ago

@SanderKo85 you should be able to test it!

If I'm not mistaken you should be able to run this in parallel to the existing custom component, since the domain and directory was renamed mitsubishi-wf-rac -> mitsubishi_wf_rac. Something like copying the contents of this branch into custom_components.

There might be something preventing this, though. If running both, maybe change the name in manifest.json to something else to be able to distinguish them in the Add Integration flow.

jeatheak commented 1 year ago

It looks good and I added some extra stuff. I will merge this into develop. If the users will delete their devices first and after the update add the devices with the same name the history is kept.