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 check to remove listener from location engine in NavigationService #1026

Merged
merged 1 commit into from
Jun 14, 2018

Conversation

danesfeder
Copy link
Contributor

Seeing a NPE on removing this LocationEngine listener in CI. Seems to be a race, when the NavigationService is created, navigation does not begin, and then the service is immediately destroyed.

Noting: this null check is defensive and the service could use a larger refactor (there are many more null checks). We should keep this in mind / how we can extract most of the logic in the service into their own Java classes to make it more testable.

@danesfeder danesfeder added bug Defect to be fixed. navigation-core labels Jun 13, 2018
@danesfeder danesfeder added this to the 0.15.0 milestone Jun 13, 2018
@danesfeder danesfeder self-assigned this Jun 13, 2018
Copy link
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

Noting: this null check is defensive and the service could use a larger refactor (there are many more null checks). We should keep this in mind / how we can extract most of the logic in the service into their own Java classes to make it more testable.

👍

@danesfeder danesfeder merged commit 45493b9 into master Jun 14, 2018
@danesfeder danesfeder deleted the dan-check-engine branch June 14, 2018 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defect to be fixed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants