Filter out VehicleActivities with invalid location - #244
Closed
bnordli wants to merge 2 commits into
Closed
Conversation
isLocationValid() already detected null/zero coordinates on VehicleMonitoring updates, but its result was only used to increment a metrics counter — the activity had already been unconditionally added to the changes map beforehand. This meant vehicles reported at 0N/0W (or with a zero-only lat or lon) were logged as invalid but still stored and served to clients. Move the isLocationValid() check into the storage-gating condition so invalid-location activities are actually dropped. Also renamed the shadowing local `keep` variable (meaning "is newer than existing") to `isNewer` to avoid conflating it with location validity again. Added regression tests covering 0/0, lat-only-zero, lon-only-zero, and null VehicleLocation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Restore the pre-existing behavior where outdatedCounter and invalidLocationCounter are each evaluated independently (matching how notMeaningfulCounter already works), instead of the mutually-exclusive if/else-if chain introduced in the previous commit. A single rejected update that is both expired/not-newer and has an invalid location will again increment both counters. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Author
|
Another (live) example, from another operator: |
Contributor
|
Keeping these "invalid" updates is intentional. "Low quality data" is reported back to the data-producers, and removing them at this point will result in this "data-error" no longer being reported. I have instead added an optional filter in the user/customer-facing api to hide them from regular usage (but still available when explicitly requested): entur/vehicle-positions#40 |
Author
|
Got it, thanks! |
Author
|
Is it possible to verify that this other fix will resolve the issue for the end user? (Note that the user has explicitly chosen a bus to view the position of.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Actually remove vehicle positions with 0 lat or long from the live feed, instead of just reporting invalid positions.
dev notes:
isLocationValid() already detected null/zero coordinates on VehicleMonitoring updates, but its result was only used to increment a metrics counter — the activity had already been unconditionally added to the changes map beforehand. This meant vehicles reported at 0N/0W (or with a zero-only lat or lon) were logged as invalid but still stored and served to clients.
Move the isLocationValid() check into the storage-gating condition so invalid-location activities are actually dropped.
Added regression tests covering 0/0, lat-only-zero, lon-only-zero, and null VehicleLocation.