esphome / issues

Issue Tracker for ESPHome
https://esphome.io/
290 stars 34 forks source link

Please move thermostat::default_mode to climate so that it is available in all derived components #2270

Open ptr727 opened 3 years ago

ptr727 commented 3 years ago

The problem

Climate controllers start in the off state, one must use on_boot to set the desired default state, cumbersome.

default_mode was added, but it was added to thermostat, so bang_bang still needs to use on_boot to set the default state.

Please move default_mode to climate such that all derived components get the benefit.

What is version of ESPHome has the issue?

1.21-dev

What type of installation are you using?

Docker

What platform are you using?

ESP8266

Board

Sonoff TH10

Component causing the issue

thermostat

Example YAML snippet

# https://esphome.io/components/climate/index.html
climate:

  # https://esphome.io/components/climate/bang_bang.html
  - platform: bang_bang
    name: ${device_name}_bangbang
    id: bangbang
    sensor: recirc_temp
    default_mode: HEAT
    default_target_temperature_low: ${target_temperature_low}
    default_target_temperature_high: ${target_temperature_high}
    heat_action:
      - logger.log: "BangBang : Heat Action, Turning Relay On."
      - switch.turn_on: relay
    idle_action:
      - logger.log: "BangBang : Idle Action, Turning Relay Off."
      - switch.turn_off: relay
    visual:
      min_temperature: 20.0
      max_temperature: 50.0
      temperature_step: 0.5

Anything in the logs that might be useful for us?

INFO Reading configuration /config/sonoff_th10_hot_recirc_pump.yaml...
INFO Detected timezone 'PST' with UTC offset -8 and daylight saving time from 13 March 02:00:00 to 06 November 02:00:00
Failed config

climate.bang_bang: [source /config/sonoff_th10_hot_recirc_pump.yaml:146]
  platform: bang_bang
  name: sonoff_th10_hot_recirc_pump_bangbang
  id: bangbang
  sensor: recirc_temp

  [default_mode] is an invalid option for [climate.bang_bang]. Please check the indentation.
  default_mode: HEAT [source /config/sonoff_th10_hot_recirc_pump.yaml:150]
  default_target_temperature_low: 30.0
  default_target_temperature_high: 40.0
  heat_action: 
    - logger.log: BangBang : Heat Action, Turning Relay On.
    - switch.turn_on: relay
  idle_action: 
    - logger.log: BangBang : Idle Action, Turning Relay Off.
    - switch.turn_off: relay
  visual: 
    min_temperature: 20.0
    max_temperature: 50.0
    temperature_step: 0.5

Additional information

No response

probot-esphome[bot] commented 3 years ago

climate source climate issues climate recent changes (message by IssueLinks)

ptr727 commented 3 years ago

I see lots of other thermostat changes going into 21, this could be done as a non-breaking change, please do consider adding it?

kbx81 commented 3 years ago

21? Did you mean https://github.com/esphome/esphome/pull/2114 ? The focus there is/was on thermostat features/functionality -- I would do this request as a separate PR since it would mean changes to the climate core component as well as multiple other components (to integrate said change). It's on the radar and there are still some other issues/requests for bang-bang and thermostat which I may also work on in the not-too-distant future. 🙂

probot-esphome[bot] commented 3 years ago

Hey there @ottowinter, mind taking a look at this issue as it has been labeled with an integration (bang_bang) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

kbx81 commented 3 years ago

@OttoWinter as long as you're ok with it, I'll poke at the bang-bang when the time comes. 🙂 I know you are working on bigger, better things, anyway. 😄

OttoWinter commented 3 years ago

What is the use-case for this?

The default mode for bang bang already defaults to heat mode (or cool/heat_cool, whatever is supported) by default. Additionally, if the device can restore the state from a previous boot, it uses those.

It should not be added to the base climate component, for example for some IR climates where you receive the mode from the device it would make no sense.

ptr727 commented 3 years ago

The default mode for bang bang already defaults to heat mode (or cool/heat_cool, whatever is supported) by default. Additionally, if the device can restore the state from a previous boot, it uses those.

From my experience the default modes for thermostat and bang_bang are off, if I do not add a mode change in on_boot I need to power them on in HA. I think it is much more elegant to support default_mode for any component that has a mode, be that bang_bang, thermostat, or anything where the state needs to be set on power on. As of today it needs to be done using on_boot at stage 500.

E.g.

  # https://github.com/esphome/issues/issues/2270
  on_boot:
    priority: 500
    then:
      - climate.control:
          id: bangbang
          mode: HEAT
kbx81 commented 3 years ago

Yup, bang-bang chooses a default mode based on how it is configured, as seen here.

The use case is to allow the user to choose the default mode they want -- this is something that was requested in thermostat (as @ptr727 pointed out).

Could we add it to the climate base as <optional> like we have for (for example) fan_mode or preset?

kbx81 commented 3 years ago

I imagine something like optional<ClimateMode> default_mode -- I think this would make sense in a lot of places, not just in thermostat and/or bang-bang. For example, heating/cooling units controlled by climate-ir may also have a default mode; this could be set to match the unit's default mode/state after a power interruption. Some may default to off, but others may default to other modes (or even their last state). When this is the case, this default_mode could be set to be consistent with that behavior or, if the user wants something different, they could set that, too. In any event, I don't have strong feelings either way, but I definitely see use cases for this beyond just the one or two obvious ones we've discussed. 😄

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ptr727 commented 2 years ago

I still think this would be a good code inheritance consolidation and simplification.

ptr727 commented 9 months ago

I still think this would be a good code inheritance consolidation and simplification.