Convert core module to Kotlin #127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sotomski
reviewed
Nov 26, 2024
app/src/main/java/org/maplibre/navigation/android/example/MockNavigationActivity.kt
Show resolved
Hide resolved
sotomski
reviewed
Nov 26, 2024
...ion/src/main/java/org/maplibre/navigation/android/navigation/v5/offroute/OffRouteDetector.kt
Outdated
Show resolved
Hide resolved
sotomski
reviewed
Nov 26, 2024
...java/org/maplibre/navigation/android/navigation/v5/navigation/MapLibreNavigationOptions.java
Show resolved
Hide resolved
sotomski
reviewed
Nov 26, 2024
...d-navigation/src/main/java/org/maplibre/navigation/android/navigation/v5/snap/SnapToRoute.kt
Outdated
Show resolved
Hide resolved
|
As discussed, I added the |
|
Round 2️⃣ Latest changes are ready now for a review. |
sotomski
reviewed
Dec 2, 2024
| const val NON_NULL_APPLICATION_CONTEXT_REQUIRED: String = | ||
| "Non-null application context required." | ||
|
|
||
| val WAYNAME_OFFSET: Array<Float> = arrayOf(0.0f, 40.0f) |
There was a problem hiding this comment.
Could we add @JvmField here to improve the kotlin -> java interop?
boldtrn
reviewed
Dec 3, 2024
| * | ||
| * @since 3.0.0 | ||
| */ | ||
| val maxSpeed: List<MaxSpeed>?, |
There was a problem hiding this comment.
Before maxSpeed was:
public abstract List<MaxSpeed> maxspeed();
Reading this value from json doesn't work anymore.
There was a problem hiding this comment.
Ahh, that was my bad. Thanks for finding this!
There was a problem hiding this comment.
JSON name is changed ✅
boldtrn
reviewed
Dec 3, 2024
CHANGELOG.md
Outdated
| @@ -4,6 +4,9 @@ Maplibre welcomes participation and contributions from everyone. | |||
|
|
|||
| ### unreleased | |||
|
|
|||
| **BREAKING CHANGES:** | |||
| - Convert module `libandroid-navigation` to Kotlin. See description of PR [#xxx] to get informed about all breaking changes | |||
There was a problem hiding this comment.
We need to fix the Changelog here.
There was a problem hiding this comment.
You mean the xxx right?
boldtrn
requested changes
Dec 3, 2024
boldtrn
approved these changes
Dec 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will convert the module
libandroid-navigationto Kotlin.I used Android Studio's converting tool and try my best to refactore the generated Kotlin code.
There are still classes that I didn't refactor, because the logic itself was already very complicated and ugly in Java. For this classes I think seperated follow up PRs make sense. Additional all Auto Models now data classes. Also I removed all unused classes from this module.
I tested the changes against the example app, and fix all occuring issues. But I suspect that I do not catched all.
I note following..
Breaking changes
org.maplibre.navigation.android.core.connectivity.* --> Removed. Not used internally. Make for me no sense to keep this.
org.maplibre.navigation.android.core.crashreporter.* --> Removed. Not used internally. Make for me no sense to keep this.
org.maplibre.navigation.android.core.FileUtils --> Removed. Only used by CrashReport
org.maplibre.navigation.android.navigation.v5.milestone.Milestone --> Remove Builder, use named arguments in constructor instead.
org.maplibre.navigation.android.navigation.v5.routeprogress.* --> Convert all @autovalue classes to Kotlin data classes. (Also Builder pattern is not used here anymore)
org.maplibre.navigation.android.navigation.v5.models.* -->
- Convert all @autovalue classes to Kotlin data classes. (Also Builder pattern is not used here anymore). Using KotlinX instead of GSON for JSON parsing.
- Convert annotation types to enums
- Using Kotlinx serialization instead of GSON for JSON parsing
- Remove classes MapLibreStreetsV8, DirectionsJsonObject, DirectionsAdapterFactory
org.maplibre.navigation.android.navigation.v5.models.utils.* --> Removed. Not used internally.
org.maplibre.navigation.android.navigation.v5.utils.abbreviation.* --> Removed. Not used internally. I think it is also not used by developers.
org.maplibre.navigation.android.navigation.v5.utils.TextUtils --> Removed. Can be replaced by Android's TextUtils or by Kotlin language functions.
org.maplibre.navigation.android.navigation.v5.utils.RouteUtils --> Now static (object) class.
org.maplibre.navigation.android.navigation.v5.navigation.NavigationLifecycleMonitor --> Removed. Not used internal.
org.maplibre.navigation.android.navigation.v5.navigation.NavigationMapRoute --> Deprecated/Removed. In UI package a newer version is available.
org.maplibre.navigation.android.navigation.v5.navigation.SdkVersionChecker --> Deprecated/Removed. Make no sense, use plain java/kotlin code.
org.maplibre.navigation.android.navigation.v5.navigation.NavigationTimeFormat --> Move to MapLibreNavigationOptions and migrate to enum class.
org.maplibre.navigation.android.navigation.v5.route.MapRouteProgressChangeListener --> Deprecated/Removed. In UI package a newer version is available.
org.maplibre.navigation.android.navigation.v5.route.OnRouteSelectionChangeListener --> Deprecated/Removed. In UI package a newer version is available.
org.maplibre.navigation.android.navigation.v5.location.MetricsLocation --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.location.replay.GpxParser --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.location.replay.ParseGpxTask --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.location.replay.ReplayJsonRouteDto --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.location.replay.ReplayJsonRouteLocationMapper --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.location.replay.ReplayLocationDto --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.location.replay.TimestampAdapter --> Removed. Seems not used.
org.maplibre.navigation.android.navigation.v5.navigation.camera.RouteInformation --> Remove Builder, use named arguments in constructor instead.
org.maplibre.navigation.android.navigation.v5.navigation.camera.Camera --> Remove function
target, because it's not used anywhere.org.maplibre.navigation.android.navigation.v5.models.RouteOptions --> Remove
*listfunctions. Seems not used anywhere.org.maplibre.navigation.android.navigation.v5.navigation.MapLibreNavigationOptions --> Remove
isFromNavigationUi, because seems not used anywhere.org.maplibre.navigation.android.navigation.v5.navigation.MapLibreNavigationOptions -->
minimumDistanceBeforeReroutingis splitt up to two new options.offRouteMinimumDistanceMetersAfterRerouteminimum meters after a reroute needs passedoffRouteThresholdRadiusMetersa fixed threshold, that will cause a off-route state, when passed.Refactor later
org.maplibre.navigation.android.navigation.v5.milestone.TriggerProperty --> Seems too complicated. Create a fresh new re-write.
org.maplibre.navigation.android.navigation.v5.location.replay.* --> Hard to refactor in clean Kotlin. Also seems too complicated. Create a fresh new re-write.
Add adapters or use DTOs instead of parsing directly to models that can be replaces by developers to use other API engines
org.maplibre.navigation.android.navigation.v5.utils.time.* --> Seems too complicated. Create a simpler version.
org.maplibre.navigation.android.navigation.v5.utils.RouteUtils --> Seems too complicated. Create simpler versions of util functions.
Rename engines to name with ending
engine. And also choose a standardized naming for implementations.- Camera --> CameraEngine
- Snap --> SnapEngine
- OffRoute --> OffRouteEngine
- FasterRoute --> FasterRouteEngine
Discuss
Can
LegAnnotationused without distance field? If not, we should thinking about to make the distance list non-null