Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #42 #46

Merged
merged 1 commit into from
Feb 24, 2023
Merged

Fix for issue #42 #46

merged 1 commit into from
Feb 24, 2023

Conversation

kkoenen
Copy link

@kkoenen kkoenen commented Feb 20, 2023

Removed dependency on deprecated method (temperature utility), as it will break in april 2023. For now, reverted to not converting any temperatures, as the rest of the code only seems to work with Celsius.

Future development; reintroduce temperature conversion, based on TemperatureConverter (home assistant unit_conversion.py in util folder).

Removed dependency on deprecated method (temperature utility), as it will break in april 2023. For now, reverted to not converting any temperatures, as the rest of the code only seems to work with Celsius.

Future development; reintroduce temperature conversion, based on TemperatureConverter (home assistant unit_conversion.py in util folder).
@kkoenen kkoenen changed the title Fix for #42 Fix for issue #42 Feb 20, 2023
@artmg artmg merged commit 4c10cc6 into ha-warmup:master Feb 24, 2023
@artmg
Copy link
Collaborator

artmg commented Feb 24, 2023

Thanks for this code change @kkoenen I have also:

  • created an enhancement request issue Rationalise temperature localisation #48 for the new conversion method
  • brought the dev branch back up to par with master, so that new code can safely be tested before release

Your contributions are very much appreciated (especially in the chill of winter 🥶 😄 )

@rct
Copy link
Collaborator

rct commented Mar 14, 2023

I posted an update in #38 -see #38 (comment)

It appears that the code at HEAD in the branch hacs_restructuring fixed #38.

I think the current code removes the conversion (via the deprecated temperature utility), and doesn't reintroduce a new conversion yet?

So I think what this is saying is the integration doesn't need to convert the temperatures, because Home Assistant is doing that.

So currently, these temperature (attributes) of the climate device seem to be localized correctly to F for me in the US:

min_temp: 41
max_temp: 86
current_temperature: 64
temperature: 59

However, these temperatures are NOT getting localized correctly and still appear to be in 'C'. So perhaps these need to have their class/unit set correctly like the others that are being localized correctly?

floor_temperature: 18
air_temperature: 21.5
away_temperature: 16
comfort_temperature: 20
fixed_temperature: 15
override_temperature: 20
sleep_temperature: 18

Thanks for the work on this!

@kkoenen kkoenen deleted the patch-1 branch March 15, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants