RobertD502 / home-assistant-flair

Custom component for Home Assistant Core for Flair pucks, vents, rooms, structures, and minisplits
MIT License
87 stars 12 forks source link

Add support for setting mode when setting temperature #61

Closed gbleaney closed 1 year ago

gbleaney commented 1 year ago

I'm back :), sorry

Summary

climate.set_temperature also exposes an hvac_mode optional parameter: https://www.home-assistant.io/integrations/climate/#service-climateset_temperature

Currently, that parameter is being silently ignored. Instead, I think it makes sense to delegate it properly to the existing hvac mode handling logic (when present)

FYI there are a few other optional parameters that are being ignored (target_temp_high and target_temp_low), and technically even temperature is optional, but I decided to keep this PR focused on just one concern.

Testing

Modified and ran the code on my local HA instance and verified that this script now sets the mode to heat, where before it did nothing:

alias: Test Generic Script
sequence:
  - service: climate.set_temperature
    data:
      hvac_mode: heat
      temperature: 70
    target:
      device_id: 041996e79b6cee8367a984314f30ce73
mode: single
RobertD502 commented 1 year ago

Regarding target_temp_high and target_temp_low:

These are not possible for Flair HVAC units. There is no such setting. Although this would relate to the Auto mode (heat_cool mode in Home Assistant), Flair doesn't handle the Auto mode with a low and high set point - only a single temperature.

I'd recommend not reading too into what Home Assistant has as options and instead cater options based on what the end device actually supports.

gbleaney commented 1 year ago

Fair point re: target_temp_high and target_temp_low. I mentioned them as an FYI in case they were also overlooked like hvac_mode, but hvac_mode is the one I care about and that I implemented in this PR. It seems logical to support it for me, since flair devices do indeed support changing modes.

I'm not sure how to read turning the PR into draft mode. Are you interested in taking this PR to support hvac_mode? If so, are there some changes you'd like before accepting?

RobertD502 commented 1 year ago

Are you interested in taking this PR to support hvac_mode? If so, are there some changes you'd like before accepting?

I am fully on board. Please see the comment left for review.

gbleaney commented 1 year ago

Sorry, I feel like I'm being a bit thick here, but I don't see the comment. I only see this one and this one, which don't seem to suggest any changes. Are you sure you clicked the "Submit Review" button after you made your review? If so, maybe just share the permalink to the comment and that'll help me figure out where I'm not looking.

RobertD502 commented 1 year ago

Sorry, I feel like I'm being a bit thick here, but I don't see the comment. I only see this one and this one, which don't seem to suggest any changes. Are you sure you clicked the "Submit Review" button after you made your review? If so, maybe just share the permalink to the comment and that'll help me figure out where I'm not looking.

Nope, not you being thick at all. Just me being an idiot. I opened a review the day you opened the PR but never submitted it, so, only I was able to see it haha. Apologies.

I went ahead and made the changes I was asking of you!

gbleaney commented 1 year ago

Thanks for making the changes and landing it!