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.

Sign up for GitHub

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

Jump to bottom

NMEA Updates #1490

Merged
merged 8 commits into from May 16, 2024
Merged

NMEA Updates #1490

merged 8 commits into from May 16, 2024

Conversation

Wackymax
Copy link
Contributor

This update includes the following changes:

  1. Android 34 now includes properties for MSL altitude so we can use that instead of the MSL from NMEA messages when available.
  2. We now support reading MSL NMEA messages from other constellations.
  3. Always start the NMEA listener so we can include GPS fix data.
  4. Upgraded Android packages to the latest versions.

Fixes: #1473
Fixes: #1479

Pre-launch Checklist

  • I made sure the project builds.
  • X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is does not need version changes.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I rebased onto main.
  • I added new tests to check the change I am making, or this PR does not need tests.
  • I made sure all existing and new tests are passing.
  • I ran dart format . and committed any changes.
  • I ran flutter analyze and fixed any errors.

mvanbeusekom
mvanbeusekom reviewed May 16, 2024
View reviewed changes
Copy link
Member

@mvanbeusekom mvanbeusekom left a comment •
edited

Choose a reason for hiding this comment

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

Hi @Wackymax,

Thanks for yet another great PR, I like the update however I have one concern about the update of the com.google.android.gms:play-services-location and androidx.core:core dependencies.

It would be great to hear your thoughts about it.

geolocator_android/android/build.gradle
Comment on lines +44 to +45
implementation 'com.google.android.gms:play-services-location:21.2.0'
implementation 'androidx.core:core:1.13.0'
Copy link
Member

Choose a reason for hiding this comment

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

I am a bit afraid updating these dependencies introduce a breaking change. I have done an upgrade to version 21.1.0 of the com.google.android.gms:play-services-location which forced users to update AGP to version 8.0.0 and Kotlin to 1.9.0.

I have reverted the change as we didn't need any of the new functionality, if we do need it to support the MSL altitude we should probably update the version number to 5.0.0 and clearly mention in the CHANGELOG that users should update their Android Gradle Plugin to version 8.0.0 and Kotlin to version 1.9.0.

@Wackymax
Copy link
Contributor Author

Wackymax commented May 16, 2024 via email

@mvanbeusekom
Copy link
Member

mvanbeusekom commented May 16, 2024 •
edited

I believe we do need it to support the new MSL functionality. I also don't like to let versions fall behind to much for security concerns etc. Id rather update to a new major version then?

That sounds great to me. Personally I also don't like to fall behind, so if you can update to a new major release and clearly mention that users need to update to AGP version 8.0.0 or higher in the CHANGELOG that would be great.

One interesting thing I noticed, the example application is still using version 7.6.1 of Gradle (see gradle-wrapper.properties) and version 7.4.2 of the Android Gradle Plugin (see settings.gradle). So maybe Google fixed something between version 21.1.0 and 21.2.0 not forcing users to upgrade anymore.

Here is the original issues people submitted when I did the update: #1441 and #1442

@Wackymax
Copy link
Contributor Author

Wackymax commented May 16, 2024 via email

@mvanbeusekom
Copy link
Member

Yeah I also haven't updated in my personal app and haven't run into any problems running on this commit. Should I still update to a newer version in this case?

Let me do a few tests, see if I can reproduce the original error with version 21.1.0 and if it still occurs when I update to 21.2.0. I will get back to you soon ;). Thanks for your patience.

@mvanbeusekom
Copy link
Member

TL;DR: Updating to version 21.2.0 is safe. No need to do a major version update.

Long(er) version:

To ensure the update of the com.google.android.gms:play-services-location dependency doesn't result in any breaking changes I created a fresh Flutter project and added a dependency to the geolocator_android package bases on a path reference:

  1. Create new Flutter project: flutter create -a java --platforms android gradle_test
  2. Add dependency on geolocator_android package:
dependencies:
  geolocator_android:
    path: /Users/maurits/sources/Baseflow/Internal/Flutter/geolocator/geolocator_android

The geolocator_android package at the moment is the version on the main branch and the com.google.android.gms:play-services-location version is set to 21.0.1, so we have a baseline. Running flutter build apk works without any problems.

Next I updated the com.google.android.gms:play-services-location version to 21.1.0 and again run the flutter build apk command. This now results in the following error (as described in issue #1441):

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:checkReleaseDuplicateClasses'.
> A failure occurred while executing com.android.build.gradle.internal.tasks.CheckDuplicatesRunnable
   > Duplicate class kotlin.collections.jdk8.CollectionsJDK8Kt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.internal.jdk7.JDK7PlatformImplementations found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.internal.jdk7.JDK7PlatformImplementations$ReflectSdkVersion found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.internal.jdk8.JDK8PlatformImplementations found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.internal.jdk8.JDK8PlatformImplementations$ReflectSdkVersion found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.io.path.ExperimentalPathApi found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.io.path.PathRelativizer found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.io.path.PathsKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.io.path.PathsKt__PathReadWriteKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.io.path.PathsKt__PathUtilsKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.jdk7.AutoCloseableKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk7-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.10)
     Duplicate class kotlin.jvm.jdk8.JvmRepeatableKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.jvm.optionals.OptionalsKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.random.jdk8.PlatformThreadLocalRandom found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.streams.jdk8.StreamsKt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.streams.jdk8.StreamsKt$asSequence$$inlined$Sequence$1 found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.streams.jdk8.StreamsKt$asSequence$$inlined$Sequence$2 found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.streams.jdk8.StreamsKt$asSequence$$inlined$Sequence$3 found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.streams.jdk8.StreamsKt$asSequence$$inlined$Sequence$4 found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.text.jdk8.RegexExtensionsJDK8Kt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)
     Duplicate class kotlin.time.jdk8.DurationConversionsJDK8Kt found in modules jetified-kotlin-stdlib-1.9.0 (org.jetbrains.kotlin:kotlin-stdlib:1.9.0) and jetified-kotlin-stdlib-jdk8-1.7.10 (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10)

     Go to the documentation to learn how to <a href="d.android.com/r/tools/classpath-sync-errors">Fix dependency resolution errors</a>.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 13s
Running Gradle task 'bundleRelease'...                             13.8s
Gradle task bundleRelease failed with exit code 1

Next I updated the com.google.android.gms:play-services-location to version 21.2.0 and again ran the flutter build apk command. This time however the build worked out without any problem.

mvanbeusekom
mvanbeusekom approved these changes May 16, 2024
View reviewed changes
Copy link
Member

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Wackymax for another great contribution.

@mvanbeusekom mvanbeusekom merged commit 6b34beb into Baseflow:main May 16, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvanbeusekom mvanbeusekom mvanbeusekom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@Wackymax @mvanbeusekom