-
Notifications
You must be signed in to change notification settings - Fork 7
support PPP devices with slightly different XML schema #2
base: master
Are you sure you want to change the base?
Conversation
e.g. common TG588v routers from Technicolor
@@ -56,7 +56,7 @@ discover1(Sock, MSearch, Timeout, Tries) -> | |||
ok = gen_udp:send(Sock, "239.255.255.250", 1900, MSearch), | |||
receive | |||
{udp, _Sock, Ip, _Port, Packet} -> | |||
case get_location(Packet) of | |||
case get_location(http_bin, Packet) of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hrm why do you start by reading the full response? Sounds like it will break other parsing but I may be mistaken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add some tests for each device type I think.
Main reason for this is that just doing httph_bin as before doesn't work. I did wonder if your devices have a different setup and so my change breaks yours, but I figured at least a simple patch to get started.
The "router" is a typically crappy TGv588 (Thomson) router, this firmware is sadly very common across Europe.
%% binary is Packet from #L59 above
{ok, Context} = nat_upnp:discover().
<<72,84,84,80,47,49,46,49,32,50,48,48,32,79,75,13,10,67,65,67,72,69,45,67,79,78,84,82,79,76,58,109,97,120,45,97,103,101,61,49,56,48,48,13,10,69,88,84,58,13,10,76,79,67,65,84,73,79,78,58,104,116,116,112,58,47,47,49,48,46,48,46,48,46,49,51,56,58,56,48,48,48,47,119,110,53,57,109,116,100,100,120,104,105,47,73,71,68,47,117,112,110,112,47,73,71,68,46,120,109,108,13,10,83,69,82,86,69,82,58,77,101,100,105,97,65,99,99,101,115,115,32,84,71,32,53,56,56,118,32,49,48,46,53,46,52,46,87,32,85,80,110,80,47,49,46,48,32,40,57,67,45,57,55,45,50,54,45,56,67,45,70,50,45,51,52,41,13,10,83,84,58,117,114,110,58,115,99,104,101,109,97,115,45,117,112,110,112,45,111,114,103,58,100,101,118,105,99,101,58,73,110,116,101,114,110,101,116,71,97,116,101,119,97,121,68,101,118,105,99,101,58,49,13,10,85,83,78,58,117,117,105,100,58,50,53,49,51,51,98,98,50,45,55,57,52,101,45,53,49,50,102,45,57,101,49,98,45,49,102,52,55,49,49,100,49,57,48,98,55,58,58,117,114,110,58,115,99,104,101,109,97,115,45,117,112,110,112,45,111,114,103,58,100,101,118,105,99,101,58,73,110,116,101,114,110,101,116,71,97,116,101,119,97,121,68,101,118,105,99,101,58,49,13,10,13,10>>
{ok,{nat_upnp,"http://10.0.0.138:8000/wn59mtddxhi/IGD/upnp/control/igd/wanpppc_1_4_1",
"10.0.0.1"}}
Thanks for the patch! I didn't have time yet to test with the hardware here to see if it still work. Btw can you tell me on which device could I test the change above? I will try to find one :) |
end. | ||
|
||
get_service_url(D, Urn, RootUrl) -> | ||
case get_service(D, Urn) of | ||
{ok, S} -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there may well be a better approach than nested case here - any better suggestion?
So I had a closer look at the issue. There are 2 versions of the IGW UPNP protocol. I will add the support for the second one ASAP. I let this PR open for now to track the issue. |
see below for gory details.