Peripheral CoroutineScope

[damischa1]

Hello,

thanks for nice library for BLE. I started using it in a project and it seems to work nice.

Here's a few questions/ideas I have for the peripheral.

1. Why do you need to specify CoroutineScope to make an instance of Peripheral by the builder function? There is nothing useful you can do with the peripheral before it is connected.

Anyways we are giving CoroutineScope implicitly for connect() function and that coroutine scope should be used for the connection.

Maybe you could make the connect function awaitCancellation() and cancel the connect-functions's coroutine scope to make the connection to disconnect.

Then you could also remove the disconnect function and make the interface event neater. All the cleanup could be done at cancellation handler of the connect function.

Also the Identifier/BluetoothDevice/Advertisement could be given as parameter for connect function so it would not be necessary to give it when constructing the peripheral.

Just ideas. Maybe there are reasons behind these...

[twyatt]

Why do you need to specify CoroutineScope to make an instance of Peripheral by the builder function? There is nothing useful you can do with the peripheral before it is connected.

Very good points. For Apple platform we do spin up Coroutines in order to monitor out-of-band states of peripherals, but we could launch those later (at time of connect). Javascript kind of throws a wrench in things, whereas the disconnect call does launch a Coroutine in order to process the disconnect request. This could probably be adjusted to just be a suspend function — and would then allow a change to the API as you're describing (passing CoroutineScope to the connect rather than being used for creation of Peripherals).

The major drawback I see with passing a CoroutineScope to the connect function is in the cases where connect is called multiple times. Currently, if two different coroutines call connect, they both will suspend until the connection is established. If we instead pass a CoroutineScope to connect, how should we handle that? Would we have only the first call succeed and the 2nd call throw an exception because a different CoroutineScope was passed in? Or, do we hold a reference to all CoroutineScopes passed in? If that is the case, what if only one is cancelled? I think from an edge-case perspective, that API would be difficult to implement and have its behavior be intuitive — despite logically being the only time you really need the CoroutineScope.

Then you could also remove the disconnect function and make the interface event neater. All the cleanup could be done at cancellation handler of the connect function.

I do favor keeping API surfaces smaller, so opportunities to do so are worth thinking about. Although in this case, I think disconnect is very familiar for developers, and it makes the API easy to learn. Being that most developers think of BLE as being a process of connecting then disconnecting. The implicit disconnect via CoroutineScope cancellation is offered as a safe guard and might be somewhat confusing for developers not familiar to coroutines if it is the only mechanism for disconnect.

Also the Identifier/BluetoothDevice/Advertisement could be given as parameter for connect function so it would not be necessary to give it when constructing the peripheral.

Similar to above, this does make sense as far as the implementation details of Kable, but the edge-case of developers calling connect multiple times with different identifiers or BluetoothDevices makes this troublesome to implement and have its behavior be intuitive. Having a Peripheral tightly coupled to an identifier/device/advertisement gives it a more immutable behavior and therefor less error prone? Despite (as you have pointed out) not being entirely necessary from the perspective of the inner workings of the Peripheral.