Skip to content

Expose comfort presets as HA presets#25491

Merged
balloob merged 5 commits into
devfrom
ecobee-presets
Aug 1, 2019
Merged

Expose comfort presets as HA presets#25491
balloob merged 5 commits into
devfrom
ecobee-presets

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Jul 25, 2019

Breaking Change:

  • Remove attributes climate_list and climate_mode. That's now part of preset.
  • Expose all Ecobee programs as presets

image

Description:

Expose the Ecobee comfort settings as presets in Home Assistant.

CC @geekofweek

Related issue (if applicable): #25488

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

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

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

@probot-home-assistant probot-home-assistant Bot added integration: ecobee small-pr PRs with less than 30 lines. labels Jul 25, 2019
@balloob balloob changed the title Expose comfort presets as HA presets WIP: Expose comfort presets as HA presets Jul 25, 2019
@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jul 30, 2019

@geekofweek will you have time soon to help me address this PR ?

@geekofweek
Copy link
Copy Markdown
Contributor

I was attempting to test it earlier today but got side tracked. Going to try in a bit and see what I find.

@geekofweek
Copy link
Copy Markdown
Contributor

Initial test, and it's what I thought might happen since I ran into this each time I tried to go down this path before. I think it's solvable with some effort.

The way it currently polls the API for the Climate / Preset it only pulls back the name value. However when trying to set holdClimateRef it is actually looking for the climateRef value. This has worked out for the defaults thus far because the name and climateRef value are the same, although lowercase which is the current problem with the default presets not working on this setup. It's passing back Home instead of home.

As an example:

Polling against name returns (which is what this integration does now for presets):

Upstairs,Fireplace,Home,Sleep,Guest,Away

Home, Sleep, Away are defaults and translate to climateRef vales home, sleep, away

However, the custom profiles translate to the autogenerated values, so if I polled climateRef the values would be:

Upstairs:  smart2
Fireplace:  smart4
Guest:  smart3

I solved this up until now by documenting the climateRef values and calling them directly

  action:
    - service: climate.set_preset_mode
      data:
        entity_id: climate.main_floor
        #Upstairs = smart2
        preset_mode: 'smart2'

Then I did a template sensor that translated the values to show in the UI. Scaleable, nope, but worked for my instance. Not super helpful for this problem, but how I knew about it from digging into this previously and just using a direct approach.

All that said, I think we have two possible courses of action.

Option 1:

  • Change the Preset value from name to climateRef
  • Probably the easiest option
  • Default profiles will still show as home, sleep, away (albeit lower case values)
  • Custom profiles will have user un-friendly names such as smart2, smart3, etc.
  • This shows promise, but haven't been able to solve it 100% yet tonight.

Option 2:

  • Poll both the name and climateRef values and reference them together.
  • From a UI standpoint it shows the user friendly name
  • From an API call it passes back the value for holdClimateRef that the Ecobee API expects
  • Probably more difficult to implement

Make any sense or did I confuse the matter even worse?

Ecobee API Sources:
Climate Object
Setting a holdClimateRef

@geekofweek
Copy link
Copy Markdown
Contributor

Now that I got my head around what's happening if I get some more time I might try and do some trial and error tests and see what I get back.

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jul 31, 2019

So in my current code, I just show the names as presets, and look up the climateRef when a name comes in for the preset. So you can send in "Away" or "GeekOfWeek Party Mode" and we set the correct climate ref.

I think that we should check that if a hold mode is set, we should make sure we set the HVAC mode to be heat-cool, heat or cool (depending on the hold mode that is set). It's no longer auto.

I also think that we should return "Hold" as current preset mode when a hold mode is active.

@geekofweek
Copy link
Copy Markdown
Contributor

Did you publish that code or are your referencing the last commit? I'd like to test if it's something different.

Not sure I completely follow on setting the HVAC mode if a hold is set, presently it ignores it and just leaves it on whatever mode it is currently in, including being off. It will still set the hold, but the HVAC mode doesn't change. I guess that kind of makes sense requiring it to be set, but hold modes can be both used for both heat or cool, they include both settings.

If you return "Hold" for any preset being hold you wouldn't know which one it was currently in, not that it's a big deal. I actually built a template sensor around what hold mode is currently active, so I personally find knowing the exact hold mode useful, but it's not a huge deal.

Essentially I built what we're trying to do as part of the integration with various automations and sensors.

Screen Shot 2019-07-31 at 2 43 26 PM

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jul 31, 2019

You're commenting on a pull request that includes those changes. Did you test this PR? The screenshot in the description is from this PR.

@geekofweek
Copy link
Copy Markdown
Contributor

Yeah I was testing it last night and today, or at least this version. Which I thought was the latest change to the climate.py file.

That's what my write up was based on. I ran into an issue that when selecting a preset I would get back:

[homeassistant.components.ecobee.climate] Received unknown preset mode:

Which is what I was referring to when I said it was trying to send back the name value and not climateRef value. I could be wrong on that, but that's what looks like is happening to me.

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jul 31, 2019

It is sending back the name, but I added code to convert name -> climateRef, maybe that needs more work .

@geekofweek
Copy link
Copy Markdown
Contributor

geekofweek commented Jul 31, 2019

It is sending back the name, but I added code to convert name -> climateRef, maybe that needs more work .

I've been attempting to solve that one testing some changes, but haven't had much luck yet. It seems like it still sends the name value back as that is what shows as the unknown preset mode value being logged.

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Aug 1, 2019

Ugh, it was a silly bug, is None vs is not None. Pushed a fix that:

  • Allows setting preset modes from dropdown
  • Reports the right name for the current preset

Also had to rebase branch due to dev branch migrating to Black.

@geekofweek
Copy link
Copy Markdown
Contributor

I stared at that for hours and didn't catch it.

I think this is it though, seems to be working exactly as you would expect. I'll keep testing, but I ran it through its paces and it loads all the proper preset names, sets the preset properly, and am able to set a preset via automation climate.set_preset_mode. This is good stuff, nice work.

@geekofweek
Copy link
Copy Markdown
Contributor

Latest commits seem to test out ok.

@balloob balloob changed the title WIP: Expose comfort presets as HA presets Expose comfort presets as HA presets Aug 1, 2019
@balloob balloob added this to the 0.97.0 milestone Aug 1, 2019
@balloob
Copy link
Copy Markdown
Member Author

balloob commented Aug 1, 2019

Thanks for the testing. I will get it merged.

@balloob balloob merged commit a398b39 into dev Aug 1, 2019
@delete-merged-branch delete-merged-branch Bot deleted the ecobee-presets branch August 1, 2019 19:32
balloob added a commit that referenced this pull request Aug 1, 2019
* Expose comfort presets as HA presets

* Fix bugs

* Handle unavailable

* log level debug on update

* Lint
@lock lock Bot locked and limited conversation to collaborators Aug 2, 2019
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.

3 participants