chipweinberger / flutter_blue_plus

Flutter plugin for connecting and communicationg with Bluetooth Low Energy devices, on Android, iOS, macOS
Other
735 stars 446 forks source link

[Feature]: cancelWhenDisconnected function for connectionState listener #758

Closed spehj closed 8 months ago

spehj commented 8 months ago

FlutterBluePlus Version

1.31.8

Flutter Version

3.16.5

What OS?

All

OS Version

Android 13

Bluetooth Module

Laird BL652-SA

What is your feature request?

Would it be possible to modify a cancelWhenDisconnected to let the last BluetoothConnectionState.disconnected be emitted before the subscription is canceled?

It would be useful to track the connection state for each device, do something when the device is disconnected, and then clear the subscription.

Here is a simple example for illustration:

// listen for disconnection
var subscription = device.connectionState.listen((BluetoothConnectionState state) async {
        print("$state");
// Right now it prints BluetoothConnectionState.disconnected, then BluetoothConnectionState.connected on connect. 
// It doesn't print anything on disconnect, since the subscription is probably canceled before the last connection state is emitted.
});

// ADDED this: Cancel subscription when state is disconnected
device.cancelWhenDisconnected(subscription, next:true);

// Connect to the device
await device.connect();

// Disconnect from device
await device.disconnect();

// cancel to prevent duplicate listeners
subscription.cancel();

Logs

No logs
chipweinberger commented 8 months ago

ive thought about this before.

feel free to open a PR. it might be a tricky change though.

edit: actually i think you just need to move this line _deviceSubscriptions[remoteId]?.forEach((s) => s.cancel()); // cancel subscriptions after this line _methodStream.add(call);

you can also just cancel the subscription in your own connectionState handler and not use cancelWhenDisconnected at all.

spehj commented 8 months ago

Yes, if _methodStream.add(call); was moved I think would probably save this case.

For now, I will just cancel the subscription inside the connectionState listener.

Minimal example for anyone looking for this (note that when you connect for the first time, it emits disconnected value).


bool isConnected = false; // Use this to not cancel the stream if not yet connected
StreamSubscription<BluetoothConnectionState>? subscription;

// listen for disconnection
subscription = device.connectionState.listen((BluetoothConnectionState state) async {
        print("$state");
        // Check if the state is 'disconnected'
    if (!isConnected && state == BluetoothConnectionState.disconnected) {
        // Don't do anything before BluetoothConnectionState.connected
    } else if (state == BluetoothConnectionState.disconnected){
        // Cancel the subscription if it's 'disconnected'
        await subscription?.cancel();
        subscription = null; // Clear the reference
        print("Disconnected and subscription canceled");
    } else if (state == BluetoothConnectionState.connected) {
        bool isConnected = true;
    }

});

// Don't need this
// device.cancelWhenDisconnected(subscription, next:true);

// Connect to the device
await device.connect();

// Disconnect from device
await device.disconnect();
chipweinberger commented 8 months ago

If you don't mind, it would be great if you could open a PR and test the change. It would make your code simpler, and be a good change.

spehj commented 8 months ago

If you don't mind, it would be great if you could open a PR and test the change. It would make your code simpler, and be a good change.

You mean by moving the _methodStream.add(call);? I'll find some time to do this tomorrow or over the weekend.

chipweinberger commented 8 months ago

No, don't move _methodStream.add(call);.

Move _deviceSubscriptions[remoteId]?.forEach((s) => s.cancel()); // cancel subscriptions to the bottom.

spehj commented 8 months ago

Yeah, you're right, I'll do it.

chipweinberger commented 8 months ago

i.e.

.. other code ..

_methodStream.add(call);

// cancel subscriptions after pushing to the stream
if (call.method == "OnConnectionStateChanged") {
  BmConnectionStateResponse r = BmConnectionStateResponse.fromMap(call.arguments);
  if (r.connectionState == BmConnectionStateEnum.disconnected) {
    _deviceSubscriptions[remoteId]?.forEach((s) => s.cancel()); // cancel subscriptions
  }
}

please test the change to make sure it fixes your problem.

spehj commented 8 months ago

Thanks, I'll test it with my BLE device and let you know.

spehj commented 8 months ago

I figured out that a await Future.delayed(Duration.zero); before canceling and removing a subscription is needed. Otherwise, subscription is removed before state is emitted.


// keep track of connection states
    if (call.method == "OnConnectionStateChanged") {

      BmConnectionStateResponse r = BmConnectionStateResponse.fromMap(call.arguments);
      var remoteId = DeviceIdentifier(r.remoteId);
      _connectionStates[remoteId] = r;
      if (r.connectionState == BmConnectionStateEnum.disconnected) {
        // to make FBP easier to use, we purposely do not clear knownServices,
        // otherwise `servicesList` would be annoying to use.
        // We also don't clear the `bondState` cache for faster performance.

        // Removed two lines here

        _mtuValues.remove(remoteId); // reset known mtu
        _lastDescs.remove(remoteId); // clear lastDescs so that 'isNotifying' is reset
        _lastChrs.remove(remoteId); // for api consistency, clear characteristic values

        // On apple, autoconnect is just a long running connection attempt
        // so the connection request must be restored after disconnection
        for (DeviceIdentifier d in _autoConnect) {
          if (Platform.isIOS || Platform.isMacOS) {
            if (_adapterStateNow == BmAdapterStateEnum.on) {
              BluetoothDevice(remoteId: d).connect(autoConnect: true, mtu: null);
            }
          }
        }
      }
    }

// Other code

_methodStream.add(call);

    // cancel subscriptions after pushing to the stream
    if (call.method == "OnConnectionStateChanged") {
      BmConnectionStateResponse r = BmConnectionStateResponse.fromMap(call.arguments);
      var remoteId = DeviceIdentifier(r.remoteId);
      if (r.connectionState == BmConnectionStateEnum.disconnected) {
       await Future.delayed(Duration.zero);// Added this to first emitt a stream before canceling subscription
        _deviceSubscriptions[remoteId]?.forEach((s) => s.cancel()); // cancel subscriptions
        _deviceSubscriptions.remove(remoteId); // delete subscriptions
      }
    }

I opened a PR.

chipweinberger commented 8 months ago

thanks for opening that PR!

hmm. That await Future.delayed(Duration.zero) is not ideal and could cause other timing bugs.

We should put this behind a flag.

// - [delayed] This option should only be used for canceling `connectionState` streams. 
//    If true, we delay canceling until after the `connectionState` stream has been updated.
void cancelWhenDisconnected(delayed: true);

of course then we need a new variable to hold these

static final Map<DeviceIdentifier, List<StreamSubscription>> _delayedSubscriptions = {}

also, I would use then instead of await

Future.delayed(Duration.zero).then(() {
        _delayedSubscriptions[remoteId]?.forEach((s) => s.cancel()); // cancel subscriptions
        _delayedSubscriptions.remove(remoteId); // delete subscriptions
}) 
spehj commented 8 months ago

I'm very busy these days, but I'll try to implement this feature ASAP.

Do you have any idea which timing bugs may appear because of this delay?

I agree with your proposal. We should keep the _deviceSubscription for "normal" subscriptions (should be removed in the first IF statement as it was before) and add _delayedSubscriptions used for the connectionState listener (removed in the second IF statement after the delay).

chipweinberger commented 8 months ago

i pushed the change to master branch.

point your pubspec.yaml to master and let me know if it works for you. thanks.

dependencies:
  flutter_blue_plus:
    git:
      url: https://github.com/boskokg/flutter_blue_plus
      ref: master
spehj commented 8 months ago

I can confirm this works perfectly, thank you!

chipweinberger commented 8 months ago

great! will be in next release

chipweinberger commented 8 months ago

added in 1.31.9