Skip to content
This repository has been archived by the owner on Dec 8, 2023. It is now read-only.

Allow WANIPConnection:2 in parse_service() #51

Merged
merged 1 commit into from
Jun 6, 2020

Conversation

kayru
Copy link
Contributor

@kayru kayru commented Jun 6, 2020

Some routers only advertise WANIPConnection v2 and not v1. In this case parse_service() simply returns None which eventually causes search_gateway() to fail.

http://upnp.org/specs/gw/UPnP-gw-WANIPConnection-v2-Service.pdf
Section 1.2 specifies that WANIPConnection v2 is compatible with v1.

Allowing WANIPConnection:2 makes the library work correctly with wider range of routers (in particular, AmpliFi HD now works correctly).

This addresses one of the issues described in #12

Some routers only advertise WANIPConnection v2 and not v1. In this case
parse_service() simply returns None which eventually causes
search_gateway() to fail.

http://upnp.org/specs/gw/UPnP-gw-WANIPConnection-v2-Service.pdf
Section 1.2 specifies that WANIPConnection v2 is compatible with v1.

Allowing WANIPConnection:2 makes the library work correctly with wider
range of routers (in particular, AmpliFi HD now works correctly).
@sbstp
Copy link
Owner

sbstp commented Jun 6, 2020

Should we also add urn:schemas-upnp-org:service:WANPPPConnection:2 by that logic?

@kayru
Copy link
Contributor Author

kayru commented Jun 6, 2020

I thought about that and it should be fine, however I am not in a position to actually test it. My router advertises only the following services: https://gist.github.com/kayru/ce569fac7b86d65b03f1e629ab79a493
Since I can't test it, I did not want to change more than strictly necessary. Though I have no strong objections to doing so.

@sbstp
Copy link
Owner

sbstp commented Jun 6, 2020

I'm looking at the spec right now, and there's no mention of WANPPPConnection:2.

@sbstp sbstp merged commit 2a8a196 into sbstp:master Jun 6, 2020
@sbstp
Copy link
Owner

sbstp commented Jun 6, 2020

Thanks for the MR, I release 0.11.1 with this fix.

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

Successfully merging this pull request may close these issues.

2 participants