Skip to content

Conversation

@kmadsen
Copy link
Contributor

@kmadsen kmadsen commented Dec 11, 2022

Description

In androidauto-v0.14.0 we added a reference to a new internal logger https://github.com/mapbox/mapbox-navigation-android/pull/6429/files#diff-2b11dc16c8381b48dcd005d941a0caf38d9fc488c65572d18f242e69b76fce3fR82
in navsdk 2.10.0-beta-3 we fixed an issue with that new internal logger using an api change that is not backwards compatible https://github.com/mapbox/mapbox-navigation-android/pull/6704/files#diff-7644c4d71ba636edcf531827e66e8b86febc6b98885ace62d7a676e275de1457L5

Essentially, androidauto-v0.14.0+ will not be compatible with 2.10-beta-3. This pull request is to make it so we can have a new release of androidauto-v0.18.1 that will be compatible with 2.10-beta-3+.

The fix is to not use the new lazy logger yet.

More details

Internal apis are not verified as being backwards compatible so this issue is not caught pre-release. We have also recently started with androidauto automated tests that will catch this type of issue. We have also not yet started the stable/beta android auto release channels so we are not yet releasing with 2.10-beta. How we have been supporting "upgradability" (allowing old versions of androidauto to use new versions of navsdk) is by only using backwards compatible apis (public semver protected). There is still one exception, the internal logging tool 😅. We may remove the internal logger from ui-androidauto for this reason.

@kmadsen kmadsen added the Android Auto Bugs, improvements and feature requests on Android Auto. label Dec 11, 2022
@kmadsen kmadsen requested a review from a team as a code owner December 11, 2022 00:06
@kmadsen kmadsen force-pushed the km-fix-upgrade-compatibility branch from 477b12e to b9e57b1 Compare December 11, 2022 00:11
@kmadsen kmadsen changed the title [Android Auto] Fix compatiblity issue with 2.10 [Android Auto] Fix compatiblity issue with 2.10.0-beta.3 Dec 11, 2022
import androidx.car.app.navigation.model.Step
import com.mapbox.navigation.ui.maneuver.api.MapboxLaneIconsApi
import com.mapbox.navigation.ui.maneuver.api.MapboxManeuverApi
import com.mapbox.navigation.utils.internal.ifNonNull
Copy link
Contributor Author

@kmadsen kmadsen Dec 11, 2022

Choose a reason for hiding this comment

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

this is unrelated but i searched for other internal imports and found this. unlikely this api will change but this function is not needed here anyways.

I'd like to add a linter for making sure no internal apis are used from the sdk, but haven't gotten around to it

@kmadsen
Copy link
Contributor Author

kmadsen commented Dec 11, 2022

0.18.1-km-fix-upgrade-compatibility-SNAPSHOT There's a snapshot in case anyone wants it before we cut releases next week

@codecov
Copy link

codecov bot commented Dec 11, 2022

Codecov Report

Merging #6714 (b9e57b1) into main (93a5e4a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #6714   +/-   ##
=========================================
  Coverage     72.50%   72.50%           
  Complexity     5510     5510           
=========================================
  Files           772      772           
  Lines         29891    29891           
  Branches       3529     3529           
=========================================
  Hits          21673    21673           
  Misses         6806     6806           
  Partials       1412     1412           

@kmadsen kmadsen merged commit 511d20f into main Dec 12, 2022
@kmadsen kmadsen deleted the km-fix-upgrade-compatibility branch December 12, 2022 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android Auto Bugs, improvements and feature requests on Android Auto.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants