DrAma999 / LittleBlueTooth

A simple library that helps you in connecting with BLE devices
MIT License
79 stars 17 forks source link

Using startListen on more than 1 characteristic at once fails with characteristicNotFound #23

Closed NeverwinterMoon closed 3 years ago

NeverwinterMoon commented 3 years ago

I have something like this

func getSpeed() -> AnyPublisher<SpeedState, LittleBluetoothError> {
    littleBT.startListen(from: Self.SPEED_CHARACTERISTIC)
      .prepend(littleBT.read(from: Self.SPEED_CHARACTERISTIC))
      .handleEvents(receiveCompletion: { completion in
        switch completion {
          case .finished:
            break

          case .failure(let error):
            log.error("SPEED_CHARACTERISTIC read/listen failed with error: \(error)")
        }
      })
      .eraseToAnyPublisher()
  }

  func getBattery() -> AnyPublisher<BatteryState, LittleBluetoothError> {
    littleBT.startListen(from: Self.BATTERY_CHARACTERISTIC)
      .prepend(littleBT.read(from: Self.BATTERY_CHARACTERISTIC))
      .handleEvents(receiveCompletion: { completion in
        switch completion {
          case .finished:
            break

          case .failure(let error):
            log.error("BATTERY_CHARACTERISTIC read/listen failed with error: \(error)")
        }
      })
      .eraseToAnyPublisher()
  }

If I subscribe to just one of these (no matter which one) at once, it works just fine. If, on the other hand, I do something like

Publishers.CombineLatest(
          bikeRepository.getSpeed(),
          bikeRepository.getBattery()
)

And subscribe to that, one of the subscriptions will immediately fail with characteristicNotFound

I was thinking of using the flow similar to what kable does (https://github.com/JuulLabs/kable). There, you can easily do:

   override fun getBattery(): Flow<Metric> = peripheral
        .observe(BATTERY_CHARACTERISTIC)
        .map { it.asBatteryMetric() }

    override fun getSpeed(): Flow<Metric> = peripheral
        .observe(SPEED_CHARACTERISTIC)
        .map { it.asSpeedMetric() }

And then

override fun getMetrics(): Flow<List<Metric>> = combine(
        getSpeed(),
        getBattery
    ) { speed, battery ->
      // do something, like create a model containing all the metrics
    }

This library actually has iOS support through KMM, so this is what observe does: https://github.com/JuulLabs/kable/blob/main/core/src/appleMain/kotlin/Peripheral.kt

DrAma999 commented 3 years ago

I will check this weekend and get back to you. Thank you for the example.

DrAma999 commented 3 years ago

Hello @NeverwinterMoon about this issue. First let me try to explain the error characteristicNotFound(_:). Usually is thrown when a specific characteristic identifier is not found in the service. Since the methods taken separately are working it means that no error is made while building the LittleBlueToothCharacteristic. If tried to replicate your error by using the mocking library that I use to run the tests and it simply works, test is run successfully combining latest values from the characteristics. I do agree that is not a real word scenario so I tried to use LightBlue application to simulate a peripheral and I've found the issue. I really don’t know how KMM is working by creating coroutines and the implementation they are using ,since they are still not available(yet) in swift, but the problem is most probably due to the nature of "cold" publisher of combine's publishers of most of combine operators. What is happening is that here we have 2 publishers (that are subscribed in parallel) that are "listening" to changes for characteristic discoveries. The first command asks to search for the first characteristic, but the second is already subscribed. So while the first successfully get the discovery the second throws an error because it has search for another characteristic but it tries to find a match with the first trigger. I hope to find a solution soon.

Best, Andrea

NeverwinterMoon commented 3 years ago

Thanks for looking into it.

As I understand, that the problem is tied to the discovery of the characteristics only. In such a case, why not introduce the discovery for multiple characteristics at once? The discoverCharacteristics(_:for:) method accepts a list of characteristics and peripheral(_:didDiscoverCharacteristicsFor:error:) is called only after all the requested characteristics are discovered, not one by one. After that, discovery/reading can be done on all of them in any order.

In fact, I do have my own implementation doing just that, and I am able to use CombineLatest on several publishers. In my use case, I am actually subscribing to 8 characteristics that way, as the UI need to display metrics from all of them at once.

Except that my code is not very generic, unlike yours, so I am not sure how plausible this is in case of your library.

But what I do:

DrAma999 commented 3 years ago

@NeverwinterMoon So basically you opened a pandora box :-D , and I'm glad that you did. The problem is more profound and really deep into the architecture, basically now my library is safe to be used only if you make operations synchronously as in the example. The main problem is due to nature of cold publisher of most of combine's publishers. I'll make you an example when you combineLatest two publishers you basically trigger them to start simultaneously this had some impact on characteristics recognition, but most importantly on reading and writing. In the pipelines I've put some checks to see for instance if the characteristic you requested is the one that has been sent from the device. Now that we have 2 publishers with different characteristics the pipeline receives 2 values. One is battery and one is speed, depending on which one is sent before the code breaks. Same happens for reading requests for a specific type, one of the 2 is not the same type you requested in one of your reading, and it breaks. I'm trying different approaches: one is acting on the back pressure in these way each read is executed until the end and they can be combined together basically I will force a sync process. The other is trying to figure out if I can filter them( by matching characteristics and type). I really grateful for your suggestion and this bug. I'm pretty sure I will find a solution :-D

DrAma999 commented 3 years ago

I guess that I've was able to fix the issue. Thank you, Andrea