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

Add fixes for SimulatedLocationProvider #327

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

ahmedre
Copy link
Contributor

@ahmedre ahmedre commented Oct 30, 2024

Instead of determining whether or not to start the simulation based on whether or not the job is null, instead, use the active state of the job to determine whether or not to start the simulator. This allows the SimulatedLocationProvider to be reused more than once, and also allows for addListener to happen before setSimulatedRoute is called without breaking the simulation.

Instead of determining whether or not to start the simulation based on
whether or not the job is null, instead, use the active state of the job
to determine whether or not to start the simulator. This allows the
SimulatedLocationProvider to be reused more than once, and also allows
for addListener to happen before setSimulatedRoute is called without
breaking the simulation.
@ahmedre
Copy link
Contributor Author

ahmedre commented Oct 30, 2024

isActive is true whenever the coroutine is neither canceled nor complete.

Before this fix, if addListener is called before setSimulatedRoute, the job will run, find the start and next state as the same, and therefore assume the simulation is done. The coroutine is marked as complete, and when setSimulatedRoute is called, nothing happens. By checking the state of the coroutine, we allow re-running this when setSimulatedRoute is called, whenever the actual job is complete (or hasn't been set or has been canceled).

There's also no way to "stop" the job without making it run all the way through at the moment, so maybe we should also add some cancelation call at some point to stop the simulation.

Copy link
Contributor

@ianthetechie ianthetechie left a comment

Choose a reason for hiding this comment

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

Good catch; Thanks!

@ianthetechie ianthetechie merged commit 6133f12 into stadiamaps:main Oct 31, 2024
14 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