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 sensor unit handling #81

Merged
merged 7 commits into from
Apr 20, 2024
Merged

Conversation

cryptk
Copy link
Contributor

@cryptk cryptk commented Jan 3, 2024

Did some general cleanup around the handling of constants in order to resolve #79 while removing some of the custom constants that duplicated built-in constants.

Removed all instances that manually updated the entity state and replaced them with updates to native_value instead allowing Home Assistant to actually perform unit conversions resolving #80

 - cleanup handling of constants, fixes mrk-its#79
 - set native_value instead of state, fixes mrk-its#80
@cryptk cryptk force-pushed the fix_sensor_conversions branch from d39a30e to 60b3ea1 Compare January 3, 2024 00:30
@cryptk cryptk changed the title Fix sensor unit handling, fixes #79 and fixes #80 Fix sensor unit handling Jan 3, 2024
@cryptk
Copy link
Contributor Author

cryptk commented Jan 3, 2024

I'm not sure why the commit message isn't properly linking this PR to both #79 and #80, but if/when this PR is accepted, both issue #79 and #80 need to be marked as resolved

@cryptk
Copy link
Contributor Author

cryptk commented Jan 5, 2024

@mrk-its any chance I can get a review on this? This should resolve the issue of the integration showing the distance in KM no matter if you select KM or MI as your units.

@mrk-its
Copy link
Owner

mrk-its commented Jan 6, 2024

@mrk-its any chance I can get a review on this? This should resolve the issue of the integration showing the distance in KM no matter if you select KM or MI as your units.

Sure, taking a look!

Copy link
Owner

@mrk-its mrk-its left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

custom_components/blitzortung/sensor.py Show resolved Hide resolved
@codyc1515
Copy link

Is this ready to merge?

@cryptk
Copy link
Contributor Author

cryptk commented Feb 5, 2024

@mrk-its sorry for the delay, had a few health issues. I made the change you requested, can I get a new review and possibly a merge? I'm running this version locally, and it starts up with no errors, and it should be fine, but there currently aren't any storms outside for me to validate the second commit.

@cryptk
Copy link
Contributor Author

cryptk commented Feb 26, 2024

Hey @mrk-its wanted to bring this up again. I made the change that you requested, would be great to get this merged in to resolve the warnings in the logs about the upcoming deprecations.

@jrbenito
Copy link

jrbenito commented Apr 1, 2024

This also fixes #82

@jrbenito jrbenito mentioned this pull request Apr 1, 2024
@bakerkj
Copy link

bakerkj commented Apr 12, 2024

@cryptk & @mrk-its I can confirm running with this PR eliminated all warnings for me as well.

@mrk-its mrk-its merged commit ada59e7 into mrk-its:master Apr 20, 2024
@mrk-its
Copy link
Owner

mrk-its commented Apr 20, 2024

Thanks @cryptk and sorry it took so long

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.

deprecated constant in HA 2024.1
5 participants