search

MatMaul / pynetgear

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

Check login with an info call #90

Closed starkillerOG closed 2 years ago

starkillerOG commented 2 years ago

Sometimes a v1 login call can report succes while actually not beeing autorized to get info, v2 login does work in that case. See: https://github.com/home-assistant/core/issues/59025 as an example.

This will check the V1 login call with a get_info call, if that fails, v2 login is automatically tried.

starkillerOG commented 2 years ago

@MatMaul could you merge this PR?

MatMaul commented 2 years ago

Hi there ! I am thinking about making v2 login the default and make v1 the fallback instead of the reverse, to match the behavior of the node.js version, which seems to behave better, cf https://github.com/MatMaul/pynetgear/issues/62#issuecomment-496873222

I think this would also solve your trouble.

MatMaul commented 2 years ago

@starkillerOG could you try #93 please ?

I don't own any Netgear devices with stock firmware anymore (OpenWRT :) ).

I am also looking for a maintainer if you are interested ;) .

starkillerOG commented 2 years ago

@MatMaul thanks for your response! Yes of course I will try #93, might take some time before I get around to it, but will certainly try within a few weeks.

I am currently in contact with Netgear and discussing the HomeAssistant integration. If they will help me out with the appropriate API documentation etc, I am certanly willing to become a maintainer. Would be greath if I could get merge rights etc for this library.

I schould know within a few weeks if Netgear is willing to cooperate.

MatMaul commented 2 years ago

Great to hear :) let me know, since I can't test I am waiting your test before merging.

starkillerOG commented 2 years ago

@MatMaul sorry for taking so long. I just tested #93 and it works like a charm, no problems on my R7000. So I think that can be merged first, I will rebase this PR on top of that one.

starkillerOG commented 2 years ago

This is tested and ready to be merged.

starkillerOG commented 2 years ago

@MatMaul could you merge this and release a new version?

gruijter commented 2 years ago

I am currently in contact with Netgear and discussing the HomeAssistant integration. If they will help me out with the appropriate API documentation etc, I am certanly willing to become a maintainer.

Any news on this @starkillerOG ? It would be awesome if Netgear is providing official API documentation.

starkillerOG commented 2 years ago

I am currently in contact with Netgear and discussing the HomeAssistant integration. If they will help me out with the appropriate API documentation etc, I am certanly willing to become a maintainer.

Any news on this @starkillerOG ? It would be awesome if Netgear is providing official API documentation.

Still in contact with them, hopping around between departments of Netgear, but they are not to quick to answer. Currently the engineering teams is discussing it internally.

MatMaul commented 2 years ago

@starkillerOG 2 small comments, and indeed I'll cut a release after merging that. Thanks for your work and tests.

starkillerOG commented 2 years ago

@MatMaul I made the 2 small changes, could you look at it again?

MatMaul commented 2 years ago

LGTM, thanks !

MatMaul commented 2 years ago

@starkillerOG I have released 0.8.0 on GitHub and PyPI, can you update homeassistant with it ?

Thanks !

MatMaul commented 2 years ago

@starkillerOG you also have been added as a contributor to this repo. Please send me your pypi login and I'll take care to put you there too so you can do a release, if you don't mind.

starkillerOG commented 2 years ago

@starkillerOG you also have been added as a contributor to this repo. Please send me your pypi login and I'll take care to put you there too so you can do a release, if you don't mind.

Thank you that is very helpfull. My pypi user name is: StarkillerOG

Do you want me to give you a few days to revieuw before I merge code I wrote myself or can I just merge emediatly if I made changes that I have tested myself?

starkillerOG commented 2 years ago

@starkillerOG I have released 0.8.0 on GitHub and PyPI, can you update homeassistant with it ?

Thanks !

Thanks, I will make a HomeAssistant PR now.

MatMaul commented 2 years ago

Do you want me to give you a few days to revieuw before I merge code I wrote myself or can I just merge emediatly if I made changes that I have tested myself?

I propose that we both do it through PRs with a mention to the other, and if no answer in 3/4 days we just merge. Works for you?

starkillerOG commented 2 years ago

Sounds good to me 👍

starkillerOG commented 2 years ago

HA PR has already been merged and will be included in 2021.12.4 which will probably be released in a few days. https://github.com/home-assistant/core/pull/62261

starkillerOG commented 2 years ago

@starkillerOG you also have been added as a contributor to this repo. Please send me your pypi login and I'll take care to put you there too so you can do a release, if you don't mind.

@MatMaul @balloob I just figured out during an attempt to upload the new pynetgear version 0.9.0 that I don't actually have been added to the pypi maintainers list....

I get this when I try to upload:

HTTPError: 403 Forbidden from https://upload.pypi.org/legacy/
The user 'StarkillerOG' isn't allowed to upload to project 'pynetgear'. See https://pypi.org/help/#project-name for more information.

Could one of you at me to the maintainters list so I can finish the upload?