jordanebelanger / SwiftyBluetooth

Closures based APIs for CoreBluetooth
MIT License
209 stars 66 forks source link

Connection timeout timer does not fire #21

Closed agildehaus closed 7 years ago

agildehaus commented 7 years ago

I'm unable to get a reliable connection timeout from this library. I've looked into it briefly and it seems the timer might be going out of scope?

I've found for these timers, Grand Central Dispatch makes cleaner code (no selectors, no @objc, no weak vars). Try doing something like this instead of using the Timer class:

// CentralProxy class-level

var connectTimer: DispatchSourceTimer?

// connect(peripheral:timeout:_:)

...

self.connectTimer = DispatchSource.makeTimerSource()
self.connectTimer!.setEventHandler {
    let uuid = request.peripheral.identifier

    self.connectRequests[uuid] = nil

    self.centralManager.cancelPeripheralConnection(request.peripheral)

    request.invokeCallbacks(error: SBError.operationTimedOut(operation: .connectPeripheral))
}

self.connectTimer!.scheduleOneshot(deadline: .now() + timeout, leeway: .seconds(0))
self.connectTimer!.resume()
jordanebelanger commented 7 years ago

Once you have scheduled a timer on a runloop, your reference to that timer might go out of scope if you don't retain it somewhere, but the actual timer is still present on your runloop until it is either invalidated or it runs its course and calls its selector.

Might I see your connect code?

There is no connection timeout by default, i.e

peripheral.connect { ... } or peripheral.connect(withTimeout: nil) { ... }

will never timeout, is that perhaps what you are doing?

agildehaus commented 7 years ago
private let peripheral: Peripheral

...

func connect(timeout: TimeInterval = 10.0) -> Promise<Void> {
        return Promise { resolve, reject in
            print("connect()")
            self.peripheral.connect(withTimeout: timeout, completion: { result in
                switch result {
                case .failure(let error):
                    reject(error)
                case .success:
                    self.peripheral.setNotifyValue(
                        toEnabled: true,
                        forCharacWithUUID: GKCard.controlPointUUID,
                        ofServiceWithUUID: GKCard.serviceUUID
                    ) { result in
                        switch result {
                        case .failure(let error):
                            let nsError = error as NSError
                            switch nsError.code {
                            case CBATTError.insufficientEncryption.rawValue:
                                reject(CardError.cardNotPaired)
                            default:
                                reject(error)
                            }
                        case .success:
                            print("connected")
                            resolve()
                        }
                    }
                }
            })
        }
    }
}

I'm using Hydra, a Swift promises library.

Everything works great except for the timeout. I modified the timer as I described above which fixes it. I'll do a bit more investigation into the cause.

jordanebelanger commented 7 years ago

Humm that is a bit disconcerting, lemme test the connect timeout stuff and see for myself.

What about threading, are the promises executing on the main thread?

agildehaus commented 7 years ago

Hydra runs the promise body on DispatchQueue.global(qos: .background) (source)

jordanebelanger commented 7 years ago

You can manually set the dispatch queue you want your Hydra promise to run on, try and run the promise on the mainQueue and see if your timeout works.

The library does not support executing bluetooth stuff on background threads, in fact I am not sure CoreBluetooth does either.

agildehaus commented 7 years ago

Okay, running the connection promise on the main thread does resolve the issue.

jordanebelanger commented 7 years ago

Woops, I read that as "does not" instead of "does" hehe

Glad to know it's working, enjoy!