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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added device event subscription #2

Merged
merged 8 commits into from
Oct 4, 2018

Conversation

markoImake
Copy link
Collaborator

Hi Eddy
I was working on my own nativescript-particle plugin until I saw you recently published this one. I added the additional features that I was using into your plugin. These include:

  1. loginWithToken - this is needed for managing shadow accounts when implementing Two-Legged Authentication (https://docs.particle.io/guide/how-to-build-a-product/authentication/#two-legged-authentication)
  2. isAuthenticated and accessToken methods to make it easier to manage shadow accounts
  3. subscribe to (and unsubscribe from) events on a TNSParticleDevice
  4. Particle Device Setup Wizard for ios, using particle 'ParticleSetup' sdk (https://github.com/particle-iot/particle-setup-ios)

@EddyVerbruggen EddyVerbruggen self-assigned this Sep 26, 2018
@EddyVerbruggen EddyVerbruggen self-requested a review September 26, 2018 16:20
@EddyVerbruggen EddyVerbruggen added enhancement New feature or request Android iOS labels Sep 26, 2018
@EddyVerbruggen EddyVerbruggen added this to the 1.1.0 milestone Sep 26, 2018
Copy link
Owner

@EddyVerbruggen EddyVerbruggen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! This is honestly excellent work! I'm very glad you decided to merge your code with the repo. And I'd be happy yo add you as a collaborator to the repo if you want to.

I've asked 2 questions, please check them out when you have time.

I'd be happy to test the updated demo app and add docs for these new features.

Thanks,
Eddy

}

public startDeviceSetupWizard(cb:any): void {
// stub for startDeviceSetupWizard
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my info: there's no device setup wizard for Android?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a device setup wizard available for Android. The device setup library is a separate native dependency - https://github.com/particle-iot/spark-setup-android, more info here https://docs.particle.io/reference/android/#android-device-setup-library. I don't have much experience with android so wasn't comfortable attempting to add that in. I do require it for the project I'm working on though. Is this something that you will be interested in adding?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, understood, fair enough! I always strive for x-platform feature parity so I might give it a stab! Not this week though, I'm swamped.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. that being said, that's not blocking for merging this PR of course.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sounds good. Do I need to do anything further to get the PR merged?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is perfect, I'll try to merge it asap.

@EddyVerbruggen EddyVerbruggen merged commit b238f87 into EddyVerbruggen:master Oct 4, 2018
EddyVerbruggen added a commit that referenced this pull request Oct 4, 2018
EddyVerbruggen added a commit that referenced this pull request Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android enhancement New feature or request iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants