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

after bonding, then call writeCharacteristic, it fail #498

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eaglewangy
Copy link

I use a wrong email in the commit for #479. Now re-commit using the correct email address for CLA @philips77 . Please review and merge to main branch

@eaglewangy
Copy link
Author

@philips77 Is there an ETA?

@philips77
Copy link
Member

The issue with your proposal is that it may work for you case but we have to test it on different Android versions.
According to our knowledge, if one for example call read characteristic which would initiate bonding some Android versions will repeat the request automatically after bonding is successful. Therefore this return; there. The onCharacteristicRead callback would be called with the value and only then the queue would be resumed.

As you see few lines above, for Android versions lower than Oreo we do call break; if there's a ongoing request.
We have not test it for some time now. Google is really good in changing these things without any information.

@philips77
Copy link
Member

But I agree, if you're using ensureBond() the code should look more like that:

if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
   if (request != null) {
	// Repeat the last command in that case.
	enqueueFirst(request);
	break;
   }
}
if (request != null) {
   // Request should be repeated automatically.
   return;
}
break;

@philips77
Copy link
Member

Also, if you're using ensureBond(), the call should finish at line 399:

if (request != null && request.type == Request.Type.CREATE_BOND) {
request.notifySuccess(device);
request = null;
break;
}

Could you post the logs, so that I can trace what's going on there?

@philips77
Copy link
Member

Ok, I did some digging.
When you call ensureBond(), and the device is already bonded, Android will remove bonding and bond again:

final Request bond = Request.createBond().setRequestHandler(this);
// bond.beforeCallback was already fired.
bond.successCallback = request.successCallback;
bond.invalidRequestCallback = request.invalidRequestCallback;
bond.failCallback = request.failCallback;
bond.internalSuccessCallback = request.internalSuccessCallback;
bond.internalFailCallback = request.internalFailCallback;
request.successCallback = null;
request.invalidRequestCallback = null;
request.failCallback = null;
request.internalSuccessCallback = null;
request.internalFailCallback = null;
enqueueFirst(bond);
// This will be added as first.
enqueueFirst(Request.removeBond().setRequestHandler(this));
nextRequest(true);

When removing bond completes (which by the way will disconnect the device), this will be called:

if (request != null && request.type == Request.Type.REMOVE_BOND) {
// The device has already disconnected by now.
log(Log.INFO, () -> "Bond information removed");
request.notifySuccess(device);
request = null;
}

and the CREATE_BOND request will be started.

Then, when bonded, you should get:

if (request != null && request.type == Request.Type.CREATE_BOND) {
request.notifySuccess(device);
request = null;
break;
}

which should also call nextRequest(true) after the switch.

@philips77
Copy link
Member

Logs from log(...) method would be helpful.

@philips77
Copy link
Member

When testing on Pixel 5 with Android 14 the following scenario:

  1. Connect
  2. Read a characteristic which triggers bonding

The phone bonds successfully and I'm getting read result afterwards:

12:09:05.401 BlinkyManagerImpl        V  Reading characteristic 00001524-1212-efde-1523-785feabcd123
12:09:05.435 BlinkyManagerImpl        D  gatt.readCharacteristic(00001524-1212-efde-1523-785feabcd123)
12:09:05.436 BluetoothGatt            D  onConnectionUpdated() - Device=7F:C6:12:4E:9F:70 interval=36 latency=0 timeout=500 status=0
12:09:05.446 BlinkyManagerImpl        I  Connection parameters updated (interval: 45.0ms, latency: 0, timeout: 5000ms)
12:09:07.758 BlinkyManagerImpl        D  [Broadcast] Action received: android.bluetooth.device.action.PAIRING_REQUEST, pairing variant: PAIRING_VARIANT_PASSKEY_CONFIRMATION (2); key: 758840
12:09:07.768 BlinkyManagerImpl        D  [Broadcast] Action received: android.bluetooth.device.action.BOND_STATE_CHANGED, bond state changed to: BOND_BONDING (11)
12:09:07.911 BluetoothGatt            D  onConnectionUpdated() - Device=7F:C6:12:4E:9F:70 interval=6 latency=0 timeout=500 status=0
12:09:07.913 BlinkyManagerImpl        I  Connection parameters updated (interval: 7.5ms, latency: 0, timeout: 5000ms)
12:09:11.532 BlinkyManagerImpl        D  [Broadcast] Action received: android.bluetooth.device.action.BOND_STATE_CHANGED, bond state changed to: BOND_BONDED (12)
12:09:11.539 BlinkyManagerImpl        I  Device bonded
12:09:11.544 BlinkyManagerImpl        I  Read Response received from 00001524-1212-efde-1523-785feabcd123, value: (0x) 01

What phone and Android version are you working with?

@ElectronicSpaceCat
Copy link
Contributor

ElectronicSpaceCat commented Jun 4, 2024

I've had issues similar to this with the following scenario:

  • My peripheral device (nrf52832, SDK_17.1.0) and phone (Android Version 8) clear of stored bonds
  • After first bond using .ensureBond() it could not write (I believe the notification/indications were not getting subscribed to)
  • Connections after the initial bonding would work fine (which I'm using autoConnect)
  • My device uses the .allow_repairing = true and if a bond exists in the whitelist but the bond on the phone was cleared, it would also connect without issues.

What fixed it for me was changing the 2 instances of:

 if (request != null && request.type == Request.Type.CREATE_BOND) { 
 	request.notifySuccess(device); 
 	request = null; 
 	break; 
 } 

to:

 if (request != null && (request.type == Request.Type.CREATE_BOND || request.type == Request.Type.ENSURE_BOND)) { 
 	request.notifySuccess(device); 
 	request = null; 
 	break; 
 } 

@philips77
Copy link
Member

Could you prepare another PR with that change? I think it seems legit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants