From 7c913dba7735b2b7111dcd2c0fc7e769b00080bf Mon Sep 17 00:00:00 2001
From: Paul Tran-Van <paul.tranvan@gmail.com>
Date: Mon, 26 Feb 2024 16:56:27 +0100
Subject: [PATCH] feat: Separate start and init tracking

We used to rely on a single `startTracking` method that was both
initializating the tracking, and started it. Furthermore, the event
handlers were made outside of this function.
This seems to be an anti-pattern, and we suspect that it was causing
tracking failures: we noticed several cases of missing
motion events, which might be the result of a wrong init.
Now, the init is done at each app start, and the tracking is actually
started when we detect the state is not enabled, just like before.
---
 .../hooks/useGeolocationTracking.ts           |  5 +-
 .../domain/geolocation/services/tracking.ts   | 12 +++-
 src/app/domain/geolocation/tracking/index.js  | 56 +++++++++++++------
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/src/app/domain/geolocation/hooks/useGeolocationTracking.ts b/src/app/domain/geolocation/hooks/useGeolocationTracking.ts
index a1fd824a7..0af29ee89 100644
--- a/src/app/domain/geolocation/hooks/useGeolocationTracking.ts
+++ b/src/app/domain/geolocation/hooks/useGeolocationTracking.ts
@@ -4,7 +4,8 @@ import { useClient } from 'cozy-client'
 
 import {
   isGeolocationTrackingEnabled,
-  checkShouldStartTracking
+  checkShouldStartTracking,
+  initGeolocationTracking
 } from '/app/domain/geolocation/services/tracking'
 import { checkGeolocationQuota } from '/app/domain/geolocation/helpers/quota'
 import { fetchAndStoreWebhook } from '/app/domain/geolocation/helpers/webhook'
@@ -16,6 +17,8 @@ export const useGeolocationTracking = (): void => {
     const initializeTracking = async (): Promise<void> => {
       if (!client) return
 
+      // The tracking must be init and enabled to be functional
+      await initGeolocationTracking()
       const trackingEnabled = await isGeolocationTrackingEnabled()
 
       if (trackingEnabled) {
diff --git a/src/app/domain/geolocation/services/tracking.ts b/src/app/domain/geolocation/services/tracking.ts
index a570738c5..c660b31ba 100644
--- a/src/app/domain/geolocation/services/tracking.ts
+++ b/src/app/domain/geolocation/services/tracking.ts
@@ -1,4 +1,6 @@
-import BackgroundGeolocation from 'react-native-background-geolocation'
+import BackgroundGeolocation, {
+  State
+} from 'react-native-background-geolocation'
 
 import {
   startTracking,
@@ -8,7 +10,8 @@ import {
   getId,
   updateId,
   stopTrackingAndClearData,
-  getShouldStartTracking
+  getShouldStartTracking,
+  initTracking
 } from '/app/domain/geolocation/tracking'
 import { isGeolocationQuotaExceeded } from '/app/domain/geolocation/helpers/quota'
 
@@ -39,6 +42,10 @@ export const setGeolocationTracking = async (
   }
 }
 
+export const initGeolocationTracking = async (): Promise<State | null> => {
+  return await initTracking()
+}
+
 export const sendGeolocationTrackingLogs = async (
   client?: CozyClient
 ): Promise<void> => {
@@ -56,7 +63,6 @@ interface GeolocationTrackingStatus {
 
 export const isGeolocationTrackingEnabled = async (): Promise<boolean> => {
   const status = await BackgroundGeolocation.getState()
-
   return status.enabled
 }
 
diff --git a/src/app/domain/geolocation/tracking/index.js b/src/app/domain/geolocation/tracking/index.js
index bee34a4a8..8f989e4aa 100644
--- a/src/app/domain/geolocation/tracking/index.js
+++ b/src/app/domain/geolocation/tracking/index.js
@@ -44,11 +44,26 @@ export {
 
 const log = Minilog('📍 Geolocation')
 
-export const startTracking = async () => {
+export const initTracking = async () => {
   try {
     const trackingConfig = await getTrackingConfig()
     Log('Config : ' + JSON.stringify(trackingConfig))
 
+    // Register on activity change
+    BackgroundGeolocation.onActivityChange(async event => {
+      return handleActivityChange(event)
+    })
+
+    // Register on motion change
+    BackgroundGeolocation.onMotionChange(async event => {
+      return handleMotionChange(event)
+    })
+
+    // Register on conectivty change
+    BackgroundGeolocation.onConnectivityChange(async event => {
+      return handleConnectivityChange(event)
+    })
+
     const state = await BackgroundGeolocation.ready({
       // Geolocation Config
       desiredAccuracy: trackingConfig.desiredAccuracy || ACCURACY,
@@ -80,20 +95,39 @@ export const startTracking = async () => {
         smallIcon: 'mipmap/ic_stat_ic_notification'
       }
     })
-    if (!state.enabled) {
+    Log('Init with state : ', JSON.stringify(state))
+    return state
+  } catch (e) {
+    Log('Error on tracking init : ', JSON.stringify(e))
+    log.error(e)
+
+    return null
+  }
+}
+
+export const startTracking = async () => {
+  try {
+    const state = await BackgroundGeolocation.getState()
+    if (!state) {
+      // This case should not happen
+      log.error('Tracking started without init')
+      await initTracking()
+      await BackgroundGeolocation.start()
+      Log('Tracking init and started')
+    } else if (!state.enabled) {
       await BackgroundGeolocation.start()
       Log('Tracking started')
+    } else {
+      Log('Tracking already started')
     }
     await storeData(
       CozyPersistedStorageKeys.ShouldBeTrackingFlagStorageAdress,
       true
     )
-
     return true
   } catch (e) {
     Log('Error on tracking start : ', JSON.stringify(e))
     log.error(e)
-
     return false
   }
 }
@@ -259,17 +293,3 @@ export const startOpenPathUploadAndPipeline = async ({
   }
   return
 }
-
-// Register on activity change
-BackgroundGeolocation.onActivityChange(async event => {
-  return handleActivityChange(event)
-})
-
-// Register on motion change
-BackgroundGeolocation.onMotionChange(async event => {
-  return handleMotionChange(event)
-})
-
-BackgroundGeolocation.onConnectivityChange(async event => {
-  return handleConnectivityChange(event)
-})