Skip to content

Integrate battery-efficient Activity Recognition and update documentation#49

Open
manojvermamv wants to merge 15 commits into
gabriel-sisjr:developfrom
manojvermamv:feature/activity-recognition-tracking
Open

Integrate battery-efficient Activity Recognition and update documentation#49
manojvermamv wants to merge 15 commits into
gabriel-sisjr:developfrom
manojvermamv:feature/activity-recognition-tracking

Conversation

@manojvermamv

@manojvermamv manojvermamv commented Jun 22, 2026

Copy link
Copy Markdown

feat: Battery-Efficient Activity Recognition & Dynamic GPS Throttling

Summary

Adds cross-platform Activity Recognition support that automatically pauses and resumes GPS updates based on the user's physical motion state, significantly reducing battery drain while stationary.

What Changed

Android

  • ActivityRecognitionProvider.kt — Play Services ActivityRecognitionClient wrapper (event-based transitions + polling)
  • ActivityProvider.kt / ActivityProviderFactory.kt — Abstract interface and factory
  • ActivityReceiver.kt — Manifest-registered BroadcastReceiver routing events to LocationService
  • LocationService.kt — Pauses/resumes requestLocationUpdates on STILL ↔ MOVING transitions
  • [Fix] LocationService.kt — Added missing fields to parseTrackingOptionsFromBundle to ensure GPS pausing behavior survives service crash-recovery and Intent redelivery.
  • [Fix] LocationService.kt — Extended isStill detection to include DetectedActivity.TILTING and DetectedActivity.UNKNOWN. TILTING fires when the device angle changes relative to gravity (e.g. placed on a desk) but is not locomotion; UNKNOWN fires when GMS lacks confidence to classify motion. Both are now treated as stationary so GPS pauses immediately without waiting for a STILL confirmation event.
  • LocationStorage.kt / TrackingStateEntity.kt — Persists activityTrackingEnabled, pauseLocationWhenStill, and activityUpdateInterval in Room so GPS-pause behaviour survives service crash-recovery.
  • LocationDatabase.kt — Schema version bumped to 2 (destructive migration) to accommodate new TrackingStateEntity columns.
  • AndroidManifest.xmlACTIVITY_RECOGNITION permission declarations

iOS

  • ActivityProvider.swiftCMMotionActivityManager wrapper via CoreMotion
  • [Fix] ActivityProvider.swift — Added a defensive NSMotionUsageDescription bundle check to prevent instantaneous iOS crashes if the developer forgets to configure Info.plist.
  • LocationManagerWrapper.swift — Implements ActivityProviderDelegate for pause/resume
  • BackgroundLocation.mm — Bridges new options through Codegen transportDictFromCodegenSpec:
  • [Fix] TrackingOptions.swift — Fixed Swift compilation error by explicitly initializing new let properties in the nil dictionary fallback initializer.
  • BackgroundLocation.podspec — Added CoreMotion to s.frameworks

TypeScript Bridge (both platforms)

  • TrackingOptions — 3 new optional fields: activityTrackingEnabled, pauseLocationWhenStill, activityUpdateInterval
  • NativeBackgroundLocation.ts (TurboModule spec), trackingOptionsMapper.ts, tracking.ts types all updated

Tests

  • ActivityRecognitionProviderTest.kt — MockK unit tests for client lifecycle and cleanup
  • TrackingOptionsTest.kt — Validates new fields, defaults, and parsing

Usage

startTracking({
  activityTrackingEnabled: true,
  pauseLocationWhenStill: true,
  activityUpdateInterval: 60000, // Android polling interval
});

@gabriel-sisjr

Copy link
Copy Markdown
Owner

Hi @manojvermamv 👋

First of all, thank you! For your contribution to this library, so it genuinely means a lot. 🎉

This is a really well-put-together PR: cross-platform parity (Android ActivityRecognitionClient + iOS CoreMotion), the new optional TrackingOptions fields kept fully backward-compatible, unit tests on both the provider lifecycle and option parsing, and the docs/CHANGELOG already updated. The battery-efficiency angle pausing GPS on STILL ↔ MOVING transitions is exactly the kind of feature this library benefits from.

I'll go through it carefully over the next few days, paying particular attention to the native lifecycle handling (pause/resume around requestLocationUpdates, receiver registration) and how the new flow interacts with the existing recovery and geofencing paths. I may leave a few inline questions or suggestions as I review.

Thanks again for taking the time to contribute i really appreciate it. I'll follow up soon. 🙏

@gabriel-sisjr gabriel-sisjr self-assigned this Jun 22, 2026
@gabriel-sisjr gabriel-sisjr added area: native Native module or bridge layer. area: hooks React hooks implementation. area: docs Documentation site or content. area: example Example app or demo project. labels Jun 22, 2026
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/utils/trackingOptionsMapper.ts 33.33% 1 Missing and 1 partial ⚠️
src/utils/trackingOptionsDefaults.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

…, pauseLocationWhenStill, activityUpdateInterval from Bundle

Without this fix the 3 activity-recognition fields were silently dropped on
service crash-recovery restarts (when intent is re-delivered). As a result
getActivityTrackingEnabledOrDefault() always returned false and GPS was never
paused when the device became stationary.
If the consumer enables activityTrackingEnabled but forgets to add
NSMotionUsageDescription to their Info.plist, CMMotionActivityManager
will crash the app instantly. This adds a safe check to gracefully
fallback to non-activity-tracked GPS instead.
Swift requires all non-optional let properties to be initialized before calling super.init(). The activityTrackingEnabled, pauseLocationWhenStill, and activityUpdateInterval properties were missing from the early-return (dictionary == nil) branch, causing a compilation error on iOS.
… activity types

TILTING fires when device angle changes relative to gravity (e.g. placed
on desk) — not locomotion. UNKNOWN fires when GMS lacks confidence to
classify motion. Both are now treated as stationary so GPS pauses
immediately without waiting for STILL confirmation.

Also persists activityTrackingEnabled, pauseLocationWhenStill, and
activityUpdateInterval in TrackingStateEntity (Room) and LocationStorage
so that GPS-pause behaviour survives service crash-recovery and Intent
redelivery. LocationDatabase version bumped to 2 for destructive migration.
@gabriel-sisjr

gabriel-sisjr commented Jun 26, 2026

Copy link
Copy Markdown
Owner

First off, thank you for this contribution, @manojvermamv, and welcome. This is a thoughtful, well-structured PR. The new ActivityProvider / ActivityProviderFactory / ActivityRecognitionProvider trio faithfully mirrors the library's existing location-provider pattern, the TurboModule option plumbing is correct end-to-end, the iOS crash-guard and PendingIntent mutability discipline show real care, and the rollout posture is safely opt-in and default-off. The architecture is the strongest part of this PR and we want to keep it. The change is close to mergeable in shape, but two critical issues block it as shipped. Both stem from this being a fleet and driver tracking library, where a lost location fix is the core failure mode.

ℹ️ A fuller line-by-line internal review backs every finding ID below. This comment is the complete, self-contained summary, and you need nothing else to act on it.

Critical blockers

C1: Data loss on upgrade (destructive Room migration)

  • Where: database/LocationDatabase.kt:19 (version=2) and :52 (.fallbackToDestructiveMigration()); android/schemas/ ships only 1.json (no 2.json).
  • Why it matters: The v1→v2 bump has no Migration(1,2), so on a 1.0.01.1.0 upgrade Room DROPs and recreates all four tables, destroying unsynced location fixes and the active-trip recovery state. The CHANGELOG calls this "non-breaking," which is inaccurate for persisted data. The schema change is in fact purely additive (3 nullable columns on tracking_state).
  • Fix: Add a Migration(1,2) (ALTER TABLE tracking_state ADD COLUMN ×3 nullable), keep fallbackToDestructiveMigration() only as a last-resort safety net, and commit the exported 2.json so the migration is validated in the build.

C2: GPS suppressed mid-trip via the documented default config

  • Where: README.md:79-80 (quick-start recommends activityTrackingEnabled: true and pauseLocationWhenStill: true); LocationManagerWrapper.swift:450 (stopUpdatingLocation()).
  • Why it matters (fleet domain): A vehicle routinely reports STILL at red lights and while idling, so the documented default pauses GPS mid-trip. On foregroundOnly iOS trips there is no significant-change fallback, so the device goes dark until motion resumes. Dropping fixes mid-trip is the exact failure this fleet and driver product exists to prevent, and the docs steer users straight into it by making the pause behavior part of the canonical first-run config.
  • Fix: Never pause while the detected activity is automotive; require a confidence threshold before pausing; add a resume grace period or timeout; and remove pauseLocationWhenStill from the default Quick Start (keep it as an explicitly opt-in battery optimization).

High findings (7)

H1 to H7, blockers (click to expand)

  • H1: Resume latency up to ~60s during real movement. LocationService.kt (startActivityUpdates wires only requestActivityUpdates) and TrackingOptions.kt:34 (DEFAULT_ACTIVITY_UPDATE_INTERVAL = 60000L). Android resume relies solely on 60s polling, so a STILL→moving transition is not seen for up to ~60s, missing fixes at the start of every trip segment. The "resumes immediately on motion" claim is false for the shipped path. (Fixed by wiring transitions; see H6.)
  • H2: UNKNOWN / TILTING / low-confidence treated as stationary (Android). LocationService.kt:379-381 treats STILL/TILTING/UNKNOWN as still; ActivityReceiver.kt:25,27 uses mostProbableActivity and forwards only activity.type, discarding confidence. So unknown states (tunnels, pocket, weak signal) and transient or low-confidence states pause GPS during genuine movement, amplifying C2. Gate on high-confidence STILL only, and thread confidence through.
  • H3: iOS data race on activity state and provider lifecycle. ios/LocationManagerWrapper.swift mutates isLocationPausedDueToActivity / activityProvider on the serial queue (:452, :461, :96, :99, :102) and on main (:133, :131-132) with no synchronization. A stopTracking cleanup racing an in-flight onActivityStateChanged yields torn reads or undefined behavior, so GPS can stay paused after stop. Fails Swift 6 strict concurrency and ThreadSanitizer. Confine all mutation to one queue.
  • H4: Observability gap, GPS pauses with no JS event (both platforms). Android LocationService.onActivityStateChanged (Log.d only) and iOS (NSLog only); existing LocationWarningType in src/types/tracking.ts is not extended. A dispatcher sees the stream stop with no way to distinguish an intentional battery pause from broken tracking. Emit onLocationWarning (e.g. LOCATION_PAUSED_STILL / LOCATION_RESUMED), surfaced via useLocationUpdates.
  • H5: Dangerous permission force-inherited by ALL consumers (Android). android/src/main/AndroidManifest.xml:24-25 unconditionally declares the runtime/dangerous ACTIVITY_RECOGNITION permission (plus the legacy GMS variant). Via manifest merge every consuming app inherits it, forcing a Play Data-Safety disclosure and "Physical activity" prompts even on apps that never enable this default-off feature. A least-privilege violation with ecosystem-wide blast radius. Remove it from the library manifest, and document consumer-side opt-in.
  • H6: Dead code and a false CHANGELOG claim (transition API not wired). requestActivityTransitionUpdates / ActivityTransitionRequest appear only in ActivityProvider.kt, ActivityRecognitionProvider.kt, and ActivityRecognitionProviderTest.kt, with no production call site invoking them; the ActivityTransitionResult branch in ActivityReceiver.onReceive is dead at runtime. The advertised low-latency mechanism does not ship, and this is also the root cause of H1. Wire the transition path (which also fixes H1) or remove the dead methods, branch, and test, then correct the CHANGELOG to describe only the polling that ships.
  • H7: Near-zero effective test coverage; CI never runs the Kotlin tests. android/src/test/.../TrackingOptionsTest.kt and ActivityRecognitionProviderTest.kt cover only getters and thin GMS-wrapper forwarding; untested: LocationService.onActivityStateChanged, handleActivityStateChanged, ActivityReceiver.onReceive. Zero iOS tests, zero TS tests. Worse, .github/workflows/ci.yml runs no Gradle/Kotlin job, so even the new Kotlin tests never execute, leaving the PR "green by omission." Add a ./gradlew … testDebugUnitTest CI job; extract the pause/resume decision into a pure, unit-tested function; add TS mapper tests.

Medium findings (10)

M1 to M10, strongly recommended (click to expand)

  • M1 (ActivityReceiver.kt:26,36): log template contains a literal \${…} (escaped dollar) so getActivityString() never runs, strong evidence this path was never exercised on-device. Remove the backslashes.
  • M2 (ActivityRecognitionProvider.kt:25,36,47,61): every call is @SuppressLint("MissingPermission") with no checkSelfPermission, and the hook never requests ACTIVITY_RECOGNITION; without the grant the feature silently no-ops. Add a runtime check and a warning.
  • M3 (ios/ActivityProvider.swift): guards availability and NSMotionUsageDescription but never checks CMMotionActivityManager.authorizationStatus(); denied motion permission is a silent no-op. Check status and emit a warning.
  • M4 (ios/TrackingOptions.swift / BackgroundLocation.mm parsed and bridged but never consumed; src/types/tracking.ts JSDoc lacks @platform Android): activityUpdateInterval is a no-op on iOS and breaks the repo's own tagging convention. Tag it @platform Android, or drop it from the iOS transport.
  • M5 (ios/LocationStorage.swift:190-195,216-221, RecoveryManager.swift:73-93, LocationManagerWrapper.recoverTracking:41-63 unchanged, vs Android TrackingStateEntity:25-27 which persists): after iOS crash-recovery the activity fields default to false and tracking silently stops, contradicting the "same logic" parity claim. Persist and restore on iOS, or document the divergence.
  • M6 (provider/ActivityProviderFactory.kt): warns but still returns the GMS provider when Play Services is unavailable; unlike sibling LocationProviderFactory, it has no fallback, so non-GMS devices (AOSP, Huawei) silently no-op. Propagate isAvailable() so LocationService can skip or warn, or provide a fallback.
  • M7 (package.json hand-bumped to 1.1.0, plus a hand-written CHANGELOG): collides with the in-flight feature/expo branch, which already claims 1.1.0, guaranteeing a CHANGELOG conflict and breaking automated release-it. Drop the manual edits and reconcile numbering with the maintainer (e.g. 1.2.0).
  • M8 (TS and native, validation absent): docs say pauseLocationWhenStill "requires activityTrackingEnabled," but nothing enforces it; setting it alone is a silent no-op footgun. Warn (via guardLogger / onLocationWarning) on the invalid combo.
  • M9 (example/android/app/src/main/AndroidManifest.xml): re-declares ActivityReceiver with an intent-filter and a misleading "MUST be registered here" comment; both are wrong (manifest merge already adds it, and the PendingIntent is explicit). Teaches a pattern that could flip the receiver to exported. Delete the block.
  • M10 (BackgroundLocation.podspec:19 adds CoreMotion, vs ios/PrivacyInfo.xcprivacy unchanged): library-rejection risk is low (verified: no persisted motion data, not a required-reason API), but consuming apps must still disclose motion data. Annotate the manifest, or document the host-app disclosure duty.

Low / Nit (9)

L1 to L9, follow-up acceptable (click to expand)

  • L1 (ios/ActivityProvider.swift:42, LocationManagerWrapper.swift:448,454, ActivityReceiver.kt:26,36, LocationService.kt:385,389): activity description and confidence logged in release builds (no new GPS coords logged, verified). Gate behind #if DEBUG / BuildConfig.DEBUG.
  • L2 (the 4 new Kotlin files): 4-space indent vs .editorconfig indent_size=2. Reformat to 2-space.
  • L3 (LocationService.kt:40, :233, and inside onDestroy): trailing whitespace on blank lines (no linter enforces it, so CI will not catch it). Trim it.
  • L4 (package.json): trailing newline stripped; cosmetic only, verified it will not fail CI (no JSON prettier gate). Restore the newline.
  • L5 (LocationManagerWrapper.swift:457, ios/LocationAccuracy.swift:17, cf. :471/:485): resume path re-implements accuracy dispatch inline instead of reusing configureAndStart. (PASSIVE/NO_POWER are valid strings, so this is duplication, not a magic-string bug.) Extract a shared helper.
  • L6 (ios/LocationManagerWrapper.swift): the // MARK: - Significant Location Monitoring section header was deleted (unrelated churn). Restore it.
  • L7 (ActivityRecognitionProvider.kt): log tag "ActivityRecProvider" is abbreviated vs the full-class-name convention used by siblings (FusedLocationProvider). Use the full class name.
  • L8 (README.md:95-103, README.md:53 / CHANGELOG, ios/ActivityProvider.swift): quick-start startTracking example silently dropped tripId; the README/CHANGELOG "causes a runtime crash on launch" claim is inaccurate (the provider guards the key and no-ops, only on startTracking). Restore tripId, and reword.
  • L9 (PR title): not conventional-commit prefixed (the individual commits are). Suggest e.g. feat(activity): ….

What's done well

This review is meant to be fair. The design and the genuinely dangerous parts were handled with real care:

  • Clean, idiomatic architecture: the ActivityProvider / ActivityProviderFactory / ActivityRecognitionProvider trio faithfully mirrors the existing LocationProvider / LocationProviderFactory / FusedLocationProvider pattern.
  • Thorough, correct cross-platform option plumbing: TS → Codegen spec → mapper → Android Bundle/Room → iOS bridge/Swift, including the BackgroundLocation.mm transport-only pattern.
  • Thoughtful iOS crash-guard: the NSMotionUsageDescription presence check degrades gracefully.
  • Correct PendingIntent mutability discipline: mutable only where GMS needs result extras, immutable everywhere else.
  • Safe rollout posture: opt-in, default-off; podspec CoreMotion plus example Info.plist NSMotionUsageDescription correct.
  • Consistent Android round-trip: TrackingOptions / Bundle / Storage is internally consistent.

✅ Concerns investigated and cleared (8), credit where due

Concern Verdict
PendingIntent mutability unsafe Safe and correct: explicit, component-locked base intent; FLAG_MUTABLE only where GMS fills result extras
Cross-app receiver spoofing Closed: ActivityReceiver is exported="false" in both manifests; PendingIntents fire intra-app
Semver classification wrong MINOR is correct: optional fields are backward compatible (the 1.1.0 number collision is a separate repo-state issue, M7)
iOS resume guard is dead code Not dead: PASSIVE/NO_POWER are real LocationAccuracy enum strings
TS plumbing drops a field Correct end-to-end: all 3 fields pass through with false/0 preserved
iOS retain cycle None: weak delegate and [weak self] capture; teardown nils correctly
Native network egress None: motion and location stay on-device
package.json newline breaks CI Will not: no JSON lint gate (see L4)

Prioritized merge checklist

Must fix before merge (blockers)

  • C1: Add a Migration(1,2) (ALTER TABLE tracking_state ADD COLUMN ×3 nullable); keep destructive only as last resort; commit exported 2.json.
  • C2: Never pause while automotive; require a confidence threshold; add a resume grace or timeout; remove pauseLocationWhenStill from the default Quick Start.
  • H6: Wire up requestActivityTransitionUpdates or remove the dead methods, branch, and test; correct the CHANGELOG to match what ships.
  • H1: Resolve resume latency (wiring H6 fixes this); stop claiming "resumes immediately."
  • H2: Treat only high-confidence STILL as a pause trigger; exclude UNKNOWN/TILTING; thread confidence through.
  • H3: Confine activity-state mutation and provider lifecycle to one queue; restrict main to CLLocationManager start/stop.
  • H4: Emit onLocationWarning (LOCATION_PAUSED_STILL / LOCATION_RESUMED) on both platforms; surface via useLocationUpdates.
  • H5: Remove ACTIVITY_RECOGNITION from the library manifest; document consumer-side opt-in.
  • H7: Add a ./gradlew … testDebugUnitTest CI job; extract the pause/resume decision into a pure, unit-tested function; add TS mapper tests.

Should fix before merge

  • M1: Remove the backslashes so the Kotlin log template interpolates.
  • M2: Add a runtime ACTIVITY_RECOGNITION check and a warning (Android).
  • M3: Check CMMotionActivityManager.authorizationStatus() and emit a warning (iOS).
  • M4: Tag activityUpdateInterval @platform Android (or drop from the iOS transport).
  • M5: Persist or restore activity fields across iOS crash recovery, or document the divergence.
  • M6: Propagate isAvailable() from ActivityProviderFactory on non-GMS devices.
  • M7: Drop the manual version and CHANGELOG edits; reconcile numbering (e.g. 1.2.0).
  • M8: Warn when pauseLocationWhenStill is set without activityTrackingEnabled.
  • M9: Delete the example manifest receiver block; never export the receiver.
  • M10: Update PrivacyInfo.xcprivacy, or document the host-app motion-disclosure duty.

Nice to have (follow-up acceptable)

  • L1: Gate activity and confidence logging behind #if DEBUG / BuildConfig.DEBUG.
  • L2: Reformat the 4 new Kotlin files to 2-space indent.
  • L3: Trim trailing whitespace in LocationService.kt.
  • L4: Restore the trailing newline in package.json.
  • L5: Extract a shared accuracy-dispatch helper on the iOS resume path.
  • L6: Restore the deleted // MARK: - Significant Location Monitoring header.
  • L7: Use the full class name for the ActivityRecognitionProvider log tag.
  • L8: Restore tripId in the README quick-start; reword the "crash on launch" claim.
  • L9: Re-title the PR with a conventional-commit prefix (e.g. feat(activity): …).

Open questions for you

A few things would help us land this faster. No rush, and thanks again:

  1. Transition API (H6/H1): Was requestActivityTransitionUpdates meant to be wired into LocationService? The CHANGELOG advertises it but only polling is connected. Would you prefer to wire the transition path (preferred, since it also fixes resume latency) or remove it?
  2. Destructive migration (C1): Was fallbackToDestructiveMigration() on the v1→v2 bump intentional? Since the change is purely additive, an ALTER TABLE migration preserves all persisted fleet data. Any reason not to add it?
  3. activityUpdateInterval on iOS (M4): Is this meant to affect iOS at all? CMMotionActivityManager is event-driven with no interval. Can we confirm it should be Android-only (and tag it accordingly)?
  4. Fleet pause semantics (C2/H2): What is the intended behavior while driving? Should the feature ever pause during automotive activity, and what confidence threshold did you have in mind?
  5. Versioning (M7): Both this PR and the in-flight feature/expo branch claim 1.1.0. How would you like to reconcile the numbering and CHANGELOG so the automated release-it flow stays authoritative?
  6. On-device verification (M1/H7): Has the new Android path been run on a physical device? The literal \${…} log template suggests maybe not. Confirming just helps us scope the testing gap.

Bottom line: Strong, idiomatic architecture that is close to mergeable in shape. Resolve the two Critical blockers and the High set (ideally the Mediums too), and this becomes a valuable, well-built battery-optimization feature we would be glad to ship. Looking forward to the next revision!

@gabriel-sisjr gabriel-sisjr added the status: under review Review in progress. label Jun 26, 2026
Add Android unit test job to CI and finalize Android SDK setup. Add Room migration (1→2) to add activityTrackingEnabled, pauseLocationWhenStill and activityUpdateInterval columns and register it to avoid destructive drops. Clarify README: NSMotionUsageDescription now gracefully falls back and update startTracking example. Remove example AndroidManifest ActivityReceiver entry. Improve Android activity-related code (ActivityReceiver, ActivityProvider, ActivityRecognitionProvider, factory) with logging/formatting cleanups. Add tracking option validation warning (pauseLocationWhenStill requires activityTrackingEnabled) and annotate activityUpdateInterval platform. Minor iOS marker and trailing-newline/package formatting fixes.
…automotive exclusion, confidence threshold, queue confinement, permission opt-in, dead code removal, warning events, CI test job
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation site or content. area: example Example app or demo project. area: hooks React hooks implementation. area: native Native module or bridge layer. status: under review Review in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants