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

Update user location directly from foreground service #963

Merged
merged 4 commits into from
Oct 3, 2024
Merged

Update user location directly from foreground service #963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6850da2
Update user location directly from foreground service
beastoin Oct 3, 2024
81a7445
Route update processing memory with location to updates user's locati…
beastoin Oct 3, 2024
86a4d1e
Revive patch geolocation to API from UI, needs more extra works befor…
beastoin Oct 3, 2024
3008e60
Increase reconnect retries
beastoin Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/lib/env/env.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ abstract class Env {

static String? get mixpanelProjectToken => _instance.mixpanelProjectToken;

// static String? get apiBaseUrl => _instance.apiBaseUrl;
static String? get apiBaseUrl => _instance.apiBaseUrl;

// static String? get apiBaseUrl => 'https://based-hardware-development--backened-dev-api.modal.run/';
static String? get apiBaseUrl => 'https://camel-lucky-reliably.ngrok-free.app/';
//static String? get apiBaseUrl => 'https://camel-lucky-reliably.ngrok-free.app/';
// static String? get apiBaseUrl => 'https://mutual-fun-boar.ngrok-free.app/';

static String? get growthbookApiKey => _instance.growthbookApiKey;
Expand Down
21 changes: 21 additions & 0 deletions app/lib/pages/home/page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ import 'dart:io';

import 'package:cached_network_image/cached_network_image.dart';
import 'package:flutter/material.dart';
import 'package:flutter_foreground_task/flutter_foreground_task.dart';
import 'package:friend_private/backend/http/api/users.dart';
import 'package:friend_private/backend/http/cloud_storage.dart';
import 'package:friend_private/backend/preferences.dart';
import 'package:friend_private/backend/schema/geolocation.dart';
import 'package:friend_private/backend/schema/plugin.dart';
import 'package:friend_private/main.dart';
import 'package:friend_private/pages/capture/connect.dart';
Expand Down Expand Up @@ -124,6 +127,21 @@ class _HomePageState extends State<HomePage> with WidgetsBindingObserver, Ticker
final Map<String, Widget> screensWithRespectToPath = {'/settings': const SettingsPage()};
bool? previousConnection;

void _onReceiveTaskData(dynamic data) async {
debugPrint('_onReceiveTaskData $data');
if (data is! Map<String, dynamic>) return;
if (!(data.containsKey('latitude') && data.containsKey('longitude'))) return;
await updateUserGeolocation(
geolocation: Geolocation(
latitude: data['latitude'],
longitude: data['longitude'],
accuracy: data['accuracy'],
altitude: data['altitude'],
time: DateTime.parse(data['time']).toUtc(),
),
);
}
Comment on lines +130 to +143
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The _onReceiveTaskData method is now asynchronous and updates the user's geolocation. However, there are no error handling mechanisms in place for potential issues that may arise during the execution of updateUserGeolocation. It would be beneficial to wrap this call in a try-catch block to handle any exceptions that might occur.

  void _onReceiveTaskData(dynamic data) async {
    debugPrint('_onReceiveTaskData $data');
    if (data is! Map<String, dynamic>) return;
    if (!(data.containsKey('latitude') && data.containsKey('longitude'))) return;
    try {
      await updateUserGeolocation(
        geolocation: Geolocation(
          latitude: data['latitude'],
          longitude: data['longitude'],
          accuracy: data['accuracy'],
          altitude: data['altitude'],
          time: DateTime.parse(data['time']).toUtc(),
        ),
      );
    } catch (e) {
      // Handle exception here
      debugPrint('Error updating user geolocation: $e');
    }
  }


@override
void initState() {
SharedPreferencesUtil().pageToShowFromNotification = 0; // TODO: whatisit
Expand Down Expand Up @@ -160,6 +178,9 @@ class _HomePageState extends State<HomePage> with WidgetsBindingObserver, Ticker
SharedPreferencesUtil().subPageToShowFromNotification = '';
}
super.initState();

// After init
FlutterForegroundTask.addTaskDataCallback(_onReceiveTaskData);
}

void _listenToMessagesFromNotification() {
Expand Down
21 changes: 1 addition & 20 deletions app/lib/pages/memories/widgets/capture.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter_foreground_task/flutter_foreground_task.dart';
import 'package:flutter_provider_utilities/flutter_provider_utilities.dart';
import 'package:friend_private/backend/http/api/users.dart';
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The new import statement for the users API is added, but it doesn't seem to be used anywhere in the provided code. If it's not used elsewhere in the file, consider removing this import to keep the code clean and maintainable.

- import 'package:friend_private/backend/http/api/users.dart';

import 'package:friend_private/backend/schema/bt_device/bt_device.dart';
import 'package:friend_private/backend/schema/geolocation.dart';
import 'package:friend_private/pages/capture/widgets/widgets.dart';
Expand Down Expand Up @@ -30,29 +31,9 @@ class LiteCaptureWidgetState extends State<LiteCaptureWidget>
context.read<CaptureProvider>().setHasTranscripts(hasTranscripts);
}

void _onReceiveTaskData(dynamic data) {
print('_onReceiveTaskData $data');
if (data is! Map<String, dynamic>) return;
if (!(data.containsKey('latitude') && data.containsKey('longitude'))) return;
if (mounted) {
// TODO: why do we need this?
context.read<CaptureProvider>().setGeolocation(
Geolocation(
latitude: data['latitude'],
longitude: data['longitude'],
accuracy: data['accuracy'],
altitude: data['altitude'],
time: DateTime.parse(data['time']),
),
);
}
}

@override
void initState() {
WavBytesUtil.clearTempWavFiles();

FlutterForegroundTask.addTaskDataCallback(_onReceiveTaskData);
WidgetsBinding.instance.addObserver(this);
SchedulerBinding.instance.addPostFrameCallback((_) async {
if (context.read<DeviceProvider>().connectedDevice != null) {
Comment on lines 31 to 39
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The _onReceiveTaskData method has been removed. This method was previously responsible for updating the user's geolocation. Ensure that this functionality is adequately replaced elsewhere in your application, as it seems crucial for the app's operation. If the location update is now handled by a foreground service, make sure the service is correctly implemented and started when needed. Without seeing the rest of the code, it's hard to say if this change is appropriate or not.

Expand Down
5 changes: 0 additions & 5 deletions app/lib/providers/capture_provider.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ class CaptureProvider extends ChangeNotifier
notifyListeners();
}

void setGeolocation(Geolocation? value) async {
if (value == null) return;
await updateUserGeolocation(geolocation: value);
}

void setAudioBytesConnected(bool value) {
audioBytesConnected = value;
notifyListeners();
Expand Down
4 changes: 2 additions & 2 deletions app/lib/services/sockets/pure_socket.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ class PureSocket implements IPureSocket {
debugPrint("[Socket] reconnect...${_retries + 1}...");
const int initialBackoffTimeMs = 1000; // 1 second
const double multiplier = 1.5;
const int maxRetries = 7;
const int maxRetries = 8;
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The maximum retries have been increased from 7 to 8. This change seems fine as long as it's in line with the application's tolerance for retry attempts. However, keep in mind that increasing the number of retries will also increase the time before the system gives up trying to reconnect, which could potentially lead to longer periods of unresponsiveness in case of persistent connection issues.

-    const int maxRetries = 7;
+    const int maxRetries = 8;


if (_status == PureSocketStatus.connecting || _status == PureSocketStatus.connected) {
debugPrint("[Socket] Can not reconnect, because socket is $_status");
Expand All @@ -221,7 +221,7 @@ class PureSocket implements IPureSocket {
int waitInMilliseconds = pow(multiplier, _retries).toInt() * initialBackoffTimeMs;
await Future.delayed(Duration(milliseconds: waitInMilliseconds));
_retries++;
if (_retries >= maxRetries) {
if (_retries > maxRetries) {
debugPrint("[Socket] Reach max retries $maxRetries");
_listener?.onMaxRetriesReach();
return;
Comment on lines +224 to 227
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The condition for checking if retries exceed the maximum limit has been changed from >= to >. This effectively increases the total number of retries by one more than the maxRetries value. If this is intended, then the code is correct. However, if you want the number of retries to be exactly equal to maxRetries, you should revert this change.

-    if (_retries >= maxRetries) {
+    if (_retries > maxRetries) {
       debugPrint("[Socket] Reach max retries $maxRetries");
       _listener?.onMaxRetriesReach();
       return;

Expand Down
34 changes: 21 additions & 13 deletions app/lib/utils/audio/foreground.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,30 @@ void _startForegroundCallback() {
}

class _ForegroundFirstTaskHandler extends TaskHandler {
DateTime? _locationUpdatedAt;

@override
void onStart(DateTime timestamp) async {
debugPrint("Starting foreground task");
_locationInBackground();
}

Future locationInBackground() async {
Future _locationInBackground() async {
if (await Geolocator.isLocationServiceEnabled()) {
if (await Geolocator.checkPermission() == LocationPermission.always) {
var locationData = await Geolocator.getCurrentPosition();
Object loc = {
"latitude": locationData.latitude,
"longitude": locationData.longitude,
'altitude': locationData.altitude,
'accuracy': locationData.accuracy,
'time': locationData.timestamp.toUtc().toIso8601String(),
};
FlutterForegroundTask.sendDataToMain(loc);
if (_locationUpdatedAt == null ||
_locationUpdatedAt!.isBefore(DateTime.now().subtract(const Duration(minutes: 5)))) {
Object loc = {
"latitude": locationData.latitude,
"longitude": locationData.longitude,
'altitude': locationData.altitude,
'accuracy': locationData.accuracy,
'time': locationData.timestamp.toUtc().toIso8601String(),
};
FlutterForegroundTask.sendDataToMain(loc);
_locationUpdatedAt = DateTime.now();
}
Comment on lines +13 to +36
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The logic for checking and updating the location is correct. However, it's important to handle potential exceptions that might occur during the process of getting the current position. For instance, Geolocator.getCurrentPosition() can throw a LocationServiceDisabledException if location services are disabled on the device or a PermissionDeniedException if location permissions are denied. These exceptions should be caught and handled appropriately.

     if (await Geolocator.isLocationServiceEnabled()) {
       if (await Geolocator.checkPermission() == LocationPermission.always) {
         try {
           var locationData = await Geolocator.getCurrentPosition();
           if (_locationUpdatedAt == null ||
               _locationUpdatedAt!.isBefore(DateTime.now().subtract(const Duration(minutes: 5)))) {
             Object loc = {
               "latitude": locationData.latitude,
               "longitude": locationData.longitude,
               'altitude': locationData.altitude,
               'accuracy': locationData.accuracy,
               'time': locationData.timestamp.toUtc().toIso8601String(),
             };
             FlutterForegroundTask.sendDataToMain(loc);
             _locationUpdatedAt = DateTime.now();
           }
         } catch (e) {
           // Handle exception here
         }
       } else {
         Object loc = {'error': 'Always location permission is not granted'};
         FlutterForegroundTask.sendDataToMain(loc);
       }
     }

} else {
Object loc = {'error': 'Always location permission is not granted'};
FlutterForegroundTask.sendDataToMain(loc);
Expand All @@ -40,13 +47,13 @@ class _ForegroundFirstTaskHandler extends TaskHandler {
@override
void onReceiveData(Object data) async {
debugPrint('onReceiveData: $data');
await locationInBackground();
await _locationInBackground();
}

@override
void onRepeatEvent(DateTime timestamp) async {
print("Foreground repeat event triggered");
await locationInBackground();
await _locationInBackground();
Comment on lines +50 to +56
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The onReceiveData and onRepeatEvent methods now call the _locationInBackground method. This seems fine, but remember to handle any potential exceptions that might be thrown by _locationInBackground.

+ void onReceiveData(Object data) async {
+   debugPrint('onReceiveData: $data');
+   try {
+     await _locationInBackground();
+   } catch (e) {
+     // Handle error here
+   }
+ }
...
+ void onRepeatEvent(DateTime timestamp) async {
+   print("Foreground repeat event triggered");
+   try {
+     await _locationInBackground();
+   } catch (e) {
+     // Handle error here
+   }
+ }

}

@override
Expand Down Expand Up @@ -101,8 +108,9 @@ class ForegroundUtil {
playSound: false,
),
foregroundTaskOptions: const ForegroundTaskOptions(
// TODO: should we do this less frequently?
interval: 30000,
// Warn: 5m, for location tracking. If we want to support other services, we use the differenct interval,
// such as 1m + self-validation in each service.
interval: 60 * 1000 * 5,
Comment on lines +111 to +113
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The comment on line 111 is a bit unclear. It seems like you're trying to explain why you've chosen a 5-minute interval for location tracking, but the explanation isn't clear. Consider revising this comment to more clearly explain your reasoning.

        // Note: The interval is set to 5 minutes for location tracking. If other services require different intervals,
        // they should implement their own validation logic.
        interval: 60 * 1000 * 5,

isOnceEvent: false,
autoRunOnBoot: false,
allowWakeLock: true,
Expand Down
53 changes: 29 additions & 24 deletions backend/routers/processing_memories.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,40 @@

import utils.processing_memories as processing_memory_utils
from models.processing_memory import DetailProcessingMemoryResponse, \
DetailProcessingMemoriesResponse
DetailProcessingMemoriesResponse, UpdateProcessingMemory, UpdateProcessingMemoryResponse, BasicProcessingMemory
from database.redis_db import cache_user_geolocation
from utils.other import endpoints as auth

router = APIRouter()

# Deprecated
@router.patch("/v1/processing-memories/{processing_memory_id}", response_model=UpdateProcessingMemoryResponse,
tags=['processing_memories'])
def update_processing_memory(
processing_memory_id: str,
updates_processing_memory: UpdateProcessingMemory,
uid: str = Depends(auth.get_current_user_uid)
):
"""
Update ProcessingMemory endpoint.
:param processing_memory_id:
:param updates_processing_memory: data to update processing_memory
:param uid: user id.
:return: The new processing_memory updated.
"""

print(f"Update processing memory {processing_memory_id}")

# Keep up-to-date with the new logic
geolocation = updates_processing_memory.geolocation
if geolocation:
cache_user_geolocation(uid, geolocation.dict())
Comment on lines +32 to +34
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

There is no null check for uid before using it in cache_user_geolocation. If uid is None, this will raise an exception. Consider adding a null check for uid.

-    geolocation = updates_processing_memory.geolocation
-    if geolocation:
-        cache_user_geolocation(uid, geolocation.dict())
+    if uid:
+        geolocation = updates_processing_memory.geolocation
+        if geolocation:
+            cache_user_geolocation(uid, geolocation.dict())


processing_memory = processing_memory_utils.get_processing_memory(uid, updates_processing_memory.id)
if not processing_memory:
raise HTTPException(status_code=404, detail="Processing memory not found")

# @router.patch("/v1/processing-memories/{processing_memory_id}", response_model=UpdateProcessingMemoryResponse,
# tags=['processing_memories'])
# def update_processing_memory(
# processing_memory_id: str,
# updates_processing_memory: UpdateProcessingMemory,
# uid: str = Depends(auth.get_current_user_uid)
# ):
# """
# Update ProcessingMemory endpoint.
# :param processing_memory_id:
# :param updates_processing_memory: data to update processing_memory
# :param uid: user id.
# :return: The new processing_memory updated.
# """
#
# print(f"Update processing memory {processing_memory_id}")
#
# updates_processing_memory.id = processing_memory_id
# processing_memory = processing_memory_utils.update_basic_processing_memory(uid, updates_processing_memory)
# if not processing_memory:
# raise HTTPException(status_code=404, detail="Processing memory not found")
#
# return UpdateProcessingMemoryResponse(result=processing_memory)
return UpdateProcessingMemoryResponse(result=BasicProcessingMemory(**processing_memory.dict()))
Comment on lines +14 to +40
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The update_processing_memory function is marked as deprecated but it's still being used. If this function is deprecated, it should not be used in the codebase and should be removed or replaced with a new implementation. Also, there is no error handling for when cache_user_geolocation(uid, geolocation.dict()) fails. It would be better to handle potential exceptions that could be raised by this function.

- # Deprecated
- @router.patch("/v1/processing-memories/{processing_memory_id}", response_model=UpdateProcessingMemoryResponse,
-               tags=['processing_memories'])
- def update_processing_memory(
-         processing_memory_id: str,
-         updates_processing_memory: UpdateProcessingMemory,
-         uid: str = Depends(auth.get_current_user_uid)
- ):
-     """
-     Update ProcessingMemory endpoint.
-     :param processing_memory_id:
-     :param updates_processing_memory: data to update processing_memory
-     :param uid: user id.
-     :return: The new processing_memory updated.
-     """
- 
-     print(f"Update processing memory {processing_memory_id}")
- 
-     # Keep up-to-date with the new logic
-     geolocation = updates_processing_memory.geolocation
-     if geolocation:
-         cache_user_geolocation(uid, geolocation.dict())
- 
-     processing_memory = processing_memory_utils.get_processing_memory(uid, updates_processing_memory.id)
-     if not processing_memory:
-         raise HTTPException(status_code=404, detail="Processing memory not found")
-  
-     return UpdateProcessingMemoryResponse(result=BasicProcessingMemory(**processing_memory.dict()))
+ # New implementation of update_processing_memory
+ @router.patch("/v2/processing-memories/{processing_memory_id}", response_model=UpdateProcessingMemoryResponse,
+               tags=['processing_memories'])
+ def update_processing_memory(
+         processing_memory_id: str,
+         updates_processing_memory: UpdateProcessingMemory,
+         uid: str = Depends(auth.get_current_user_uid)
+ ):
+     """
+     Update ProcessingMemory endpoint.
+     :param processing_memory_id:
+     :param updates_processing_memory: data to update processing_memory
+     :param uid: user id.
+     :return: The new processing_memory updated.
+     """
+ 
+     print(f"Update processing memory {processing_memory_id}")
+ 
+     # Keep up-to-date with the new logic
+     geolocation = updates_processing_memory.geolocation
+     if geolocation:
+         try:
+             cache_user_geolocation(uid, geolocation.dict())
+         except Exception as e:
+             raise HTTPException(status_code=500, detail=str(e))
+ 
+     processing_memory = processing_memory_utils.get_processing_memory(uid, updates_processing_memory.id)
+     if not processing_memory:
+         raise HTTPException(status_code=404, detail="Processing memory not found")
+  
+     return UpdateProcessingMemoryResponse(result=BasicProcessingMemory(**processing_memory.dict()))



@router.get(
Expand Down
Loading