ofekp / TinyUPnP

A very small UPnP IGD implementation for your ESP8266 for automatic port forwarding
GNU Lesser General Public License v2.1
104 stars 15 forks source link

Implementation for knowing what all services are present on the network #61

Closed Gungeoneer closed 4 years ago

Gungeoneer commented 4 years ago

Hi, I am using your library along with the Arduino for ESP8266 SDK. I was working with the example SimpleServer from your library. However, I am unable to see my other SSDP Devices in the logs from the example. Do I need to make any changes in the code to be able to do that?

ofekp commented 4 years ago

This is probably what you'll have to change, I am only looking for IGD devices https://github.com/ofekp/TinyUPnP/blob/master/src/TinyUPnP.cpp#L529

You can consult with a branch I currently have but not merged to understand how to fire multiple M-SEARCH queries or you can use a wide deviceType with a single M-SEARCH query (all). Note that it is better to narrow it as much as you can. You can narrow based on the response you get from the device which should include the correct deviceType.

Gungeoneer commented 4 years ago

Thank you very much for your quick reply :D

I did try that branch as you have suggested. I am using the SSDP example from the Arudino SDK for ESP8266 which registers a device on the network as a Philips Hue Clone. I am able to see that device as well as another device on in the Networks folder on Windows 10. However, I am unable to find them in the debug logs.

Given how the branch is not yet merged, is this feature still a work in progress? I am looking to implement a SSDP listener so I can get a list of any devices on the network. If I can understand what you have suggested correctly, I can fire a single M-SEARCH query for all devices and it will list down whatever devices are on the network?

ofekp commented 4 years ago

Instead of using the branch (I am not sure I will ever merge that), can you try changing the line https://github.com/ofekp/TinyUPnP/blob/master/src/TinyUPnP.cpp#L529 to strcat_P(body_tmp, PSTR("ST: ssdp:all\r\n\r\n"));

Gungeoneer commented 4 years ago

Thanks for your continued guidance and assistance!

Okay, I have tried what you suggested. I have also removed the return false which stops the program from proceeding if any device other than an Internet Gateway Device is found.

However, despite this I am only able to see communication between the device and the Router. At the end I get the message saying that all ports and devices were discovered without my device ever showing up. For additional reference, my device has the following property:

SSDP.setDeviceType("upnp:rootdevice");

Are there any additional changes that would be required to show such types of devices?

ofekp commented 4 years ago

Okay, I have tried what you suggested. I have also removed the return false which stops the program from proceeding if any device other than an Internet Gateway Device is found.

Nice, I actually forgot about that one :)

Can you try to replace the same line with this: strcat_P(body_tmp, PSTR("ST: upnp:rootdevice\r\n\r\n"));

Please also make sure you are running the code you intend to run. You can add a compilation error on purpose and compile to see if your changes are actually being used.

By seeing communication with the router, you mean you can see in the debug logs the response to the M-SEARCH message which contains, among other headers, the Location header, is this correct?

For the device you are setting up with SSDP.setDeviceType("upnp:rootdevice"); Is it listening on port 1900 UDP?

Do you have access to a linux machine? Can you use something like this maybe? to see if the device responds? I have not tried it myself, if you find a better way to check for all upnp devices in your network please let me know what you found.

Gungeoneer commented 4 years ago

Do you have access to a linux machine? Can you use something like this maybe? to see if the device responds?

Through gssdp, I am able to see all the devices in my network on Linux. I can also selectively target specific device types to show only the ones that I need.

For the device you are setting up with SSDP.setDeviceType("upnp:rootdevice"); Is it listening on port 1900 UDP?

Yes, it is listening on port 1900

I have not tried it myself, if you find a better way to check for all upnp devices in your network please let me know what you found.

I think the tutorial you linked for gssdp is the best way to discover all devices, thanks a lot for the help :)

ofekp commented 4 years ago

I would like to understand if you were able to find the device using TinyUPnP, if so what did you do. If you no longer need it, can I close the issue?

BTW, I found that there are two places where I filter based on the assumption that I the response should be received from an IGD device, did you remove both of them when trying the package?

  1. waitForUnicastResponseToMSearch - if (remoteIP != gatewayIP) {
  2. waitForUnicastResponseToMSearch - if (strstr(responseBuffer, INTERNET_GATEWAY_DEVICE) == NULL) {
Gungeoneer commented 4 years ago

Hey ofekp, I really appreciate your quick responses and all the help which you have provided to me. I wasn't able to reply in the past few days due to some work issues.

If you no longer need it, can I close the issue?

Yes, I would like for you to close the issue, I have found the solution to my problem.

BTW, I found that there are two places where I filter based on the assumption that I the response should be received from an IGD device, did you remove both of them when trying the package? waitForUnicastResponseToMSearch - if (remoteIP != gatewayIP) { waitForUnicastResponseToMSearch - if (strstr(responseBuffer, INTERNET_GATEWAY_DEVICE) == NULL) {

Yes, I did remove both of these as well as what we had previously discussed in order to make sure all the devices were visible. It works like a charm now :)

I would like to understand if you were able to find the device using TinyUPnP, if so what did you do.

If you would like to see exactly the parts that I changed, I can make a pull request which you can leave as it is or I can send you a diff file with my changes.

ofekp commented 4 years ago

Awesome, I am happy it worked :) and NP I would love to see what you change for the purpose of being able to help in the future, I might also incorporate this into the code with a flag or simply reference to this change from the README should someone need this in the future. So, yeah, if you can please share either a diff or a PR 👍 Thank you.

Gungeoneer commented 4 years ago

Couldn't upload a diff file directly so here is a text file which you can change the extension of: ChangesForAllDevices.txt

ofekp commented 4 years ago

Beautiful, thank you :) I will find a way to make it available for others in the README or via a flag 👍 Closing this for now.

ofekp commented 4 years ago

FYI, I just pushed this PR which gives a dedicated method to list all SSDP devices based on on the diff you provided (but as a function). I also provided an example to show how to use it. Thank you.