Skip to content

Comments

Integrate FLP as a Location source#220

Draft
ithinkihaveacat wants to merge 3 commits intoandroid:mainfrom
ithinkihaveacat:integrate-flp
Draft

Integrate FLP as a Location source#220
ithinkihaveacat wants to merge 3 commits intoandroid:mainfrom
ithinkihaveacat:integrate-flp

Conversation

@ithinkihaveacat
Copy link
Contributor

@ithinkihaveacat ithinkihaveacat commented Jan 23, 2024

  • Comments welcome, but please don't look too deeply into this for the moment!

TODO

  • Make FLP conditional, and default to off. (Or move the extension function to horologist.)
  • Implement onAvailabilityChanged() in the proxy? (Might not be worth it.)
  • Pass through accuracy? (Probably not worth it.)
  • Any way to avoid the @Suppress annotations?
  • … and probably other stuff.

* [callbackFlow] is used to bridge between a callback-based API and Kotlin flows.
*/
@OptIn(ExperimentalCoroutinesApi::class)
@SuppressLint("MissingPermission")
Copy link
Contributor

Choose a reason for hiding this comment

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

Worrying to have suppressions. Are there ways to structure without needing a suppression?

requestLocationUpdates(locationRequest, locationListener, Looper.getMainLooper()) // is this the right looper?
awaitClose { removeLocationUpdates(locationListener) }

Log.d("qqqqqq", "requested location updates")
Copy link
Contributor

@yschimke yschimke Jan 23, 2024

Choose a reason for hiding this comment

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

this won't display, awaitClose{} is blocking. move above?


override fun onExerciseUpdateReceived(update: ExerciseUpdate) {

val hasLocation = update.latestMetrics.getData(DataType.LOCATION).isNotEmpty()
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth some comments why you are working around WHS. Or is this something health-services-client should be doing?


@SuppressLint("RestrictedApi")
@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
fun DataPointContainer.fromLocation(l: Location) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels awkward, ideally would only be in test code.

If you want to leave it, it should have a TODO linking to a tracking bug requesting these APIs be public.

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.

2 participants