troystribling / BlueCap

iOS Bluetooth LE framework
MIT License
714 stars 114 forks source link

Discovered Services and Characteristics are removed on new search #75

Closed CImrie closed 6 years ago

CImrie commented 6 years ago

Hey!

When I am reading/writing to characteristics on my BLE device, I configure the device and then start getting notified for updates to a particular characteristic (so that I can monitor a button press).

Alongside this I regularly poll the battery level and signal strength of the peripheral but whenever I try to discover new services/and or characteristics, the previous services and characteristics are removed and forgotten about. This means that when the button is pressed I see the log message come through from CoreBluetooth that a characteristic was updated, but it can't find which peripheral the characteristic belongs to.

I can see references to discoveredServices.removeAll() and discoveredCharacteristics.removeAll() in a few places.

Is there a reason that services and characteristics are removed from the Peripheral when you try to discover new ones? I'm happy to submit a PR to resolve this if desired but wanted to check beforehand.

Thanks!

troystribling commented 6 years ago

I thought the implementation to remove discovered peripherals the simplest since keeping them around can result in a leak of CBPeripherals that can crash CoreBluetooth. It was problem I had with apps that ran discovery in areas with a lot of Bluetooth devices.

You can save Peripherals in your app if you want them to live through multiple discoveries.

This would also be a big change in the default behavior. I am reluctant to make the change because of impact to current users.

CImrie commented 6 years ago

@troystribling Thanks for getting back to me. I was more wondering about the discovered services and characteristics. When I connect to a peripheral I'm able to maintain a connection. However, when I subscribe to a characteristic so that it notifies me on button presses (for example), if I try to discover more characteristics at that point it forgets about the original characteristic and can't find the correct handler for the button press.

I've just tried and It's not possible to force it to remember the characteristic. The function in Peripheral.swift looks like this:

    internal func didUpdateValueForCharacteristic(_ characteristic: CBCharacteristicInjectable, error: Error?) {
        guard let bcCharacteristic = characteristicWithCBCharacteristic(characteristic) else {
            Logger.debug("characteristic not found uuid=\(characteristic.uuid.uuidString)")
            return
        }
        Logger.debug("uuid=\(characteristic.uuid.uuidString), name=\(bcCharacteristic.name)")
        bcCharacteristic.didUpdate(error)
    }

Note that it calls characteristicWithCBCharacteristic to fetch the characteristic relating to the update

This appears to be due to the following code:

Peripheral.swift:

internal func didDiscoverServices(_ services: [CBServiceInjectable], error: Error?) {
        Logger.debug("peripheral name=\(self.name), peripheral uuid=\(identifier.uuidString), service count \(discoveredServices.count)")
        discoveredServices.removeAll()        <--------------
        if let error = error {
         ...
}

and: Service.swift

internal func didDiscoverCharacteristics(_ characteristics: [CBCharacteristicInjectable], error: Swift.Error?) {
        guard peripheral != nil else {
            return
        }
        discoveredCharacteristics.removeAll()      <--------------
      ...
}

-- basically anywhere that it discovers a service or characteristic, it seems to forcefully forget about any previously found. This includes ones that you're currently listening for changes on.

Is this intended behaviour?

troystribling commented 6 years ago

This is the designed behavior which has the purpose of aggressively managing CBPeripheral resources. The way to save a discovered peripheral is to use two CBCentralManagers. One runs discovery and the other manages connections. When a peripheral that you are interested in is discovered you fetch the CBPeripheral from the CoreBluetooth Service using the provided methods. This is the approach used in the example BlueCap app.

CImrie commented 6 years ago

@troystribling I think we may both be misunderstanding each other a little. I am specifically talking about characteristics being forgotten about while actively connected to a single peripheral. The peripheral is not cleared from memory at any point. I am not scanning for new devices but simply calling discoverCharacteristics on the peripheral object that I have.

Does that make sense or am I fundamentally misunderstanding something here?

troystribling commented 6 years ago

I do not understand why you would want to run discovery on a peripheral that you have already discovered. Do your peripheral services and characteristics change with time?

CImrie commented 6 years ago

When you are scanning for a service/characteristic you can provide the UUID of only that service/characteristic combo. I do this to fetch the ones I need only when they're needed.

Having said that I subscribe to button presses then monitor signal strength (which is separate from characteristics of course) and the battery life characteristic - it polls it on an interval. It's when the first battery life is checked that it forgets what the button press should do (because it has refreshed and found only the battery life characteristic). I'm just not sure why you would ever want to forget about a previously found characteristic - or at least have the option of not forgetting them?

Of course I could scan for all of the services/characteristics I'll ever need up front but that ruins the abstraction I have for managing each peripheral depending on it's functionality (supporting more than one device for the app).

troystribling commented 6 years ago

I still think using different centrals is a better approach. The centrals would run discovery for different services/characteristics. It maps better to your abstraction and is better isolated. I have also seen this used in other apps.

I do not want to change the default behavior of deleting everything prior to a discovery since this is the way it has always been. Changing this could cause issues for other people using the framework and up till now know one has asked to change it.

CImrie commented 6 years ago

I can understand not changing it if it affects others. How would you use different centrals to fix the issue?

Essentially this is what happens in my code each time I am trying to use a characteristic:

self.peripheral.discoverServices([someServiceUUID]).andThen {
    // ... get correct 'service' out of peripheral services
   // now find characteristic on the service
    service.discoverCharacteristics([someCharacteristicUUID]).andThen {
            for characteristic in service.characteristics(withUUID: characteristic) ?? [Characteristic]() {
               // characteristic.doSomething...
            }
    }
}

The process is:

The only solution I can think of at the moment is just discovering all of the characteristics and services I need at once, but it's not efficient if I don't end up needing them and I've used the BT radio to discover them unnecessarily. Otherwise I can't rely on them being there.

troystribling commented 6 years ago

The BlueCap example app uses separate CentralManagers for scanning and discovery. https://github.com/troystribling/BlueCap/tree/master/Examples/BlueCap

They are defined here.

https://github.com/troystribling/BlueCap/blob/master/Examples/BlueCap/BlueCap/AppDelegate.swift#L60-L68

Here is the code where the discovery CentralManager fetches a discovered peripheral to manage.

https://github.com/troystribling/BlueCap/blob/master/Examples/BlueCap/BlueCap/Central/PeripheralsViewController.swift#L472-L493

This line does re fetch.

        guard Singletons.discoveryManager.retrievePeripherals(withIdentifiers: [peripheral.identifier]).first != nil else {
            Logger.debug("Discovered peripheral not found")
            return
        }

You can also use the scanning Central to monitor RSSI.