search

rsnodgrass / pyadtpulse

Python interface for ADT Pulse security systems
Other
15 stars 5 forks source link

Fixes #17

Closed rlippmann closed 1 year ago

rlippmann commented 1 year ago

Make username, password, and fingerprint mandatory timeout changes, rework example-client Add booleans to update methods Simplify/Rework HTTP headers Cache zones and update from cache alarm testing, rework, sync, json file

rsnodgrass commented 1 year ago

Fantastic work and cleanup. Thanks @rlippmann !!! I did spot code reviews on about half the files, which all looked fine, so I assume rest is fine (too many changes at once! LOL)

rsnodgrass commented 1 year ago

FYI, published to PyPi as 0.3.0 https://pypi.org/project/pyadtpulse/0.3.0/

rlippmann commented 1 year ago

Fantastic work and cleanup. Thanks @rlippmann !!! I did spot code reviews on about half the files, which all looked fine, so I assume rest is fine (too many changes at once! LOL)

Thanks!

The HA side seems slow for some reason, so I’m going to look at that now.

I have a couple of questions/issues: 1) can it be assumed that multiple sites will never be supported? The code is kind of messy, so I’d like to remove it. 2) I was looking at making it async. It looks like the pulse site just fires up async calls to sync check and orb. I’d like to be able to do that too, which will also enable me to make it push instead of poll. Can I break the existing sync calls, or should I wrap them and make separate async calls? 3) it looks like the ADT site makes a call to get some JS to compute the fingerprint, but not being a JS/web developer, I’m not exactly sure what it’s doing. It would be nice to automatically figure out the fingerprint instead of having people try and find it. Do you have any experience with that?

Thanks, Rob

rsnodgrass commented 1 year ago

Rob,

Have at it! This was hacked together just to support getting SOMETHING from the ADT site, so feel free to change however you'd like. I'm not really working on it as I had barely the time to throw something together originally. I switched to using the HomeBridge integration because I didn't have any time or desire to maintain it (though would be happy to switch back).

Regarding sync/async do you mean breaking the existing sync pyadtpulse interface? I think I would try to avoid that because who knows who is using this. Once you get async working, you could just gut the existing sync API and have it wrap around the async version (though with a default event loop thread in background...the caller should be able to inject their own event loop to avoid the separate one).

I've never looked at ADT's JS or web, especially since they added two-factor fingerprint.

Thanks for having some passion to improve this.

Ryan

On Sat, Jan 21, 2023 at 12:52 PM rlippmann @.***> wrote:

Fantastic work and cleanup. Thanks @rlippmann https://github.com/rlippmann !!! I did spot code reviews on about half the files, which all looked fine, so I assume rest is fine (too many changes at once! LOL)

Thanks!

The HA side seems slow for some reason, so I’m going to look at that now.

I have a couple of questions/issues:

  1. can it be assumed that multiple sites will never be supported? The code is kind of messy, so I’d like to remove it.
  2. I was looking at making it async. It looks like the pulse site just fires up async calls to sync check and orb. I’d like to be able to do that too, which will also enable me to make it push instead of poll. Can I break the existing sync calls, or should I wrap them and make separate async calls?
  3. it looks like the ADT site makes a call to get some JS to compute the fingerprint, but not being a JS/web developer, I’m not exactly sure what it’s doing. It would be nice to automatically figure out the fingerprint instead of having people try and find it. Do you have any experience with that?

Thanks, Rob

— Reply to this email directly, view it on GitHub https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399330422, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQY4XF57BWJDN7NEKH2F4TWTREBFANCNFSM6AAAAAAUCIOAW4 . You are receiving this because you modified the open/close state.Message ID: @.***>

rlippmann commented 1 year ago

Can I break the query method? It really should be private anyway. Otherwise I need to use both requests and aiohttp.

Thanks, Rob

On Jan 21, 2023, at 6:27 PM, Ryan @.***> wrote:



Rob,

Have at it! This was hacked together just to support getting SOMETHING from the ADT site, so feel free to change however you'd like. I'm not really working on it as I had barely the time to throw something together originally. I switched to using the HomeBridge integration because I didn't have any time or desire to maintain it (though would be happy to switch back).

Regarding sync/async do you mean breaking the existing sync pyadtpulse interface? I think I would try to avoid that because who knows who is using this. Once you get async working, you could just gut the existing sync API and have it wrap around the async version (though with a default event loop thread in background...the caller should be able to inject their own event loop to avoid the separate one).

I've never looked at ADT's JS or web, especially since they added two-factor fingerprint.

Thanks for having some passion to improve this.

Ryan

On Sat, Jan 21, 2023 at 12:52 PM rlippmann @.***> wrote:

Fantastic work and cleanup. Thanks @rlippmann https://github.com/rlippmann !!! I did spot code reviews on about half the files, which all looked fine, so I assume rest is fine (too many changes at once! LOL)

Thanks!

The HA side seems slow for some reason, so I’m going to look at that now.

I have a couple of questions/issues:

  1. can it be assumed that multiple sites will never be supported? The code is kind of messy, so I’d like to remove it.
  2. I was looking at making it async. It looks like the pulse site just fires up async calls to sync check and orb. I’d like to be able to do that too, which will also enable me to make it push instead of poll. Can I break the existing sync calls, or should I wrap them and make separate async calls?
  3. it looks like the ADT site makes a call to get some JS to compute the fingerprint, but not being a JS/web developer, I’m not exactly sure what it’s doing. It would be nice to automatically figure out the fingerprint instead of having people try and find it. Do you have any experience with that?

Thanks, Rob

— Reply to this email directly, view it on GitHub https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399330422, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQY4XF57BWJDN7NEKH2F4TWTREBFANCNFSM6AAAAAAUCIOAW4 . You are receiving this because you modified the open/close state.Message ID: @.***>

— Reply to this email directly, view it on GitHubhttps://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399353098, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AQ4ZQLN7ULV4XYWOIQDH4KTWTRWENANCNFSM6AAAAAAUCIOAW4. You are receiving this because you were mentioned.Message ID: @.***>

rsnodgrass commented 1 year ago

Have at it. As long as the Home Assistant integration still works I'm fine with any changes.

On Mon, Jan 23, 2023 at 4:53 PM rlippmann @.***> wrote:

Can I break the query method? It really should be private anyway. Otherwise I need to use both requests and aiohttp.

Thanks, Rob

On Jan 21, 2023, at 6:27 PM, Ryan @.***> wrote:



Rob,

Have at it! This was hacked together just to support getting SOMETHING from the ADT site, so feel free to change however you'd like. I'm not really working on it as I had barely the time to throw something together originally. I switched to using the HomeBridge integration because I didn't have any time or desire to maintain it (though would be happy to switch back).

Regarding sync/async do you mean breaking the existing sync pyadtpulse interface? I think I would try to avoid that because who knows who is using this. Once you get async working, you could just gut the existing sync API and have it wrap around the async version (though with a default event loop thread in background...the caller should be able to inject their own event loop to avoid the separate one).

I've never looked at ADT's JS or web, especially since they added two-factor fingerprint.

Thanks for having some passion to improve this.

Ryan

On Sat, Jan 21, 2023 at 12:52 PM rlippmann @.***> wrote:

Fantastic work and cleanup. Thanks @rlippmann https://github.com/rlippmann !!! I did spot code reviews on about half the files, which all looked fine, so I assume rest is fine (too many changes at once! LOL)

Thanks!

The HA side seems slow for some reason, so I’m going to look at that now.

I have a couple of questions/issues:

  1. can it be assumed that multiple sites will never be supported? The code is kind of messy, so I’d like to remove it.
  2. I was looking at making it async. It looks like the pulse site just fires up async calls to sync check and orb. I’d like to be able to do that too, which will also enable me to make it push instead of poll. Can I break the existing sync calls, or should I wrap them and make separate async calls?
  3. it looks like the ADT site makes a call to get some JS to compute the fingerprint, but not being a JS/web developer, I’m not exactly sure what it’s doing. It would be nice to automatically figure out the fingerprint instead of having people try and find it. Do you have any experience with that?

Thanks, Rob

— Reply to this email directly, view it on GitHub < https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399330422>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAQY4XF57BWJDN7NEKH2F4TWTREBFANCNFSM6AAAAAAUCIOAW4>

. You are receiving this because you modified the open/close state.Message ID: @.***>

— Reply to this email directly, view it on GitHub< https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399353098>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/AQ4ZQLN7ULV4XYWOIQDH4KTWTRWENANCNFSM6AAAAAAUCIOAW4>.

You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1401219794, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQY4XGIB3T733LUQWAXST3WT4RZ5ANCNFSM6AAAAAAUCIOAW4 . You are receiving this because you modified the open/close state.Message ID: @.***>

rlippmann commented 1 year ago

Hi,

So, I need your advice on something.

I currently have it working with asyncio/aiohttp. It also creates 2 tasks, 1 to repeatedly poll the AjaxCheckSrv to see if updates are available, and another to repeatedly wake up every 5 minutes to send a keepalive to the server to prevent it from being logged out.

As a result of this, it’s now a push instead of a poll. There’s no need to actually call an update method, and the check to see if updates are available is just a check/wait of an asyncio.Event

My problem is what to do with the non-async api. Specifically what to do if the user calls time.sleep() vs asyncio.sleep() since that will block the background tasks on the event loop. This would also apply for other ways the user doesn’t give the event loop a chance to run.

As far as I can determine, my options are: 1) require the user to not use time.sleep and call asyncio.sleep via creating another thread and doing run_coroutine_threadsafe to do an await on asyncio.sleep() 2) doing this internally via a sleep method 3) since I’m creating threads anyway, just convert the background tasks to threads. I feel that this somewhat defeats the purpose of using asyncio in the first place.

There’s also a similar problem of blocking the event loop with many of the existing sync methods (like login and logout). Since I’m using aiohttp, I moved the existing sync methods to new methods named async_originalmethodname. And to use the existing sync methods, I need to do the create a new thread/run_couroutine_threadsafe to call those methods under the covers.

So, what do you suggest?

Thanks, Rob

On Jan 23, 2023, at 8:52 PM, Ryan @.***> wrote:



Have at it. As long as the Home Assistant integration still works I'm fine with any changes.

On Mon, Jan 23, 2023 at 4:53 PM rlippmann @.***> wrote:

Can I break the query method? It really should be private anyway. Otherwise I need to use both requests and aiohttp.

Thanks, Rob

On Jan 21, 2023, at 6:27 PM, Ryan @.***> wrote:



Rob,

Have at it! This was hacked together just to support getting SOMETHING from the ADT site, so feel free to change however you'd like. I'm not really working on it as I had barely the time to throw something together originally. I switched to using the HomeBridge integration because I didn't have any time or desire to maintain it (though would be happy to switch back).

Regarding sync/async do you mean breaking the existing sync pyadtpulse interface? I think I would try to avoid that because who knows who is using this. Once you get async working, you could just gut the existing sync API and have it wrap around the async version (though with a default event loop thread in background...the caller should be able to inject their own event loop to avoid the separate one).

I've never looked at ADT's JS or web, especially since they added two-factor fingerprint.

Thanks for having some passion to improve this.

Ryan

On Sat, Jan 21, 2023 at 12:52 PM rlippmann @.***> wrote:

Fantastic work and cleanup. Thanks @rlippmann https://github.com/rlippmann !!! I did spot code reviews on about half the files, which all looked fine, so I assume rest is fine (too many changes at once! LOL)

Thanks!

The HA side seems slow for some reason, so I’m going to look at that now.

I have a couple of questions/issues:

  1. can it be assumed that multiple sites will never be supported? The code is kind of messy, so I’d like to remove it.
  2. I was looking at making it async. It looks like the pulse site just fires up async calls to sync check and orb. I’d like to be able to do that too, which will also enable me to make it push instead of poll. Can I break the existing sync calls, or should I wrap them and make separate async calls?
  3. it looks like the ADT site makes a call to get some JS to compute the fingerprint, but not being a JS/web developer, I’m not exactly sure what it’s doing. It would be nice to automatically figure out the fingerprint instead of having people try and find it. Do you have any experience with that?

Thanks, Rob

— Reply to this email directly, view it on GitHub < https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399330422>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAQY4XF57BWJDN7NEKH2F4TWTREBFANCNFSM6AAAAAAUCIOAW4>

. You are receiving this because you modified the open/close state.Message ID: @.***>

— Reply to this email directly, view it on GitHub< https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1399353098>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/AQ4ZQLN7ULV4XYWOIQDH4KTWTRWENANCNFSM6AAAAAAUCIOAW4>.

You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1401219794, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQY4XGIB3T733LUQWAXST3WT4RZ5ANCNFSM6AAAAAAUCIOAW4 . You are receiving this because you modified the open/close state.Message ID: @.***>

— Reply to this email directly, view it on GitHubhttps://github.com/rsnodgrass/pyadtpulse/pull/17#issuecomment-1401298770, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AQ4ZQLLBFUMTFXID3BS2JKDWT4YWFANCNFSM6AAAAAAUCIOAW4. You are receiving this because you were mentioned.Message ID: @.***>