kable
kable copied to clipboard
Make disconnect timeout configurable instead of requiring use of `withTimeout`
Per documentation, when disconnect
is called, it should be wrapped with withTimeout
. This makes the API error-prone, as forgetting to do so can create very rare and hard to track down bugs.
It would be better to configure the disconnect timeout via the peripheral
builder lambda (with a sane default).
Should scanner also be considered?
val advertisement = Scanner() .advertisements .first { it.name?.startsWith("Example") }
Can potententially suspend indefinitelly and on the SensorTag project is actually also wrapped with withTimeoutOrNull(SCAN_DURATION_MILLIS)
Should scanner also be considered?
Interesting idea, although with advertisements
being a Flow
it would be a bit more difficult to internally provide a timeout (as apposed to the Peripheral.disconnect
method which is a suspend fun
and therefore very easy to internally wrap its actions in withTimeoutOrNull
).
Scanning (being powered by a Flow
) also carries a familiar API so I feel is a bit less error prone? Being that it is familiar for developers to have to ultimately collect
or use a terminal operator on a Flow
, I think the potential pitfalls and ways to avoid them are well rehearsed. Whereas the disconnect()
function conveys its behavior by its naming, so developers will read it as "the operation to disconnect a peripheral" but it isn't obvious that it could wait an indefinite amount of time — as I believe other similar connection-based paradigms (such as sockets) would not have a similar "potentially indefinite" behavior. On top of that, disconnect
really shouldn't ever stall, but Android BLE is (as many are aware) a strange and unpredictable beast, so the timeout is a safeguard/defensive measure against Android not informing Kable that the disconnect was successful (i.e. in the event that Android never delivers the STATE_DISCONNECTED
event). If Android's BLE API always behaved correctly, then it would not be an issue at all (Kable would always get the signal that disconnect succeeded and would never stall). But since number of BLE connections on Android is very limited, the timeout prevents connection leaks in the event Android BLE API misbehaves.
Additionally, there may eventually be an official Flow
timeout
operator (https://github.com/Kotlin/kotlinx.coroutines/issues/2624)?
Sorry for the long-winded response, that is all to say, I'm not apposed to the idea of offering a built in timeout for scanning, but it'll likely be something to revisit further down the road.