mrk-its / homeassistant-blitzortung

Custom Component for fetching lightning data from blitzortung.org
MIT License
186 stars 38 forks source link

Fix sensor unit handling #81

Closed cryptk closed 3 months ago

cryptk commented 6 months ago

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

cryptk commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

@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!

codyc1515 commented 5 months ago

Is this ready to merge?

cryptk commented 5 months ago

@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 commented 5 months ago

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 commented 3 months ago

This also fixes #82

bakerkj commented 3 months ago

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

mrk-its commented 3 months ago

Thanks @cryptk and sorry it took so long