Wolox / WLXBluetoothDevice

A block-based wrapper of CoreBluetooth
http://www.wolox.com.ar
MIT License
25 stars 8 forks source link

Timing issue with initialisation of WLXDeviceDiscoverer #47

Open drdaz opened 8 years ago

drdaz commented 8 years ago

I'm seeing an issue with the order of things when trying to discover services. I've got the following code:

self.bluetoothDeviceManager = [WLXBluetoothDeviceManager deviceManager];
NSObject<WLXDeviceDiscoverer> *discoverer = self.bluetoothDeviceManager.discoverer;
discoverer.delegate = self;
[discoverer discoverDevicesNamed:@"MyServiceName" withServices:nil andTimeout:10];

The delegate callbacks are not happening; the discoverDevicesNamed:withServices:andTimeout call is returning NO since bluetoothOn is no at the time of the call. It gets set to YES shortly after the call (by WLXCentralManagerDelegate).

Is this not the recommended way of starting discovery?

drdaz commented 8 years ago

So I added some retry logic to my fork to hack around this. I then made a mess of pull request #46, as I was trying to make a new PR... but instead the change got added to the same one. :-/

guidomb commented 8 years ago

Yeah this happened to me a couple of times when I was prototyping. The thing is that in production code never happened because most of the UI that is in charge is managing the discovery process is not enabled if bluetooth is disabled.

I do understand that this is confusing but I am not 100% this is the "best" approach because we should do the same for all the other operation (like connecting to an already discovered devices that has been persisted)

Regarding the PR #46 could remove that commit because I don't want to merge that change yet I only want to merge the podspec.

You can create a separate PR for the fix and we can discuss this over there. I'm gonna check if there is another option. My recommendation would be to avoid starting the discovery process if the bluetooth is not enabled. Listen to WLXBluetoothDeviceBluetoothPowerStatusChanged or WLXBluetoothDeviceBluetoothIsOn notification and then if the bluetooth is enabled start the discovery process

drdaz commented 8 years ago

Bluetooth is enabled actually (or rather I haven't disabled it anywhere)... This code is thrown right into the viewDidAppear of a UIViewController, since I'm trying out your library for potential use in a project and that's a nice quick entry point. So I'm quite sure the fix approach I took isn't the best; I just needed something to work and allow me to explore further.

It is unfortunate, however, that the code you provide in the wiki to demonstrate discovery (which is almost a carbon copy of what I wrote) won't really work.

About PR #46... I'm not sure how to remove that commit. It was never my intention to include the hack in that PR; I clicked 'Create Pull Request' and it just added it to #46... I wanted to create a separate PR for it. Do you have any idea how to do that on my end? I don't want to lose the changes in my fork for the time being...

drdaz commented 8 years ago

Ok... #46 should be cleaned up now.

rbrignoni commented 8 years ago

A workaround for this that I used was to check if bluetoothOn == NO, subscribe to the WLXBluetoothDeviceBluetoothPowerStatusChanged notification and wait unil it was set to YES, because it could also occur that the user simply had the bluetooth antenna turned off and may turn it back on while you're messaging them/waiting.

guidomb commented 8 years ago

@drdaz Thanks for changing #46. Regarding the problem about the bluetooth being enabled I agree with you that not having a clear example in the wiki that works out of the box is a big issue for new comers. What I think is that maybe the "best" solution would be to update the documentation, but I am not 100% sure.

What is actually happening is that for some reason, no matter if the bluetooth is turned on, after you create a CBCentralManager it takes a second to for the central to be ready to be used. In order to be able to use anything bluetooth related you need to wait until the central is being told that the bluetooth is enabled (which is an async operation). As @rbrignoni said a workaround is to subscribe to WLXBluetoothDeviceBluetoothPowerStatusChanged notification. I could add this to the wiki but, again I am not sure which is the best solution yet and I don't want to introduce a breaking change in the API or make a quick hack with giving it a second thought.

I am 100% open for discussion.

Another option could be to do something like

self.deviceManager = [WLXBluetoothDeviceManager deviceManagerWithInitBlock:^(WLXBluetoothDeviceManager * deviceManager){
    // This block will be called the first time the bluetooth is ready to be used
   id<WLXDeviceDiscoverer> discoverer = deviceManager.discoverer;
   discoverer.delegate = self;
   [discoverer discoverDevicesNamed:@"MyServiceName" withServices:nil andTimeout:10];
}];
drdaz commented 8 years ago

As long as I know that I’m the last one to receive that notification, that’ll work… I mean the only time this would be an issue is on initialisation, right?

On 06 Jan 2016, at 19:34, Guido Marucci Blas notifications@github.com wrote:

@drdaz https://github.com/drdaz Thanks for changing #46 https://github.com/Wolox/WLXBluetoothDevice/pull/46. Regarding the problem about the bluetooth being enabled I agree with you that not having a clear example in the wiki that works out of the box is a big issue for new comers. What I think is that maybe the "best" solution would be to update the documentation, but I am not 100% sure.

What is actually happening is that for some reason, no matter if the bluetooth is turned on, after you create a CBCentralManager it takes a second to for the central to be ready to be used. In order to be able to use anything bluetooth related you need to wait until the central is being told that the bluetooth is enabled (which is an async operation). As @rbrignoni https://github.com/rbrignoni sais a workaround is to subscribe to WLXBluetoothDeviceBluetoothPowerStatusChanged notification. I could add this to the wiki but, again I am not sure which is the best solution yet and I don't want to introduce a breaking change in the API or make a quick hack with giving it a second thought.

I am 100% open for discussion.

Another option could be to do something like

self.deviceManager = [WLXBluetoothDeviceManager deviceManagerWithInitBlock:^(WLXBluetoothDeviceManager * deviceManager){ // This block will be called the first time the bluetooth is ready to be used id discoverer = deviceManager.discoverer; discoverer.delegate = self; [discoverer discoverDevicesNamed:@"MyServiceName" withServices:nil andTimeout:10]; }]; — Reply to this email directly or view it on GitHub https://github.com/Wolox/WLXBluetoothDevice/issues/47#issuecomment-169413057.

guidomb commented 8 years ago

Yeah this is only necessary for the initialization in case you want to do an operation right after you created an instance of device manager.