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.

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

Use retired 1s to prevent race condition #846

Merged
merged 1 commit into from
Sep 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion app/lib/providers/device_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption
setConnectedDevice(null);
setIsConnected(false);
updateConnectingStatus(false);
periodicConnect('coming from onDisconnect');
await captureProvider?.resetState(restartBytesProcessing: false);
captureProvider?.setAudioBytesConnected(false);
print('after resetState inside initiateConnectionListener');
Expand All @@ -211,6 +210,11 @@ class DeviceProvider extends ChangeNotifier implements IDeviceServiceSubsciption
);
});
MixpanelManager().deviceDisconnected();

// Retired 1s to prevent the race condition made by standby power of ble device
Future.delayed(const Duration(seconds: 1), () {
periodicConnect('coming from onDisconnect');
});
}

void onDeviceReconnected(BTDeviceStruct device) async {
Expand Down
4 changes: 1 addition & 3 deletions app/lib/services/devices.dart
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,7 @@ class DeviceService implements IDeviceService {
mutex = true;

try {
if (_connection != null && _connection?.device.id != deviceId) {
throw Exception("There is a connected device ${_connection?.device.id}, you have to disconnect it first.");
}
debugPrint("ensureConnection ${_connection?.device.id} ${_connection?.status}");
if (_connection?.status == DeviceConnectionState.connected) {
var pongAt = _connection?.pongAt;
var shouldPing = (pongAt == null || pongAt.isBefore(DateTime.now().subtract(const Duration(seconds: 5))));
Comment on lines +212 to 215

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 removal of the check for a different connected device could lead to unexpected behavior. If a connection attempt is made with a different device ID while another device is already connected, the new connection will be attempted without disconnecting the previous one. This could potentially lead to multiple devices being connected at the same time, which might not be the intended behavior.

To fix this, you should reintroduce the check and throw an exception if a different device is already connected. Here's how you can do it:

+      if (_connection != null && _connection?.device.id != deviceId) {
+        throw Exception("There is a connected device ${_connection?.device.id}, you have to disconnect it first.");
+      }
       debugPrint("ensureConnection ${_connection?.device.id} ${_connection?.status}");
       if (_connection?.status == DeviceConnectionState.connected) {
         var pongAt = _connection?.pongAt;
         var shouldPing = (pongAt == null || pongAt.isBefore(DateTime.now().subtract(const Duration(seconds: 5))));

This way, we ensure that only one device is connected at any given time, maintaining the integrity of the connection process.

Expand Down
Loading