miniupnp.tuxfamily.org Forum Index miniupnp.tuxfamily.org
The forum about miniupnp and libnatpmp
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

Better UPNP_GetValidIGD()

 
Post new topic   Reply to topic    miniupnp.tuxfamily.org Forum Index -> miniupnpc Feature Request
View previous topic :: View next topic  
Author Message
zmorris



Joined: 05 Aug 2011
Posts: 4

PostPosted: Thu Sep 01, 2011 9:45 pm    Post subject: Better UPNP_GetValidIGD() Reply with quote

Just a small oversight, I noticed that UPNP_GetValidIGD() doesn't return which device was found, so it makes it difficult after this routine to store things like UPNPDev->descURL in our own classes. Can you guys update the one in miniupnp to work like this? Another option is to have it be struct UPNPDev ** devlist as the first parameter so that it gets set with the iterator that was chosen on exit.

P.S. I had one question, upnpDiscover() doesn't seem to ever find more than one device on my LAN. So for example I have an Actiontec router at 192.168.0.1 with a DishNetwork box hooked up to it at 192.168.0.9, and the only time the DishNetwork box shows up is when the UPnP crashes in the router if I request a mapping with an empty description. I'm still writing my tool to handle multiple routers defensively in case upnpDiscover() starts returning more than one, which is what led me to this issue of associating the proper descURL with the urls/data parameters returned by UPNP_GetValidIGD().

Thanx,

--Zack

Code:
// UPNP_GetValidIGD() doesn't return which upnp was found, so fix it to return the iterator
int      UPNP_GetValidIGD_Better(struct UPNPDev * devlist,
                 struct UPNPUrls * urls,
             struct IGDdatas * data,
             char * lanaddr, int lanaddrlen,
             UPNPDev ** foundDev)
{
   char * descXML;
   int descXMLsize = 0;
   struct UPNPDev * dev;
   int ndev = 0;
   int state; /* state 1 : IGD connected. State 2 : IGD. State 3 : anything */
   if(!devlist)
   {
#ifdef DEBUG
      printf("Empty devlist\n");
#endif
      return 0;
   }
   for(state = 1; state <= 3; state++)
   {
      for(dev = devlist; dev; dev = dev->pNext)
      {
         /* we should choose an internet gateway device.
          * with st == urn:schemas-upnp-org:device:InternetGatewayDevice:1 */
         descXML = (char*) miniwget_getaddr(dev->descURL, &descXMLsize,
                                       lanaddr, lanaddrlen);
         if(descXML)
         {
            ndev++;
            memset(data, 0, sizeof(struct IGDdatas));
            memset(urls, 0, sizeof(struct UPNPUrls));
            parserootdesc(descXML, descXMLsize, data);
            free(descXML);
            descXML = NULL;
            if(0==strcmp(data->CIF.servicetype,
               "urn:schemas-upnp-org:service:WANCommonInterfaceConfig:1")
               || state >= 3 )
            {
              GetUPNPUrls(urls, data, dev->descURL);

#ifdef DEBUG
              printf("UPNPIGD_IsConnected(%s) = %d\n",
                 urls->controlURL,
                  UPNPIGD_IsConnected(urls, data));
#endif
              if((state >= 2) || UPNPIGD_IsConnected(urls, data))
              {
               *foundDev = dev;
               return state;
              }
              FreeUPNPUrls(urls);
              if(data->second.servicetype[0] != '\0') {
#ifdef DEBUG
                printf("We tried %s, now we try %s !\n",
                       data->first.servicetype, data->second.servicetype);
#endif
                /* swaping WANPPPConnection and WANIPConnection ! */
                memcpy(&data->tmp, &data->first, sizeof(struct IGDdatas_service));
                memcpy(&data->first, &data->second, sizeof(struct IGDdatas_service));
                memcpy(&data->second, &data->tmp, sizeof(struct IGDdatas_service));
                GetUPNPUrls(urls, data, dev->descURL);
#ifdef DEBUG
                printf("UPNPIGD_IsConnected(%s) = %d\n",
                   urls->controlURL,
                    UPNPIGD_IsConnected(urls, data));
#endif
                if((state >= 2) || UPNPIGD_IsConnected(urls, data))
               {
                 *foundDev = dev;
                 return state;
               }
                FreeUPNPUrls(urls);
              }
            }
            memset(data, 0, sizeof(struct IGDdatas));
         }
#ifdef DEBUG
         else
         {
            printf("error getting XML description %s\n", dev->descURL);
         }
#endif
      }
   }
   return 0;
}
[/code]
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Sat Sep 10, 2011 6:39 am    Post subject: Reply with quote

It is useless to know which element of devlist is "choosen" because all usefull data is copied to the struct UPNPUrls * urls and struct IGDdatas * data structures...
Indeed, dev->descURL is not, but if needed, we could add a rootdescURL field to the UPNPUrls structure.

Code:
/* UPNP_GetValidIGD() :
 * return values :
 *     0 = NO IGD found
 *     1 = A valid connected IGD has been found
 *     2 = A valid IGD has been found but it reported as
 *         not connected
 *     3 = an UPnP device has been found but was not recognized as an IGD
 *
 * In any non zero return case, the urls and data structures
 * passed as parameters are set. Donc forget to call FreeUPNPUrls(urls) to
 * free allocated memory.
 */

_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
zmorris



Joined: 05 Aug 2011
Posts: 4

PostPosted: Sat Sep 10, 2011 11:59 pm    Post subject: Reply with quote

Ya I needed the descURL of the one chosen so that I could parse it for icons and other optional URLs. But since there was no way to determine which device was used to fill the UPNPUrls struct, the existing routine was fairly useless to me. I was thinking that returning which device was chosen (like an iterator) would be simplest, but your idea of just putting the descURL right into the UPNPUrls struct is much better. Then we could use that as a unique key to determine which device was chosen.

So anyway, I know this seems like a small thing, but every little bit makes the library that much easier to use.

P.S. The XML parser is also quite handy and I used it to pull every major field like manufacturer and serial from the description.

Thanx,

--Zack
Back to top
View user's profile Send private message
ghuxe885sbh
Guest





PostPosted: Tue Sep 20, 2011 11:59 am    Post subject: Reply with quote

many thanks,here is some sites for your reference:
Back to top
Danny



Joined: 16 May 2012
Posts: 1

PostPosted: Wed May 16, 2012 11:42 am    Post subject: Reply with quote

Hi There

I just wanted to make a suggestion here which you may find useful.
I noticed in the "upnpurns.h" header, that when searching for an IGD you use the URN types:
Code:

"urn:schemas-upnp-org:device:InternetGatewayDevice:1"

or
Code:

"urn:schemas-upnp-org:device:InternetGatewayDevice:2"


This is correct and is the UPnP standard, but i have noticed that there are many IGD's out there that do not comply with this standard, for example a D-Link DSL-2640R will have a URN type as:
Code:

urn:schemas-upnp-org:device:MediaServer:1

Unfortunately you are going to miss these crafty devils, so just as an idea, it may be worth searching for all available 'UPnP' devices using the URN type:
Code:

upnp:rootdevice

And then parse the 'Location' field from each response, (IGD location field should always contain the default gateway address) do a string comparison against the default gateway address on the client via the contents of '/proc/net/route' and then attempt to connect to it over TCP. A bit messy i know, but this method should allow you to find any non-compliant UPnP IGD's.

I hope this may be of some help.

-Danny
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Wed May 16, 2012 8:00 pm    Post subject: Reply with quote

miniupnpc does already search for upnp:rootdevice if no IGD anwser to urn:schemas-upnp-org:device:InternetGatewayDevice:1

It then tries to check if the IGD is connected to the internet.

filtering out UPnP devices which are not on the same IP as the default gateway is the strategy chosen by windows. But it pauses problem with some setups, so I'm not likely to do it.
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
Display posts from previous:   
Post new topic   Reply to topic    miniupnp.tuxfamily.org Forum Index -> miniupnpc Feature Request All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group
Protected by Anti-Spam ACP
© 2007 Thomas Bernard, author of MiniUPNP.