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

Stop tracking unwanted trips in fleet mode on iOS #234

Merged
merged 5 commits into from
Sep 6, 2024

Conversation

catarial
Copy link
Contributor

@catarial catarial commented Sep 6, 2024

Unwanted trips where there is no BLE beacon are happening on iPhone when using a fleet server. This is caused by not checking for fleet mode when receiving a VISIT_ENDED transition.

This can be tested by using an opcode for a fleet server then forcing a transition into VISIT_ENDED. In the unpatched code this triggers a trip start.

Log showing trip start:

In TripDiaryStateMachine, received transition T_VISIT_ENDED in state STATE_WAITING_FOR_TRIP_START
DEBUG: In TripDiaryStateMachine, received transition T_VISIT_ENDED in state STATE_WAITING_FOR_TRIP_START
data has 96 bytes, str has size 96
data has 69 bytes, str has size 69
started fine location tracking with accuracy = -1 and distanceFilter = 1
DEBUG: started fine location tracking with accuracy = -1 and distanceFilter = 1
In TripDiaryStateMachine, received transition T_TRIP_STARTED in state STATE_WAITING_FOR_TRIP_START
DEBUG: In TripDiaryStateMachine, received transition T_TRIP_STARTED in state STATE_WAITING_FOR_TRIP_START
data has 98 bytes, str has size 98
data has 69 bytes, str has size 69
Moved from STATE_WAITING_FOR_TRIP_START to STATE_ONGOING_TRIP

This was fixed by simply ignoring a VISIT_ENDED transition for fleet mode.

Log after patch, trip doesn't start:

DEBUG: In TripDiaryStateMachine, received transition T_VISIT_ENDED in state STATE_WAITING_FOR_TRIP_START
data has 97 bytes, str has size 97
data has 68 bytes, str has size 68
Got transition T_VISIT_ENDED in state STATE_WAITING_FOR_TRIP_START with fleet mode, ignoring, no beacon found
Ignoring silent push notification
THREAD WARNING: ['DataCollection'] took '10,231.5830' ms. Plugin should use a background thread.
DEBUG:parseState: state = STATE_WAITING_FOR_TRIP_START; 
      platformId = ios
DEBUG: parseState: state = STATE_WAITING_FOR_TRIP_START; 
      platformId = iOS

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

LGTM! Can you also change the NSLog messages related to fleet transitions to the LocalNotificationHandler logs so that we can see them on real phones and real trips?

plugin.xml Outdated Show resolved Hide resolved
@catarial
Copy link
Contributor Author

catarial commented Sep 6, 2024

Changes to fleet logs compiled and ran. I couldn't verify if it shows in the actual phone log but it shows in the Xcode log.

Got transition T_VISIT_ENDED in state STATE_WAITING_FOR_TRIP_START with fleet mode, ignoring, no beacon found
DEBUG: Got transition T_VISIT_ENDED in state STATE_WAITING_FOR_TRIP_START with fleet mode, ignoring, no beacon found

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

LGTM!

@shankari shankari merged commit d0005b6 into e-mission:master Sep 6, 2024
@shankari
Copy link
Contributor

shankari commented Sep 6, 2024

🥳 Congratulations to @catarial for the first merge 🎉

shankari added a commit to shankari/e-mission-phone that referenced this pull request Sep 7, 2024
Before this fix, we checked for fleet status only in the EXITED_GEOFENCE case.
This meant that the VISIT transitions started trip tracking, which led to
spurious trips. Fixed by adding a fleet check to this case as well.

Related PR: e-mission/e-mission-data-collection#234
Related release: https://github.com/e-mission/e-mission-data-collection/releases/tag/v1.9.0
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