search

brbeaird / SmartThings_MyQ

Integrate SmartThings with MyQ (Obsolete)
https://github.com/brbeaird/SmartThings-MyQ-Edge
Apache License 2.0
410 stars 896 forks source link

Improve authentication management #59

Closed rboy1 closed 5 years ago

rboy1 commented 5 years ago

Handle login and API responses better Retry token expiration responses with a limit Only login when required to reduce load on servers Indicate code type to reduce user errors while copy/pasting code the IDE

brbeaird commented 5 years ago

This is great - thanks so much for taking the time to review some of this. I had forgotten about the token expiration issue. I think at some point I reduced it because I was seeing unexpected expirations and just needed to get it working again - I had intended to add a retry as well, so that's awesome.

I don't know how much load it will really take off since the app isn't actually doing any regular polling. It's essentially been only logging in and generating a token once per door event, and for most people that would not be more than 10-15 per day. Still, this is definitely the right thing to do and any decrease is a good one as far as I'm concerned.

I've been very busy with other things lately, but I'll see if I can take a deeper look and get this merged in soon. Thanks again!

dyerttrey commented 5 years ago

Thank you all so much for this app. I am new to smartthings and this was my first venture into using github and custom handlers/apps.

One thing I have not done and am unsure of is the updating of my app with new code. Any help would be appreciated. Will it update automatically or will I have to download something and update? Any procedure would be appreciated. Would you also send the correct link for me to make donation. I want to make sure I have the correct one.

Thank you

Trey

Trey Dyer

256.933.0351 dyertrey@gmail.com

On Wed, Mar 13, 2019, 4:15 PM Brian Beaird notifications@github.com wrote:

This is great - thanks so much for taking the time to review some of this. I had forgotten about the token expiration issue. I think at some point I reduced it because I was seeing unexpected expirations and just needed to get it working again - I had intended to add a retry as well, so that's awesome.

I don't know how much load it will really take off since the app isn't actually doing any regular polling. It's essentially been only logging in and generating a token once per door event, and for most people that would not be more than 10-15 per day. Still, this is definitely the right thing to do and any decrease is a good one as far as I'm concerned.

I've been very busy with other things lately, but I'll see if I can take a deeper look and get this merged in soon. Thanks again!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/brbeaird/SmartThings_MyQ/pull/59#issuecomment-472627220, or mute the thread https://github.com/notifications/unsubscribe-auth/At9VEg21OoFp01S-ANB04vPdxjnin-y3ks5vWXhkgaJpZM4brPP9 .

brbeaird commented 5 years ago

@dyerttrey - it will not update automatically; you've got three options:

  1. Get the Community Installer SmartApp. Open it from time to time via the ST app and trigger the code updates through there. (recommended)
  2. Set up GitHub integration in your SmartThings IDE. Trigger an update to pull the latest code.
  3. Manually update the code yourself, copying/pasting new code.

There's not really an automated way to notify when a new version is available aside from when I post to the thread on the SmartThings Community forums. I'd suggest you "watch" that thread and otherwise just check there and/or this repo to see if there's been an update.

The correct donation link is https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=6QH4Y5KCESYPY. All donations are certainly appreciated :-)

rboy1 commented 5 years ago

This is something we learnt during our work with Blink, because of the way most systems are setup with caching etc, authentication is not cached and a very slow/expensive operation for the DB's. So when you think about 10-15 x 1000's login's per day, it adds up. Plus folks do customize code :)

In this commit I've set the token expiration to every 24 hours which I've been running without any issues for months now. Ideally it should be set to infinite or a much higher number like 7 days. Then token generation only happens when the server tells it that it's expired. If you would like to do that you can easily change state.session.expiration = now() + (24*60*60*1000) // 24 hours default to state.session.expiration = now() + (7*24*60*60*1000) // 7 days default

brbeaird commented 5 years ago

Merged. I'll update the manifest versioning in a bit. Thanks again.