Rework 'setup complete' handling to allow users to create a custom Intent.

particle-iot / spark-setup-android

Former home of the Particle Device Setup library for Android

Apache License 2.0

23 stars 30 forks source link

Rework 'setup complete' handling to allow users to create a custom Intent. #21

Closed charleskorn closed 8 years ago

charleskorn commented 8 years ago

Notes:

I've marked a couple of methods in ParticleDeviceSetupLibrary as deprecated. These should be removed completely at some point in the future.
This change probably removes the need for the broadcast when device setup is complete, but I haven't removed it yet to avoid breaking users who currently rely on it.

jensck commented 8 years ago

Hey, thanks for the contribution! I really like the idea of optionally firing a dev-defined Intent at the end of the setup process, especially as an alternative to passing in an Activity class reference. (That wasn't one of my finer moments in API design...)

Could this be based around a PendingIntent instead? That would provide the same functionality but even more flexibility.

jensck commented 8 years ago

One last thing: have you signed the CLA? --> https://goo.gl/FJiZP0

charleskorn commented 8 years ago

What advantage do you see in using PendingIntents? Aren't they only really applicable if you're passing an Intent to another application (eg. NotificationManager)?

jensck commented 8 years ago

re: PendingIntent, they're nice because they can be used to start activities, start services, fire broadcasts, whatever. The SDK doesn't have to care, it just has to call .send().

However, after considering it further, let's stick with Intent. The great majority of SDK users won't need the flexibility, and those who do want it will be able to (ultimately) get the behavior they want by listening for the ACTION_DEVICE_SETUP_COMPLETE broadcast.

jensck commented 8 years ago

Looks like you've addressed all the feedback. As soon as you've signed the CLA (mentioned above), this looks good to merge!

charleskorn commented 8 years ago

Awesome, should be able to do that in the next day or so hopefully. (Just waiting for my company to give me the OK.)

charleskorn commented 8 years ago

I've just submitted the CLA - let me know if you need me to do anything else.

jensck commented 8 years ago

Merged! Thanks again for the great patch.

charleskorn commented 8 years ago

No worries, thank you.

Do you have plans to release a new version of the library with this change soon?

jensck commented 8 years ago

I've got an important bug to fix that should get shipped soon, and this is a nice enhancement, so I should probably do a release this weekend.

In the short term, if you want to create app builds with these updates, you can just make a build of the setup lib and refer to it locally in your build.gradle: https://goo.gl/2xSAUz

HTH

charleskorn commented 8 years ago

Hey @jensck, do you have any updates on that release?

jensck commented 8 years ago

@charleskorn done! Pull down 0.3.7!

charleskorn commented 8 years ago

Awesome, thanks.