Skip to content

Conversation

@VysotskiVadim
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

Changelog

Features

Bug fixes and improvements

  • Fixed an issue where DirectionsRoute#duration ignored charge time after refresh operation. [#7121](https://github.com/mapbox/mapbox-navigation-android/pull/7121)
  • Improved quality of continuous alternatives, now the mechanism respects routes from current navigation session. [#7137](https://github.com/mapbox/mapbox-navigation-android/pull/7137)
  • Improved EV offline navigation, now it fallbacks to a regular onboard routing. [#7137](https://github.com/mapbox/mapbox-navigation-android/pull/7137)
  • Improved location simulation when DR ends, i.e. when a driver leaves a tunnel. [#7137](https://github.com/mapbox/mapbox-navigation-android/pull/7137)

Known issues ⚠️

Other changes

Android Auto Changelog

Features

Bug fixes and improvements

@VysotskiVadim VysotskiVadim marked this pull request as ready for review April 25, 2023 16:09
Comment on lines +78 to +81
/**
* Not finished yet, see https://mapbox.atlassian.net/browse/NAVAND-1311
*/
internal const val NOTIFICATION = 11
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dzinad , @averkhaturau , as I know the notification feature isn't stable on server side and the API may change. Do I understand correctly that current implementation in NN works for current design and just stop working in case of changes on server side without crashes?

Copy link
Contributor

Choose a reason for hiding this comment

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

@VysotskiVadim
Copy link
Contributor Author

I haven't found any existing filtering mechanism for RoadObjectMatcherObserver.onRoadObjectMatched. Do you think it's okay if it will be called with an internal type which is not supported yet?
cc @dzinad , @LukasPaczos , @DzmitryFomchyn

@LukasPaczos
Copy link

Could we instead pass an error with a message that this type is not supported?

@VysotskiVadim
Copy link
Contributor Author

Could we instead pass an error with a message that this type is not supported?

@LukasPaczos , maybe don't call callback at all?

@LukasPaczos
Copy link

We're not exposing RoadObjectMatcher.matchNotificationObjects at all so do I understand correctly that it should never truly happen? In this case we could even assert 🙃

@VysotskiVadim
Copy link
Contributor Author

We're not exposing RoadObjectMatcher.matchNotificationObjects at all so do I understand correctly that it should never truly happen? In this case we could even assert 🙃

Unfortunately I'm not really familiar with EH, but I see that route object matcher is a part of public API.

From what I see, it can happen. @LukasPaczos , I plan to add filtering to road object matcher similar to how it works here, do you see any red flag in this approach?

@LukasPaczos
Copy link

I don't see any red flags.

@VysotskiVadim
Copy link
Contributor Author

I explored RoadObjectMatcher and found out that it works by request and always return objects of custom type. So no filtering for notifications is needed there

@VysotskiVadim VysotskiVadim enabled auto-merge (squash) April 26, 2023 14:49
@VysotskiVadim VysotskiVadim merged commit 7490829 into main Apr 26, 2023
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.

4 participants