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.

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

[Android Auto] Allow early initialization #1603

Merged
merged 5 commits into from
Aug 22, 2022
Merged

Conversation

kmadsen
Copy link
Contributor

@kmadsen kmadsen commented Aug 16, 2022

Summary of changes

The MapboxCarMap has boilerplate to setup in android auto because it cannot be initialized until the CarContext has received an onCreate lifecycle callback. There is a clear pattern in any session or screen that can be resolved with an extension.

Here is the current solution. You keep a nullable or lateinit var of the MapboxCarMap. You then init a lifecycle observer which will create MapboxCarMap after the onCreate lifecycle callback. The MapboxCarMapObserver is an asynchronous callback, so it does not make sense to require asynchronous construction of the MapboxCarMap

class MapSession : Session() {
  private val carMapShowcase = CarMapShowcase()
  private var mapboxCarMap: MapboxCarMap? = null

  override fun onCreateScreen(intent: Intent): Screen {
    return MapScreen(mapboxCarMap!!)
  }

  override fun onCarConfigurationChanged(newConfiguration: Configuration) {
    carMapShowcase.loadMapStyle(carContext)
  }

  init {
    lifecycle.addObserver(object : DefaultLifecycleObserver {
      override fun onCreate(owner: LifecycleOwner) {
        mapboxCarMap = MapboxCarMap(MapInitOptions(carContext))
          .registerObserver(carMapShowcase)
          .registerObserver(CarMapWidgets())
      }

      override fun onDestroy(owner: LifecycleOwner) {
        mapboxCarMap?.clearObservers()
        mapboxCarMap = null
      }
    })
  }
}

Synchronous construction

This change separates construction from initialization. To make it easier to configure the MapboxCarMap before session is created. This is still lazy loading data, but is making the syntax and data management easier.

class MapSession : Session() {
  private val carMapShowcase = CarMapShowcase()
  private var mapboxCarMap = MapboxCarMap()

  override fun onCreateScreen(intent: Intent): Screen {
    return MapScreen(mapboxCarMap)
  }

  override fun onCarConfigurationChanged(newConfiguration: Configuration) {
    carMapShowcase.loadMapStyle(carContext)
  }

  init {
    lifecycle.addObserver(object : DefaultLifecycleObserver {
      override fun onCreate(owner: LifecycleOwner) {
        mapboxCarMap = mapboxCarMap.setup(carContext)
          .registerObserver(carMapShowcase)
          .registerObserver(CarMapWidgets())
      }

      override fun onDestroy(owner: LifecycleOwner) {
        mapboxCarMap?.clearObservers()
        mapboxCarMap = null
      }
    })
  }
}

New installers

After the MapboxCarMap can be initialized before onCreate, this makes it so you can initialize a Session or Screen with a specified configuration. This removes the need to add a lifecycle observer, but still allows you to do so . And you can specify which lifecycle your observers should be attached to (on every app open? only when the session is created? and so on).

class MapSession : Session() {
  private val carMapShowcase = 
  private val mapboxCarMap = mapboxMapInstaller()
    .created(CarMapWidgets())
    .started(carMapShowcase)
    .install {
      // Create custom map init options here.
      MapInitOptions(it)
    }

  override fun onCreateScreen(intent: Intent): Screen = MapScreen(mapboxCarMap)

  override fun onCarConfigurationChanged(newConfiguration: Configuration) {
    carMapShowcase.loadMapStyle(carContext)
  }
}

Screens are also simple, this allows you to specify controllers like a gesture handler without thinking. The focus is put on designing MapboxCarObserver implementations that create the desired experiences.

class MapScreen(
  mapboxCarMap: MapboxCarMap
) : Screen(mapboxCarMap.carContext) {
  private var isInPanMode: Boolean = false
  private val carCameraController = CarCameraController()

  init {
    mapboxMapInstaller(mapboxCarMap)
      .created(carCameraController)
      .gestures(carCameraController.gestureHandler)
      .install()
  }

...

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
  • Add example if relevant.
  • Document any changes to public APIs.
  • Run make update-api to update generated api files, if there's public API changes, otherwise the verify-api-* CI steps might fail.
  • Update CHANGELOG.md or use the label 'skip changelog', otherwise check changelog CI step will fail.
  • If this PR is a v10.[version] release branch fix / enhancement, merge it to main firstly and then port to v10.[version] release branch.

Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

PRs must be submitted under the terms of our Contributor License Agreement CLA.

@kmadsen kmadsen force-pushed the km-allow-early-initialization branch 3 times, most recently from 3f41b7c to 9eec41a Compare August 16, 2022 17:17
@kmadsen kmadsen force-pushed the km-allow-early-initialization branch from 9eec41a to 58fb234 Compare August 16, 2022 17:24
@kmadsen kmadsen marked this pull request as ready for review August 16, 2022 17:24
@kmadsen kmadsen requested review from a team as code owners August 16, 2022 17:24
@kmadsen kmadsen force-pushed the km-allow-early-initialization branch 3 times, most recently from 748743d to c7b318a Compare August 16, 2022 17:43
@kmadsen kmadsen force-pushed the km-allow-early-initialization branch from c7b318a to abe6ab0 Compare August 16, 2022 17:53
Copy link
Contributor

@evil159 evil159 left a comment

Choose a reason for hiding this comment

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

LGTM :D

@pengdev pengdev requested a review from yunikkk August 19, 2022 09:57
@pengdev
Copy link
Member

pengdev commented Aug 19, 2022

In general looks good to me, besides a few nits.

@kmadsen kmadsen force-pushed the km-allow-early-initialization branch from a892ec2 to cfa7318 Compare August 19, 2022 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants