Closed david-kalbermatten closed 3 years ago
@bbbates I would like to integrate your suggested changes so the fan mode doesn't break the reverse_cycle
compatibility. However, you either need to explain to me how you control your device with reverse_cycle
or I add you to my fork so you can add your own commits to this pull request :D
@zacs I also plan on adding the dehumidifier/dry mode
in a similar manner to the fan mode, since it will probably be requested at some point down the line... (I could imagine a use-case for drying clothes in a small apartment)
@david-kalbermatten thanks, from a reverse cycle POV this looks better. I haven’t tested it, but seems like it would work just from reading through the commit.
Thinking about this more, I really don’t think this whole change is the right approach for this component. I’ll add my thoughts to the original issue thread.
Apologies for the delay in reviewing here (busy in life). Let me take some time this afternoon to think about it. It sounds like there are two discreet things we are talking about building:
generic
entity that turns a thing on when a sensor goes into a state. climate
, which I think I want, but I also do not have the time to support/build the value_template
portions etc. Again, I appreciate the discussion and contributions. I am considering at first glance keeping dualmode
as-is and forking/creating a template climate
platform to address the improvements @david-kalbermatten has contributed.
I definitely think the ideas in this PR are valuable - nice work @david-kalbermatten ! I think they've definitely exposed a missing part of HA that I can see being very useful.
@bbbates @zacs I want to thank you guys for your encouraging words of praise :D I don't know how useful it'd be making a template version of the climate entity when this (SmartIR) already exists. I literally just found it today by accident and I think this will be the way for me going forward.
As for the PR, if you guys still want to integrate it, you wouldn't have to worry about it breaking existing installations since I added options for existing config files (reverse_cycle: True/False)
But as @bbbates already said it's up to you @zacs ;D
Hi guys @bbbates @zacs , it's been a hot minute. I finally received my Broadlink RM4C mini (absolutely love it :D). And after setting everything up, I had to realize that I wasn't able to use SmartIR
how I intended to...
This being said however, I made some more changes to the code. I basically made all the switches
/input_booleans
optional, so the user can decide which modes he wants to use (my HVAC only supports Cool
, Dry
, Fan_only
). This together with template_switches
makes for a great way to make my HVAC controllable via IR.
# climate.yaml
- platform: dualmode_generic
name: Bedroom Thermostat
heater: input_boolean.heater
cooler: input_boolean.air_conditioner
fan: input_boolean.fan
fan_behavior: cooler
dryer: input_boolean.dryer
dryer_behavior: cooler
target_sensor: sensor.shelly_shht_1_f03be6_temperature
min_temp: 16
max_temp: 30
cold_tolerance: 0.8
hot_tolerance: 0.4
min_cycle_duration:
minutes: 20
fan_behavior: [cooler, neutral, heater] # <-- only one
dryer_behavior: [cooler, neutral, heater] # <-- only one
reverse_cycle: cooler, heater, dryer, fan # <-- multiple are possible, (True/False) are still valid for backward compatibility
I updated the ReadMe to reflect the changes, should you be interested in merging. (also merged version change from upstream be9c179)
Wow, this is a really nice addition of features. I will go through and leave comments, but give me a minute. I also wonder if we shouldn't rebase the component with the core generic_thermostat
at some point in order to take advantage of the many changed to HA core in the past year. Out of scope for this PR though :).
I merged the latest changes for the addition of heat_cool
. However, I think that the current implementation of heat_cool
needs more work...
The temperature selectors for "target_temp_high" and "target_temp_low" should only be visible when the device is on heat/cool mode unless of course, this is how it is meant to be used...
I initially made my changes so the heater and cooler are optional as well. However, after merging the heat_cool
changes I reverted back to requiring them since heat_cool
needs both to work. I'll revisit this in the future to make the heater and cooler entity optional again by disabling the heat_cool
mode if only one of the two is present.
also fixes #23
I also added an explicit option to enable the heat_cool
mode with:
enable_heat_cool: true
This config entry is optional and defaults to false
.
@milandzuris You need to add my fork to your HACS or ask @zacs to merge this PR ;D
Open Context Menu of HACS in Integrations
Remove Dual-Mode Generic Thermostat if you already see an entry there
Add my GitHub URL and select Category Integration
https://github.com/david-kalbermatten/ha-dualmodegeneric
But it's much easier if my changes get accepted into the master branch of the original repository...
@zacs I contacted you via email if you want to talk about the changes of this PR ;D (Easier than trying to convey complex thoughts through PRs and Issues)
@zacs It now uses implicit checks for target_temp_high
and target_temp_low
Commit on my Fork
The line where enable_heat_cool
is set, ensures that all requirements are met:
# This part of the code checks whether both cooler and heater are defined and deactivates heat_cool
# mode if necessary. Additionally, it checks whether target_temp_high/low are set and deactivates heat_cool
# mode should this not be the case.
enable_heat_cool = self.cooler_entity_id and self.heater_entity_id and target_temp_high and target_temp_low
if enable_heat_cool:
self._support_flags = SUPPORT_FLAGS | SUPPORT_TARGET_TEMPERATURE_RANGE
else:
self._support_flags = SUPPORT_FLAGS
self._hvac_list = [HVAC_MODE_COOL, HVAC_MODE_HEAT,
HVAC_MODE_DRY, HVAC_MODE_FAN_ONLY,
HVAC_MODE_OFF, HVAC_MODE_HEAT_COOL]
# Removes unsupported modes based on whats available from the config
if self.cooler_entity_id is None:
self._hvac_list.remove(HVAC_MODE_COOL)
if self.heater_entity_id is None:
self._hvac_list.remove(HVAC_MODE_HEAT)
if self.fan_entity_id is None:
self._hvac_list.remove(HVAC_MODE_FAN_ONLY)
if self.dryer_entity_id is None:
self._hvac_list.remove(HVAC_MODE_DRY)
if not enable_heat_cool:
self._hvac_list.remove(HVAC_MODE_HEAT_COOL)
Should also fix #25 since there would always be a high/low target temperature coming from config...
closes #23
Took a look and the last commit looks great.
I am a bit blind since I don't have the component running myself, but want to confirm that you are using this branch and not seeing problems so far? Just want to triple check before merging, thanks!
Works as far as I can tell
enable_heat_cool: True
enable_heat_cool: False
Merged, thanks for your patience and for the contribution!
I added the fan/ventilator-entity as a cooling device as @alexvenom asked in his issue #3. Tested with input booleans. Seems to work as expected
Edit: What it looks like at the moment