jmcollin78 / versatile_thermostat

A full featured Thermostat for Home Assistant: presets, window, motion, presence and overpowering management
MIT License
346 stars 38 forks source link

Underlying config changes #590

Closed hilburn closed 4 weeks ago

hilburn commented 1 month ago

All (modified) tests working Allows entry of an arbitrary number of entities into a climate, switch, or valve VTherm controller

Possibility of mild breaking changes for existing template/triggers as the attributes of the resulting climate entity have changed, from: underlying_X_[1-4] to underlying_entities - however I don't see that being a commonly referenced attribute in those contexts

Should supercede https://github.com/jmcollin78/versatile_thermostat/pull/551

image

jmcollin78 commented 1 month ago

Hello @hilburn ,

This is really a great change ! Thank you so much for this. I have a dev on the go but when I finish my dev (may be one or two days), I will review and integrate your changes. So be patient please. I hope this will not generate conflict (or small conflicts on config_flow only).

I will also close the #551 PR.

hilburn commented 1 month ago

The underlying_entities attribute handling could be moved into Base Thermostat, as it's now common to all the types, but I left it in the child classes for now as it could be changed back to climates, valves, and switches.

jmcollin78 commented 1 month ago

Hello @hilburn , You have one failed validation. data_description have been updated it seems. You have also to change the strings.json (a copy of en.json)

hilburn commented 1 month ago

Ok, I think that should be all matching now

jmcollin78 commented 1 month ago

I hv resolved the conflicts with my last release. It was minor.

jmcollin78 commented 4 weeks ago

I will merge and debug it in local. I don't understand why the test fails.

hilburn commented 4 weeks ago

Awesome, I'm glad it all looks good on your end

jmcollin78 commented 4 weeks ago

I have this error on test:

if errors:
>           raise er.MultipleInvalid(errors)
E           voluptuous.error.MultipleInvalid: extra keys not allowed @ data['climate_entity_id']

on test_user_config_flow_over_climate_auto_start_stop (my last test). I try to find out why.

EDIT: ok. Have to use CONF_UNDERLYING_LIST instead CONF_CLIMATE.

jmcollin78 commented 4 weeks ago

It is fine now. I will do a beta release. Can you please give it a try ? Ping @pounard if you want you can try also this beta release when ready (should come rapidly).

Pre-release is here: https://github.com/jmcollin78/versatile_thermostat/releases/tag/6.6.0.beta1

Many thanks @hilburn and @pounard for this great enhancement.

hilburn commented 4 weeks ago

I just installed it and reloaded on my HA - everything seemed to import correctly Except - for some reason all my VTherm entities have "auto fan mode" set to High image Despite none of them having fans or being set to that when I originally set them up. I don't know if this is a new bug or it's been around for a while and I just never noticed though

jmcollin78 commented 4 weeks ago

You never configure this fan-mode ? You don't change this and me too.

It is set to "turbo" on my side. That was my previous value.

Nothing weird for me.

hilburn commented 4 weeks ago

Ahhh never mind, it just defaults to that during setup and I never noticed or turned it off

jmcollin78 commented 4 weeks ago

Released ! https://github.com/jmcollin78/versatile_thermostat/releases/tag/6.6.0