Update user location directly from foreground service #963
Conversation
|
WalkthroughThis update primarily focuses on enhancing the geolocation tracking functionality within a foreground service. It includes changes to track and send location data at intervals, ensuring data is sent only if the location has significantly changed. The retry logic in the Changes
Uplevel your code reviews with Entelligence.AI ProEntelligence.AI ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, Entelligence.AI Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (3)
- app/lib/pages/memories/widgets/capture.dart (2)
- app/lib/providers/capture_provider.dart (1)
- app/lib/utils/audio/foreground.dart (3)
Files skipped from review due to trivial changes (2)
- app/lib/pages/memories/widgets/capture.dart
- app/lib/providers/capture_provider.dart
Review comments generated (3)
- Review: 3
- LGTM: 0
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
app/lib/utils/audio/foreground.dart
Outdated
| import 'package:friend_private/backend/http/api/users.dart'; | ||
| import 'package:friend_private/backend/schema/geolocation.dart'; |
There was a problem hiding this comment.
The new imports for friend_private/backend/http/api/users.dart and friend_private/backend/schema/geolocation.dart are added correctly. However, ensure that the updateUserGeolocation function from users.dart and Geolocation class from geolocation.dart are properly defined and exported in their respective files.
+ import 'package:friend_private/backend/http/api/users.dart';
+ import 'package:friend_private/backend/schema/geolocation.dart';
app/lib/utils/audio/foreground.dart
Outdated
| DateTime? _locationUpdatedAt; | ||
|
|
||
| @override | ||
| void onStart(DateTime timestamp) async { | ||
| debugPrint("Starting foreground task"); | ||
| } | ||
|
|
||
| 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)))) { | ||
| await updateUserGeolocation( | ||
| geolocation: Geolocation( | ||
| latitude: locationData.latitude, | ||
| longitude: locationData.longitude, | ||
| accuracy: locationData.accuracy, | ||
| altitude: locationData.altitude, | ||
| time: locationData.timestamp.toUtc(), | ||
| ), | ||
| ); | ||
| _locationUpdatedAt = DateTime.now(); | ||
| } |
There was a problem hiding this comment.
The _locationInBackground method has been significantly modified to include a check for when the location was last updated (_locationUpdatedAt). If it's null or more than 5 minutes ago, the user's geolocation is updated using the updateUserGeolocation function. This is a good approach to avoid unnecessary updates if the location hasn't changed significantly. However, there's no error handling for the updateUserGeolocation function call. If this function throws an error (e.g., network issues), it will cause the entire method to fail. Consider adding try-catch blocks around it.
+ DateTime? _locationUpdatedAt;
...
+ Future _locationInBackground() async {
+ if (await Geolocator.isLocationServiceEnabled()) {
+ if (await Geolocator.checkPermission() == LocationPermission.always) {
+ var locationData = await Geolocator.getCurrentPosition();
+ if (_locationUpdatedAt == null ||
+ _locationUpdatedAt!.isBefore(DateTime.now().subtract(const Duration(minutes: 5)))) {
+ try {
+ await updateUserGeolocation(
+ geolocation: Geolocation(
+ latitude: locationData.latitude,
+ longitude: locationData.longitude,
+ accuracy: locationData.accuracy,
+ altitude: locationData.altitude,
+ time: locationData.timestamp.toUtc(),
+ ),
+ );
+ _locationUpdatedAt = DateTime.now();
+ } catch (e) {
+ // Handle error here
+ }
+ }
+ } else {
+ Object loc = {'error': 'Always location permission is not granted'};
+ FlutterForegroundTask.sendDataToMain(loc);
+ }
+ }
+ }| await _locationInBackground(); | ||
| } | ||
|
|
||
| @override | ||
| void onRepeatEvent(DateTime timestamp) async { | ||
| print("Foreground repeat event triggered"); | ||
| await locationInBackground(); | ||
| await _locationInBackground(); |
There was a problem hiding this comment.
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
+ }
+ }
There was a problem hiding this comment.
Files selected (1)
- backend/routers/processing_memories.py (1)
Review comments generated (3)
- Review: 3
- LGTM: 0
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| DetailProcessingMemoriesResponse, UpdateProcessingMemory | ||
| from database.redis_db import cache_user_geolocation | ||
| from utils.other import endpoints as auth |
There was a problem hiding this comment.
The import statement on line 9 seems to be misaligned. It's a minor issue but it's good to keep the code clean and consistent.
- from utils.other import endpoints as auth
+ from utils.other import endpoints as auth| @router.patch("/v1/processing-memories/{processing_memory_id}", response_model=EmptyResponse, | ||
| tags=['processing_memories']) | ||
| def update_processing_memory( | ||
| processing_memory_id: str, | ||
| updates_processing_memory: UpdateProcessingMemory, | ||
| uid: str = Depends(auth.get_current_user_uid) | ||
| ): | ||
| """ | ||
| @Deprecated | ||
| 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}") | ||
|
|
||
| geolocation = updates_processing_memory.geolocation | ||
| if geolocation: | ||
| cache_user_geolocation(uid, geolocation.dict()) | ||
|
|
||
| # @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) | ||
| raise HTTPException(status_code=405, detail="Deprecated") |
There was a problem hiding this comment.
The update_processing_memory function has been deprecated and now raises an HTTPException with status code 405. However, before raising the exception, it caches the user geolocation if it exists. This could lead to unexpected behavior because clients might not expect any operation to be performed when they receive a 405 error. If you want to keep this functionality, consider creating a new endpoint for caching user geolocation or clearly document this behavior in the API documentation.
| geolocation = updates_processing_memory.geolocation | ||
| if geolocation: | ||
| cache_user_geolocation(uid, geolocation.dict()) |
There was a problem hiding this comment.
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())…e moving to background. Set location background repeated interval 5m
There was a problem hiding this comment.
Files selected (6)
- app/lib/env/env.dart (1)
- app/lib/pages/home/page.dart (3)
- app/lib/pages/memories/widgets/capture.dart (2)
- app/lib/services/sockets/pure_socket.dart (1)
- app/lib/utils/audio/foreground.dart (3)
- backend/routers/processing_memories.py (1)
Files skipped from review due to trivial changes (1)
- app/lib/env/env.dart
Review comments generated (10)
- Review: 7
- LGTM: 3
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| @@ -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'; | |||
There was a problem hiding this comment.
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';| 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) { |
There was a problem hiding this comment.
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.
| _retries++; | ||
| if (_retries >= maxRetries) { | ||
| if (_retries > maxRetries) { | ||
| debugPrint("[Socket] Reach max retries $maxRetries"); | ||
| _listener?.onMaxRetriesReach(); | ||
| return; | ||
| } | ||
| int waitInMilliseconds = pow(multiplier, _retries).toInt() * initialBackoffTimeMs; | ||
| await Future.delayed(Duration(milliseconds: waitInMilliseconds)); |
There was a problem hiding this comment.
The retry logic has been improved to correctly calculate the wait time before retries. However, there's a potential issue with this approach. The _retries variable is incremented before calculating waitInMilliseconds. This means that for the first retry, the delay will be multiplier * initialBackoffTimeMs, not just initialBackoffTimeMs. If you want the first retry to happen after initialBackoffTimeMs, you should increment _retries after calculating the delay.
- _retries++;
- int waitInMilliseconds = pow(multiplier, _retries).toInt() * initialBackoffTimeMs;
+ int waitInMilliseconds = pow(multiplier, _retries++).toInt() * initialBackoffTimeMs;
await Future.delayed(Duration(milliseconds: waitInMilliseconds));
- _reconnect();
+ _reconnect();This change ensures that the first retry happens after initialBackoffTimeMs, and subsequent retries follow the exponential backoff strategy.
| 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(), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
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');
}
}| 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(); | ||
| } |
There was a problem hiding this comment.
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);
}
}| // 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, |
There was a problem hiding this comment.
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,| @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") | ||
|
|
||
| # @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())) |
There was a problem hiding this comment.
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()))
There was a problem hiding this comment.
Files selected (1)
- app/lib/services/sockets/pure_socket.dart (2)
Review comments generated (2)
- Review: 2
- LGTM: 0
Tips
Chat with Entelligence.AI Bot (@Entelligence.AI)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@Entelligence.AIin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@Entelligence.AI: ignoreanywhere in the PR description to pause further reviews from the bot.
| @@ -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; | |||
There was a problem hiding this comment.
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 (_retries > maxRetries) { | ||
| debugPrint("[Socket] Reach max retries $maxRetries"); | ||
| _listener?.onMaxRetriesReach(); | ||
| return; |
There was a problem hiding this comment.
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;
Issue: #946
Summary by Entelligence.AI
PureSocketclass for better performance and increased the maximum retries from 7 to 8._onReceiveTaskDatamethod and removed its usage ininitState. Also, deprecated theupdate_processing_memoryfunction and adjusted it to handle geolocation data and cache it accordingly.apiBaseUrlgetter in theEnvclass to ensure accurate API base URL retrieval.