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

Four named fan speeds #41

Closed
tathamoddie opened this issue Apr 28, 2023 · 3 comments
Closed

Four named fan speeds #41

tathamoddie opened this issue Apr 28, 2023 · 3 comments
Labels

Comments

@tathamoddie
Copy link
Contributor

Issue #27 and then PR #31 introduced support for HomeKit, by translating numbered fan speeds (1, 2, 3) to names (Low, Medium, High).

My AC supports four fan speeds though, and Intesis exposes all of them:

telnet intesis 3310
LIMITS:FANSP
LIMITS:FANSP,[AUTO,1,2,3,4]

So now I see the list in HA as:

  • Auto
  • Low
  • Medium
  • High
  • 4

Which looks a bit funny ...

It looks like the HomeKit integration actually also supports four modes:

https://github.com/home-assistant/core/blob/dev/homeassistant/components/homekit/type_thermostats.py#L121

ORDERED_FAN_SPEEDS = [FAN_LOW, FAN_MIDDLE, FAN_MEDIUM, FAN_HIGH]

I'm not sure of the best way to fix this.

Option 1 would be just to extend the mapping at https://github.com/jbergler/hass-intesisbox/blob/ed6c592f9c91405442c415149e405c237b271cac/custom_components/intesisbox/climate.py#L73-L78:

FAN_MODE_I_TO_E = {
    'AUTO': 'auto',
    '1': 'low',
    '2': 'middle',
    '3': 'medium',
    '4': 'high',
}

But then people with 3-speed systems would see Low/Middle/Medium, instead of Low/Medium/High, and that would be confusing.

Option 2 would be to use different mappings depending on if the limits return three or four speeds.

Option 3 would be to look at the HomeKit integration, and see if we can remove the filter there, so it just passes through all of the numbers. (But the filter is probably there for a reason.)

Any other ideas?

(And while here, I'm guessing we should swap the mapping from hardcoded names, to using the climate constants defined in https://github.com/home-assistant/core/blob/dev/homeassistant/components/climate/const.py#L69-L79)

@github-actions
Copy link

This issue is being marked as stale because it has been 30 days with no activity.
Remove the stale label or leave a comment to prevent this issue from being closed in 7 days

@github-actions github-actions bot added the Stale label May 28, 2023
@jnimmo
Copy link
Owner

jnimmo commented May 28, 2023

Hey @tathamoddie - sounds like Option 2 would be a good way to proceed, migrating to the climate constants. The dynamic fan maps for IntesisBox are actually implemented already in the IntesisHome integration (the logic likely implemented in pyIntesisHome )

@github-actions github-actions bot removed the Stale label May 29, 2023
@github-actions
Copy link

This issue is being marked as stale because it has been 30 days with no activity.
Remove the stale label or leave a comment to prevent this issue from being closed in 7 days

@github-actions github-actions bot added the Stale label Jun 28, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants