troystribling / BlueCap

iOS Bluetooth LE framework
MIT License
715 stars 115 forks source link

How to catch the error when a connected peripheral was been power down? #80

Closed liuwin7 closed 6 years ago

liuwin7 commented 6 years ago

As the README.md say, we can give the Future a failure callback. But for the error PeripheralError.disconnected, it just stand for the disconnection without error, which should happen when the manager actively disconnect the peripheral.

For the unexpected disconnection, such as shutting down the peripheral, exceeding the effective distance, the delegate will received a message with error in BluetoothKit. In this case, your design just put it to the failure promise.

if let error = error {
                Logger.debug("disconnecting with errors uuid=\(identifier.uuidString), name=\(self.name), error=\(error.localizedDescription), disconnection count=\(_disconnectionCount) ")
                connectionPromise?.failure(error)
            } else  {
                Logger.debug("disconnecting with no errors uuid=\(identifier.uuidString), name=\(self.name)")
                self.connectionPromise?.failure(PeripheralError.disconnected)
            }

Is there a better resolution for this condition?

troystribling commented 6 years ago

PeripheralError.disconnect is an Swift.Error. https://github.com/troystribling/BlueCap/blob/d4592cc4479fb6705bea527e850db1ef079d7077/BlueCapKit/Errors.swift#L57-L75

So returning PeripheralError.disconnected is returning an error. In the code sample above PeripheralError.disconnected is returned if there is no other low level error. Such, as the example you describe.

I am not sure I understand your question. Are you asking is there another way to send he disconnect event other than the error callback? If so my opinion is that on the connection future success is only used for connections everything else is an error.

liuwin7 commented 6 years ago

No, I do not challenge using the error callback. I mean that you should give an enum case to indicate the disconnection with a NSError or Swift.Error. However, in your code, you just give the original NSError to the callback instead of your defined PeripheralError case.

if let error = error {
                Logger.debug("disconnecting with errors uuid=\(identifier.uuidString), name=\(self.name), error=\(error.localizedDescription), disconnection count=\(_disconnectionCount) ")
                connectionPromise?.failure(error) // HERE is what I confused.
            } else  {
                Logger.debug("disconnecting with no errors uuid=\(identifier.uuidString), name=\(self.name)")
                self.connectionPromise?.failure(PeripheralError.disconnected)
            }

I hope that I have explain it. Thanks.

troystribling commented 6 years ago

Are you suggestion having one System error case in the enum or mapping all system errors to a Peripheral error? How exactly would you implement the first case? Would you need an associated value to store the code and message?

You would still need to do the cast to PeripheralError since futures take a Swift.Error.

liuwin7 commented 6 years ago

Maybe, you can give a more clear case instead of just a disconnection, for example disconnectionWithError. One will ignore the disconnection with error when using the disconnection to indicate the no error case.

troystribling commented 6 years ago
This would work but it would be a breaking change since people will be using casts like in the examples. Swift 5 will be a big update. I may address this later. Thanks,

public enum PeripheralError : Swift.Error, LocalizedError { case disconnected case forcedDisconnect case connectionTimeout case serviceDiscoveryTimeout case disconnectedWithError(error: Swift.Error)

public var errorDescription: String? {
    switch self {
    case .disconnected:
        return NSLocalizedString("Peripheral disconnected.", comment: "PeripheralError.disconnected")
    case .forcedDisconnect:
        return NSLocalizedString("Peripheral disconnect called.", comment: "PeripheralError.forcedDisconnect")
    case .connectionTimeout:
        return NSLocalizedString("Peripheral connection timeout.", comment: "PeripheralError.connectionTimeout")
    case .serviceDiscoveryTimeout:
        return NSLocalizedString("Service discovery timeout.", comment: "PeripheralError.serviceDiscoveryTimeout")
    case .disconnectedWithError(let error):
        return error.localizedDescription
    }
}

}

liuwin7 commented 6 years ago

Ok, all right. No matter what, thanks for your efforts on encapsulation of the CoreBluetooth. : )