Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

State restoration support? #16

Closed
jakerockland opened this issue Dec 20, 2016 · 14 comments
Closed

State restoration support? #16

jakerockland opened this issue Dec 20, 2016 · 14 comments

Comments

@jakerockland
Copy link
Contributor

Does the framework currently support state preservation and restoration? It doesn't seem to but I can't tell if I'm missing something. 馃槗

Looking specifically at this documentation from Apple.

@jakerockland
Copy link
Contributor Author

Working on a proposed implementation of this, would be great to get your thoughts @tehjord, even if state restoration isn't something you intend to support with this framework. Was thinking of a solution similar to that I proposed for enabling underlying access, using a flag SWIFTYBLUETOOTH_STATE_RESTORATION, something like this:

// Uncomment this line to enable state restoration for the underlying CBCentral
SWIFTYBLUETOOTH_STATE_RESTORATION = // SWIFTYBLUETOOTH_STATE_RESTORATION

// Do not modify this line. Instead, remove comments above as needed to enable desired features.
SWIFTYBLUETOOTH_FLAGS = $(SWIFTYBLUETOOTH_STATE_RESTORATION)

SWIFT_ACTIVE_COMPILATION_CONDITIONS= $(inherited) $(SWIFTYBLUETOOTH_FLAGS)
final class CentralProxy: NSObject {
    
    // Other stuff here, not shown
    
    override init() {
        #if SWIFTYBLUETOOTH_STATE_RESTORATION
        self.centralManager = CBCentralManager(delegate: nil, queue: nil, options: [CBCentralManagerOptionRestoreIdentifierKey: "SwiftyBluetoothCentral"])
        #else
        self.centralManager = CBCentralManager(delegate: nil, queue: nil)
        #endif
        super.init()
        self.centralManager.delegate = self
    }

}

@jakerockland
Copy link
Contributor Author

Above is just the initial thoughts for a solution, does not handle all of the necessary state restoration stuff, just enables the central to be restored.

@jakerockland
Copy link
Contributor Author

jakerockland commented Dec 20, 2016

This solution would maintain current default behavior of no state restoration and allow enabling it for those who need it.

@jakerockland
Copy link
Contributor Author

It looks like the restoration delegate method has already been implemented:

    func centralManager(_ central: CBCentralManager, willRestoreState dict: [String: Any]) {
        self.postCentralEvent(.CentralManagerWillRestoreState, userInfo: dict)
    }

So it looks like state restoration would just need to be enabled (for example using a solution like the one I proposed above) and then one would be able to restore their previous CoreBluetooth state by observing CentralEvent.CentralManagerWillRestoreState?

@jordanebelanger
Copy link
Owner

jordanebelanger commented Dec 20, 2016

I look at all this, love your ideas, but I am not very fond fond macros and would like to find an alternate solution if possible.

More and more, it seems to me like, as it often happens when you go the singleton route, it ends up bitting you later on.

On one hand, I think you agree that it's good to keep the library as a bit of a small overlay on top of CoreBluetooth focused on giving access to a closure interface. On the other hand, there are certain things that are kind of nice to have, like for me, that would be the autoconnection and discovery of characteristics when you first try to read one. There is a fine balance between the two and we're sort of edging on either side.

I'm not sure whats best tbh, I went the singleton route because having multiple CBCentralManager is kind of unorthodox, but, at the same time, it's not unheard of. As I'd rather avoid throwing macros in, it might be a good idea to simply get rid of singleton pattern and have users initialize their own Central through an init method that let you pass in those CBCentralManager options upon initialization.

Hell, we could even create an initializer that let you pass in your own CBCentralManager while we're at it.

@jakerockland
Copy link
Contributor Author

jakerockland commented Dec 21, 2016

Hmm, you're right that this is a bit tricky haha 馃槀. I'm generally a fan of the singleton model because, you're right that having multiple CBCentralManagers is unorthodox (it also can cause unexpected/undetermined behavior from what I've read).

However, if you did want to go the route of having the CBCentralManager being something the user sets, or is set on initialization, I would also be happy with this solution and then could just go ahead and wrap it in my own singleton class.

I personally don't really have a problem with the macro flags as coming from a C background they're commonplace for me, but I also recognize that they aren't the Swifty-est nor most elegant solution. Still, I do think enforcing the singleton model is a very beneficial thing for people who are trying to quickly get started with CoreBluetooth.

@jordanebelanger
Copy link
Owner

jordanebelanger commented Jan 7, 2017

I saw one thing they do on the chromecast library is, they allow you to "set" the singleton (once) initially passing in initializer parameters, it looks like this:

 GCKCastContext.setSharedInstanceWith(GCKCastOptions(supportedNamespaces [chromecastNameSpace]))

We could do something similar by having the Central sharedinstance bet set like above but receiving CBCentralManager options type parameters (state restauration being one).

You would have to put this in your appDelegates appDidFinishLoading method for things to work, or we could make it optional, have the singleton be lazily initiated with no state restore parameter the first time you try and use one of its function if you didnt "set it" manually.

@jakerockland
Copy link
Contributor Author

@tehjord Sorry for the long delay in my response here! 馃槗 Been quite busy at work. I generally really like this idea of lazily initiated singleton that can be "set" once initially if desired--seems like a great idea to me!

@jakerockland
Copy link
Contributor Author

Am happy to help in the implementation of this, though it will be a little while until I have the time to get started on it. 馃槗

@jordanebelanger
Copy link
Owner

jordanebelanger commented Feb 22, 2017

Haha I feel you Jake, I have been pretty busy as well, in fact I haven't even released the "Result" object based branch :S One of the thing that annoys about updating the library is updating the podspec, I personally use Carthage but I am guessing the majority of the people finding the libraries are coming from the cocoapods website.

I also saw that these guys have copied my APIs https://github.com/Polidea/RxBluetoothKit :D The async bluetooth state is my invention :P But the library above forces you to use the pretty big RxSwift library. Last time I tried to use RxSwift, it pretty much permanently crashed my XCode/Sourcekit

Since I haven't released the Result branch yet, I was actually thinking about dropping the result Idea and actually moving towards using promises instead.

This guy has a clean <100 loc promise implementation that I feel the library could use:
https://github.com/kean/Pill

I would pretty much just drop the Promise.swift file from that project into our library, no external dependencies.

EDIT: Still not sure about promises though, every implementation I see of a Promise are using a class and therefore requiring an heap allocation every time a promise is returned. Call me OCD but I like to keep everything on the stack as much as possible.

@jakerockland
Copy link
Contributor Author

Cocoapods certainly has always seemed like a mess to me 馃槄. Though I appreciate it's utility and ubiquity, I'm much more fond of Carthage and have steered clear of Cocoa in my projects. We're getting ready to release a simple iOS SDK for interfacing with our device though and I'll have to dive into the weeds with Cocoapods soon enough...

Huh, I hadn't seen RxBluetoothKit before! Seems interesting but I'm much more fond of the modularity and size of this library haha 馃槃!

What was your reasoning for wanting to move to promises instead of the Result implementation? I like the Result idea quite a bit and can appreciate your desire to keep things on the stack as much as possible--I generally agree!

@jordanebelanger
Copy link
Owner

Mostly, I like way the promises flatten multiple async callbacks instead of having multiple level of nesting, it's easier to read. There is also that if you do error handling and use a result object, you have to do error handling in each individual nested block if you want to abort, with a promise you can have just one error handler.

It's kind of a trend I think.

@jakerockland
Copy link
Contributor Author

Hmm yeah that makes sense. I don't have a strong opinion on it either way honestly 馃槗 will have to look into things a bit more to be able to give a sophisticated input. I've only been programming in Swift for a bit less than a year so don't know the exact implications of different promise implementations.

@jordanebelanger
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants