Skip to content
New issue

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

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

Already on GitHub? # to your account

Add new keys for wifi, lfr, and battery #835

Merged
merged 10 commits into from
Jan 2, 2024
Merged

Conversation

mkmer
Copy link
Contributor

@mkmer mkmer commented Dec 30, 2023

Description:

The API has changed - appears the wifi_strength and battery_voltage keys have been removed and a new key:
signals:{'lfr': 5, 'wifi': 4, 'battery': 3} has been added.
If the new key (signals) exists, we populate the values using the sub keys, if not, fall back to previous key names.
Add "sync_signal_strength" to attributes.

I'm not sure if the original wifi_strength key was 0-5 or actual signal strength (I think was was signal strength).
I suppose we need to change the HA side to not show dBm? Should we do a percentage (5/5 = 100%, 1/5 - 20%) or maybe do this on the HA side?

Related issue (if applicable): fixes #
home-assistant/core#106509

Checklist:

  • Local tests with tox run successfully PR cannot be meged unless tests pass
  • Changes tested locally to ensure platform still works as intended
  • Tests added to verify new code works

Copy link

codecov bot commented Dec 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5e58c1a) 99.86% compared to head (6703cb8) 99.86%.
Report is 6 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #835   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files           8        8           
  Lines        1453     1458    +5     
=======================================
+ Hits         1451     1456    +5     
  Misses          2        2           
Flag Coverage Δ
unittests 99.86% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mkmer
Copy link
Contributor Author

mkmer commented Dec 30, 2023

I think we need to go with a percentage and update HA to native unit of as a percentage.

Copy link
Owner

@fronzbot fronzbot left a comment

Choose a reason for hiding this comment

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

I think if you just rip out the conversion to percentage (or make is a separate key if you really want it- either approach is fine with me) then we are good to merge.

remove extra battery_voltage assignment
@mkmer
Copy link
Contributor Author

mkmer commented Jan 1, 2024

If we want to change the battery_voltage to battery_strength or battery_level, I would like to wrap that change into the HA update as well. Aka get the change in the next API release if we are going to do it.

  • Looking at HA, we don't have a battery_voltage sensor, only the battery state - so timing doesn't really matter :)

@mkmer mkmer mentioned this pull request Jan 2, 2024
3 tasks
@fronzbot fronzbot merged commit 738a93a into fronzbot:dev Jan 2, 2024
@mkmer mkmer deleted the new-keys branch January 4, 2024 20:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants