MatMaul / pynetgear

Python library to control Netgear wireless routers through the SOAP-api.
MIT License
238 stars 75 forks source link

Added Device Management Functionality #50

Closed caffeinatedMike closed 5 years ago

caffeinatedMike commented 5 years ago

Closes Issue #16

Co-authored-by gskluzacek@gmail.com

MatMaul commented 5 years ago

Nice 🙂 I don't know if I'll have time I am going to holidays. Can you squash your commits into one with rebase ? Also I think it would be nice to add @gskluzacek as a co author, see here: https://help.github.com/articles/creating-a-commit-with-multiple-authors

caffeinatedMike commented 5 years ago

Sure, I'll work on doing that in the next few hours

caffeinatedMike commented 5 years ago

@Matmaul I just tried and pretty sure I did it wrong. You're better off taking care of all that.

MatMaul commented 5 years ago

Ok I'll do it 😉

caffeinatedMike commented 5 years ago

@MatMaul I'm happy to say the new feature is fully operational & returning expected results. I believe this feature is ready for the master branch whenever you have the time to review it this weekend :grin: :tada:

CC: @gskluzacek, @gruijter

Side Note: Now, who can implement this feature/functionality correctly in the device_tracker.netgear component for Home Assistant?

MatMaul commented 5 years ago

Final version here, I'll merge and release soon. I haven't test the new code yet. https://github.com/MatMaul/pynetgear/tree/allow-device-blocking

Next time please use pep8 for the syntax check, a lot of empty lines with spaces or tabs instead of spaces :) Ignore E501 line too long however if the length is still reasonable, 80 is too low.

caffeinatedMike commented 5 years ago

Will do! To be completely honest, the reason there were tabs (I never use tabs) is because I made the requested changes from my phone in the mobile chrome browser :stuck_out_tongue_winking_eye:

I always use spaces when coding and wipe out blank lines when "properly" coding on a PC.

Good to hear it'll be released soon! Hoping you have the same success I did when testing it, let me know :)

And, not sure if this is something you do, but do you have any idea how this can be incorporated into the Home Assistant component?

MatMaul commented 5 years ago

ok no problem ;) Forgot to answer for home assistant, for device_tracker I think it's not made for that, but there is a new concept of Device in recent release and I haven't look if it could fit there.

caffeinatedMike commented 5 years ago

That's what I figured. I'll look into the new concept as well. But, regardless I NEED a way to use this functionality in my automations. It's perfect for so many cases of mine. The best one the comes to mind is:

  1. New Device detected on router
  2. Send Notification to my Android phone with 2 buttons (Allow/Deny)
  3. If Deny clicked HA would block access to the new device/intruder

Edit: If you have a link to the new device concept could you provide it? I can't seem to find anything about it.

MatMaul commented 5 years ago

https://www.home-assistant.io/blog/2018/09/28/release-79/ https://github.com/home-assistant/architecture/issues/36

Not sure yet if it's useful. Doesn't look like there is any way to act on a device. However we could probably have an option to create a switch and create a device containing the device_tracker and the switch.

caffeinatedMike commented 5 years ago

That sounds like the ideal situation. Creating a device containing the device_tracker and the switch. However, we would still need to create the switch component (under the Netgear platform) that allows us to pass the Mac ID and state to said switch. Is that possible?

MatMaul commented 5 years ago

Tested approved and merged, thanks :) Probably possible, to be seen !

caffeinatedMike commented 5 years ago

@MatMaul Glad I could help! Hit me up when you need more help, I enjoy being able to contribute :)

And let's get on that, incorporating this block functionality into Home Assistant somehow, ASAP ;)

MatMaul commented 5 years ago

New version released, PR sent on home-assistant. https://github.com/home-assistant/home-assistant/pull/17652