Skip to content
This repository has been archived by the owner on Apr 23, 2023. It is now read-only.

Refactor ClientManager #244

Merged
merged 2 commits into from
Oct 13, 2017
Merged

Refactor ClientManager #244

merged 2 commits into from
Oct 13, 2017

Conversation

sarahsnow1
Copy link
Contributor

Overview

Fixes a bug in which cached data in LostClientManager prevented initial locations from being sent to listeners which had been removed and re-added.

Proposed Changes

  • Removes ReportedChanges class and simplifies LostClientManager to update information related to last reported location inline rather than use a transactional style API (void updateReportedValues(ReportedChanges changes))
  • Simplifies ClientManager API (removes internal state which makes shutdown unnecessary)
  • Adds LocationRequestReport to allow simplification of LostClientManager internal state (keeps track of last reported location for given LocationRequest and listener)
  • Updates FusionEngine to use recommended Location#getElapsedRealtimeNanos instead of non monotonic Location#getTime
  • Updates Clock interface to use nanos rather than millis
  • Adds HandlerFactory interface to allow constructor injection and easy mocking in tests
  • Updates and adds test coverage
  • Updates to gradle plugin 2.3.3

Closes #241

Copy link

@tallytalwar tallytalwar left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Thanks @sarahlensing for explaining the context and issue this was solving offline! 👍

@sarahsnow1 sarahsnow1 merged commit 20fc93e into master Oct 13, 2017
@sarahsnow1 sarahsnow1 deleted the 241-loc-updates branch October 13, 2017 23:37
@skypal
Copy link

skypal commented Jan 18, 2019

Unfortunately this commit broke the way GPX files are processed:
in LostClientManager.iterateAndNotify(), the timing of locations is eventually compared against location.getElapsedRealtimeNanos(). (Via SystemClock.getElapsedTimeInNanos()
However in TraceThread.nodeToLocation(), there is only a call to location.setTime() but not to location.setElapsedRealtimeNanos()

I created a new issue (#265) that addresses this problem in order to provide a fix as pull request...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants