Supporting connection with no timeout

[jakerockland]

Related to #13, I also realized that the library does not currently support connecting to a peripheral without a timeout, something that is sometimes necessary and is the normal expected behavior of the underlying CoreBluetooth framework (https://developer.apple.com/videos/play/wwdc2012/705/).

I'm happy to implement this but were wondering your thoughts on how to expose this, as the current timeout variable has a default value, so making it an optional variable type seems a little odd. One option would be to have two different public methods, something along the lines of:

public func connect(withTimeout timeout: TimeInterval?, completion: @escaping ConnectPeripheralCallback) {
    if let timeout = timeout {
        self.peripheralProxy.connect(timeout: timeout, completion)
    } else {
        self.peripheralProxy.connect(completion)
    }
}

public func connect(completion: @escaping ConnectPeripheralCallback) {
        self.peripheralProxy.connect(timeout: 10, completion)
}

[jakerockland]

I'm struggling to figure out what would be the cleanest way to implement this. 😓

[jordanebelanger]

Don't think it's really important, you shouldn't try to connect without a timeout as far as I am concerned and the parameter is optional anyway since it has a default value.

If someone really needs to connect without a timeout, they can always pass in Int.max as a timeout.

[jakerockland]

Why should you not try to connect without a timeout? This is directly contrary to Apple's specified behavior... I realize that it is optional anyway since it has a default value, which is what makes this a bit messy to try to implement. I do think it is something important to implement, but will do it on our fork if you don't want to have it as part of the library here. Unfortunately it ends up having to be a bit of a destructive change that pushes our fork further from the main fork here.

[jordanebelanger]

Why should you?

[jakerockland]

Have you watched the Advanced Bluetooth WWDC video from 2012 that I linked to above? Adding a timeout to connection is a useful feature in some cases, but it is not the default behavior of CoreBluetooth and removes the ability to specify connecting to a peripheral that is not currently in range or able to be connected to but is expected to be at some undetermined point in the future. For example, automatically connecting to a smart home device as soon as one is within range of the house to turn on some IoT device. To give a very explicit example, think of a Bluetooth enabled smart lock that unlocks when you come within a certain distance of it. Having the connect method behave as Apple intended it is necessary for such a use case.

[jakerockland]

Given that timeouts are a new additional functionality being added on top of CoreBluetooth's functionality, it would make more sense to me if the timeout parameter was just optional rather than having a default value, making it behave by default as Apple's underlying implementation does and only exposing the timeout as an additional functionality for times where a timeout is beneficial?

[jordanebelanger]

I see, no I did not check out the video as I am at work. I wasn't aware of that, I guess if you want to do it with a quick fix, make the timeout parameter an optional with a default value of nil. If the user pass in nil, just call the central proxy with a Int.max timeout and it should work just fine.

Default value could also remain 10 despite the timeout being optional, but that's not as clear. There could also be 2 method, one with a non optional timeout parameter defaulted at 10, and one without timeout. The one without timeout call with Int.max

[jakerockland]

No worries, that video is a bit cumbersome to watch, just wasn't sure if you had seen it before. It wasn't something I was previously aware of either but realized is something we likely may need in the future and is Apple's expected behavior.

I like your first proposed solution, I feel like it ends up being the cleanest and most explicit to the user (if they specify a timeout it uses their timeout, otherwise there is no timeout), makes the fix really quick:

    /// Connect to the peripheral through Ble to our Central sharedInstance
    public func connect(withTimeout timeout: TimeInterval?, completion: @escaping ConnectPeripheralCallback) {
        if let timeout = timeout {
            self.peripheralProxy.connect(timeout: timeout, completion)
        } else {
            self.peripheralProxy.connect(timeout: TimeInterval.infinity, completion)
        }
    }

Opening up a PR now 👍