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

Check login with an info call #90

Merged
merged 7 commits into from
Dec 18, 2021
Merged

Check login with an info call #90

merged 7 commits into from
Dec 18, 2021

Conversation

starkillerOG
Copy link
Collaborator

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

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

Sometimes a v1 login call can report succes while actually not beeing autorized to get info, v2 login does work in that case.
See: home-assistant/core#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
Copy link
Collaborator Author

@MatMaul could you merge this PR?

@MatMaul
Copy link
Owner

MatMaul commented Nov 21, 2021

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 #62 (comment)

I think this would also solve your trouble.

@MatMaul
Copy link
Owner

MatMaul commented Nov 21, 2021

@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
Copy link
Collaborator Author

@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
Copy link
Owner

MatMaul commented Nov 22, 2021

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

@starkillerOG
Copy link
Collaborator Author

@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 starkillerOG changed the title Check v1 login with an info call Check login with an info call Dec 14, 2021
@starkillerOG
Copy link
Collaborator Author

This is tested and ready to be merged.

@starkillerOG
Copy link
Collaborator Author

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

@gruijter
Copy link

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
Copy link
Collaborator Author

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.

pynetgear/__init__.py Show resolved Hide resolved
pynetgear/__init__.py Outdated Show resolved Hide resolved
@MatMaul
Copy link
Owner

MatMaul commented Dec 16, 2021

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

@starkillerOG
Copy link
Collaborator Author

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

@MatMaul
Copy link
Owner

MatMaul commented Dec 18, 2021

LGTM, thanks !

@MatMaul MatMaul merged commit c7962d3 into MatMaul:main Dec 18, 2021
@MatMaul
Copy link
Owner

MatMaul commented Dec 18, 2021

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

Thanks !

@MatMaul
Copy link
Owner

MatMaul commented Dec 18, 2021

@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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

@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
Copy link
Owner

MatMaul commented Dec 18, 2021

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
Copy link
Collaborator Author

Sounds good to me 👍

@starkillerOG
Copy link
Collaborator Author

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

@starkillerOG
Copy link
Collaborator Author

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants