home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
74.15k stars 31.12k forks source link

Race Condition Receiving Location Updates from Mobile App #126972

Open wbyoung opened 2 months ago

wbyoung commented 2 months ago

The problem

Today I noticed some location updates that weren't really possible to have happen (if the device GPS was indeed accurate):

This is a 35 mph road, and there's no way that I was traveling 120 mph part way down the road just to turn around and head back to the store a minute later at 50mph. :wink:

Probable Cause

I think the mobile app tried to send a few updates to the server, and for whatever reason the HTTP requests ended up taking some time to complete or needing to be retried. (This seems like it should be expected to happen decently often with the mobile app since mobile networks aren't incredibly reliable.) In the meantime, several other updates were processed and made their way to the server. The requests just ended up making their way to the server out of order making it appear as if I moved back and forth.

Related Code

The registered webhook simply dispatches a SIGNAL_LOCATION_UPDATE message that data was received, and the data gets written directly into the entity.

Possible Solution

The data that gets sent to the webhook for iOS and Android should include a device_timestamp which can be stored along with the data when written. Additionally, the timestamp can be compared to the last value stored. The data should only be updated when:

What version of Home Assistant Core has the issue?

core-2024.9.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

device_tracker

Link to integration documentation on our website

https://www.home-assistant.io/integrations/device_tracker/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 2 months ago

Hey there @home-assistant/core, mind taking a look at this issue as it has been labeled with an integration (device_tracker) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `device_tracker` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign device_tracker` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


device_tracker documentation device_tracker source (message by IssueLinks)

home-assistant[bot] commented 2 months ago

Hey there @home-assistant/core, mind taking a look at this issue as it has been labeled with an integration (mobile_app) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `mobile_app` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign mobile_app` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


mobile_app documentation mobile_app source (message by IssueLinks)

dshokouhi commented 2 months ago

Android and iOS handle location updates completely different. This should be split up and appropriate bugs should be filed in the appropriate repos. For android we have logs to help troubleshoot location issues but for iOS I cannot speak for that. The apps are responsible for sending the latest location updates to the server, much like they are responsible for sending the latest state update.

wbyoung commented 1 month ago

I've cross posted to:

I just experienced this issue again today. Previously it was from an iOS device. Today it was from an Android device. The behavior is the same between the two. Clustered updates are arriving and being processed in an order that doesn't make sense since the device was traveling in a straight line (and not zig-zagging back to where they came from).

dshokouhi commented 1 month ago

are you using the person entity for tracking or the actual device_tracker entity? this issue is tough to follow without core debug logs. also we shoudl not keep duplicates open we need to determine where the issue is first, if any.

wbyoung commented 1 month ago

I'm using the person entity.

Is it not even worth discussing conceptually whether the race condition is possible (on each platform) and whether or not that behavior is acceptable?

dshokouhi commented 1 month ago

For the person entity try to double check its configuration and make sure the correct devices are attached to it. We have seen users have issues with misconfigured person entities.

Before looking at the data the server gets its important to verify the data the app is sending to the server. The android app actually has certain logic in place to ensure we send the latest update to the server. The history and logs show the evaluation process already done.

wbyoung commented 1 month ago

For both me and my wife, the devices are configured properly on each person. It's only tracking one device for each of us.