Android: enforce 2 second disconnect delay

[jasaw]

Addresses issue #927. This workaround enforces a 2 second delay between connect and disconnect. Android bug report: https://issuetracker.google.com/issues/37121040

[chipweinberger]

the delay should be configurable, i think.

In FlutterBluePlus.setOptions

[jasaw]

Updated the PR. Delay is configurable, defaults to 2s.

[chipweinberger]

thanks for the PR and being receptive to changes

thinking about the api more

it should probably be better as an argument to the disconnect function

and called androidDelay

thoughts?

[jasaw]

it should probably be better as an argument to the disconnect function

I imagine most people wouldn't mess with the default 2s delay unless they want to go through vigorous testing themselves, so hiding it in FlutterBluePlus.setOptions makes more sense.

[chipweinberger]

appreciate your thoughts

I think with a good a good function comment it will be clear why it is there.

but yes, i think it is better as a disconnect argument.

our api already has precendent for this (see requestMtu)

[jasaw]

Yup, done, moved to disconnect argument. 👍

[chipweinberger]

looks good. I'd call it

_ensureAndroidDisconnectionDelay

[chipweinberger]

I'd also simplify this comment since we already have the comment elsewhere

  // record connection time
  if (Platform.isAndroid) {
    FlutterBluePlus._connectTimestamp[remoteId] = DateTime.now();
  }

[chipweinberger]

Also I dont think we need this code. seems unecessary to remove it I think, and makes things more complicated

  if (Platform.isAndroid) {
    // Disconnected, remove connect timestamp
    FlutterBluePlus._connectTimestamp.remove(remoteId);
  }

[chipweinberger]

also "workaround a race condition that leaves connection stranded."

id clarify what "leaves connection stranded." means

"that causes android to silently ignore the disconnection request"?

[chipweinberger]

also, im still not convinced we need both this 2-second delay change + the other PR.

The other PR seems sufficient. Why is it not sufficient?

[jasaw]

_ensureAndroidDisconnectionDelay

Done

simplify this comment since we already have the comment elsewhere

Done

Also I dont think we need this code. seems unecessary to remove it I think, and makes things more complicated

I don't like ever growing memory usage. Yes it's just a small amount of memory, but my app is designed to run for at least 3 months without interruption or restart, and it scans and makes Bluetooth connection while running, all without human intervention. This ever growing memory usage is just going to look exactly like a memory leak when I profile the memory usage.

id clarify what "leaves connection stranded." means

Done.

also, im still not convinced we need both this 2-second delay change + the other PR. The other PR seems sufficient. Why is it not sufficient?

This 2-second delay is to workaround an Android bug that establishes connection even though gatt has been disconnected and closed. On successful connection, FBP's gatt callback function is not called, so we end up with a connection that cannot be killed because there is no gatt. Race condition is between connect and disconnect calls. The connect call merely places the connect request into a queue, so no attempt to establish a connection yet.

The other PR fixes the Android bug that ignores disconnect, but still FBP's gatt callback is still called, so we have the gatt, we can disconnect. Race condition is between disconnect call and Android stack establishing connection.

[chipweinberger]

merged

[chipweinberger]

4015129121bfcb22f68ffbda758e34225e29f9d8

[chipweinberger]

1.32.10