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!: some sensors should be "unavailable" when the tracker is offline #372

Merged
merged 6 commits into from
Mar 10, 2024

Conversation

andreasbrett
Copy link
Contributor

Following sensors IMHO should not retain their last value but rather show up as unavailable when the tracker itself is offline (either because it is turned off or because it has no reception):

  • the device_tracker itself

binary sensors

  • valid signal

sensors

  • cell tower ID
  • gsm strength
  • GPS satellites
  • battery

One could argue to let the battery retain its last state. IMHO it is better to indicate that the state is unclear. In the end the actual value could be far off from the last retained one. I'm not married to that idea though. The other sensors though definitely make no sense to expose when there's no signal.

Copy link

codecov bot commented Mar 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.12%. Comparing base (ac1cf1a) to head (3420a20).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
- Coverage   98.20%   98.12%   -0.09%     
==========================================
  Files          20       20              
  Lines         668      799     +131     
==========================================
+ Hits          656      784     +128     
- Misses         12       15       +3     
Flag Coverage Δ
unittests 98.12% <100.00%> (-0.09%) ⬇️

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.

Copy link
Owner

@eifinger eifinger left a comment

Choose a reason for hiding this comment

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

Can you please also add a basic test? One test with a mocked return value and asserting all those changes is enough

@eifinger eifinger changed the title [fix] some sensors should be "unavailable" when the tracker is offline fix: some sensors should be "unavailable" when the tracker is offline Mar 10, 2024
@eifinger eifinger changed the title fix: some sensors should be "unavailable" when the tracker is offline fix!: some sensors should be "unavailable" when the tracker is offline Mar 10, 2024
@@ -112,7 +112,19 @@ def get_trackers_phone_call_available_subtrahend_missing_fixture():
def get_trackers_not_a_pet_tracker_missing_fixture():
"""Static result when retrieving data from weenect."""
response = copy.deepcopy(GET_RACKERS_RESPONSE)
response["items"][0]["type"] = "familykid"
response["items"][0]["position"][0]["type"] = "familykid"
Copy link
Owner

Choose a reason for hiding this comment

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

Was this change intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, I'm just ill and need sleep. deleted something unintentionally and put it back together incorrectly. thanks for the hint :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to get pytest running locally in order to test outside of the public.... haven't had time to get it running yet.

Copy link
Owner

Choose a reason for hiding this comment

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

It's a real hastle with poetry. I will change this project to rye in the coming days

@eifinger
Copy link
Owner

Nice! Thank you!!

@eifinger eifinger merged commit 067bd3b into eifinger:main Mar 10, 2024
8 checks passed
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.

2 participants