maplibre / maplibre-navigation-android

Maplibre Navigation SDK for Android
MIT License
96 stars 53 forks source link

Offroute listener issue? #58

Closed moiatto closed 1 year ago

moiatto commented 1 year ago

Yesterday I updated the dependancies to last versions (before v1.2.0 & v10.0.0-pre.0)

implementation 'com.github.maplibre:maplibre-navigation-android:2.0.0'
implementation 'org.maplibre.gl:android-sdk:10.2.0'

This is the inizialitation (it's the same as before. Only changed LocationEngineProvide (deprecated) with LocationEngine)

// Initialize MapboxNavigation and add listeners
MapboxNavigationOptions options = MapboxNavigationOptions.builder()
        .isDebugLoggingEnabled(true)
        .build();
navigation = new MapboxNavigation(context, options);
navigation.addNavigationEventListener(this);
navigation.addMilestoneEventListener(this);

LocationEngine locationEngine = new LocationEngineProxy(new MapboxFusedLocationEngineImpl(context.getApplicationContext()));
navigation.setLocationEngine(locationEngine);

For each navigation, the userOffRoute listener is always called even if you follow the route (about every 100 meters).

I tried to add in the MapboxNavigationOptions options = MapboxNavigationOptions.builder()

.maximumDistanceOffRoute(20)
.minimumDistanceBeforeRerouting(50)

but nothing.

Did I forget to add something?

boldtrn commented 1 year ago

Not sure if it helps, but at least for us the MapboxFusedLocationEngineImpl did not work well (depends on the device). I created a PR already at maplibre to have the AndroidProvider as a public class. Not sure if this is related to your case, but maybe you can try the GoogleLocationEngineImpl if it works with that.

moiatto commented 1 year ago

Thanks for your reply. I can't find anything to implement GoogleLocationEngineImpl.

It's hard without a help guide. I spent a lot of time to understand all the library.

Thanks to this guide I implemented offline map: https://medium.com/@ty2/how-to-display-offline-maps-using-maplibre-mapbox-39ad0f3c7543 it might be useful to others as well...but can be better if anyone help to add more than one map ;-)

boldtrn commented 1 year ago

It's hard without a help guide. I spent a lot of time to understand all the library.

It's an open source project, feel free to contribute documentation, guides and more, I am happy to review them. I wanted to update quite a few things as well, but unfortunately, right now my time is very limited.

Fabi755 commented 1 year ago

We have the same problem. I took a look into the code.

The minimumDistanceBeforeRerouting option seems to be used not only for rerouting. It is also used for calculating the off-route distance. While the maximumDistanceOffRoute option is not used for any logic.

The calculations are done in the OffRouteDetector and ToleranceUtils classes.

I would like to fix this problem so that it works again. But before I can do that,

  1. I need to understand why only one option is used?
  2. And we need to find out if we need only one option. Or both values and we should use them in different ways.

Do you have some ideas or hints for this topic?

boldtrn commented 1 year ago

I made a change here about half a year ago: https://github.com/maplibre/maplibre-navigation-android/commit/84e9f77a5563f620777197af46649e05b4cc7136

Before, we used constants that were not changeable. Now we use the option. I used the same option as the constant that was used before.

I am still not 100% sure about the issue here, AFAIK it would be good enough to have one option.

Fabi755 commented 1 year ago

Thanks for your response and the linked commit!

I spent more time on the code and existing Mapbox issues and found multiple issues:

  1. As already mentiond the option maximumDistanceOffRoute is not used
  2. On OffRouteDetector.java#L231 the < should be a > (or even an other calculation).
  3. There are a similar Mapbox issues (Mapbox#1081) that describe our problem too, maybe we need to fix here something more.

The second point is the cause of @moiatto's problem. If you moving away from current step, the off route will triggered immediately and not as expected after the minimum value of 50 meters that are defined in MINIMUM_BACKUP_DISTANCE_FOR_OFF_ROUTE. This can happen when the GPS is not accurate enough.

I will now start to fix these problems and then create a PR for it.

boldtrn commented 1 year ago

I think you are right, the method movingAwayFromManeuver could use some tune up.

Fabi755 commented 1 year ago

The problem is fixed.