arcam / CocoaUPnP

CocoaUPnP is a logical progression of upnpx; designed to be easy, modern and block-based.
MIT License
85 stars 40 forks source link

Please add an option to ignore the `NOTIFY` messages on delegate 'discovery:didFindDevice:' #57

Closed funnel20 closed 3 years ago

funnel20 commented 5 years ago

Hi, we're working with your example. When we change the service in timerFired: to a specific one instead of "ssdp:all", still devices with other types are shown.

- (void)timerFired:(NSTimer *)timer
{
    [[UPPDiscovery sharedInstance] startBrowsingForServices:@"urn:schemas-upnp-org:device:MediaServer:1"];
}

According to the docu of discovery:didFindDevice: devices are also reported when sending the Notify message:

Inform class that a new UPnP device was found: either through responding to an M-SEARCH request or from periodic NOTIFY messaging to the multicast address.

Since we're searching for a specific URN, it's inconvenient to also receive detected devices that don't match this URN. Would it be possible to add a property or parameter to ignore devices from the notify message?

squarefrog commented 5 years ago

That should work. When you send out a search with a specific device or service identifier, only those devices should be returned.

funnel20 commented 5 years ago

Hi @squarefrog , thanks for your prompt response. We've tested with multiple device and service identifiers. Even from a custom one that we created ourselves in the device.xml. But besides the requested Service, still the delegate reports a lot of different services from router and NAS.

funnel20 commented 5 years ago

When looking to the code where Notify is received, there is no condition to check whether a specific service was requested, but [self _notifyDelegateWithFoundService:] is always called: https://github.com/arcam/CocoaUPnP/blob/3a4b58a82fa0e3db142f5d1be55a98c6c464aac9/CocoaUPnP/Networking/SSDP/SSDPServiceBrowser.m#L181-L208

squarefrog commented 5 years ago

OK I've just tried this. It does work, but that doesn't mean the devices on your network respect the search target. For example, this is a search with ssdp:all:

Simulator Screen Shot - iPhone 11 - 2019-10-26 at 18 06 57


This is a search with urn:schemas-upnp-org:device:MediaRenderer:1:

Simulator Screen Shot - iPhone 11 - 2019-10-26 at 18 07 46


Notice there are a few devices that _should _not__ have responded, for example the Philips Hue. What I'd do in this case is filter these devices in the delegate method:

func discovery(_ discovery: UPPDiscovery, didFind device: UPPBasicDevice) {
  if device.deviceType != "urn:schemas-upnp-org:device:MediaRenderer:1" {
    return
  }

  // do something with the device
}
funnel20 commented 5 years ago

Can it be the case that they respond to a discovery request from another device? Or will that not be received?

Of course we can filter all devices in the delegate method, but it would be convenient to not receive the Notify devices at all.

squarefrog commented 5 years ago

I don’t think so, the search header includes a return IP address so other searches shouldn’t affect it.

What would affect it is if you do two different searches in two different places, the library will notify the delegate of both.

I agree it would be convenient to not have to filter, but I’m not sure what the more elegant solution would be here.

If we make the discovery object filter these devices based on the last search target this could pose an issue when searching for multiple devices (eg renderers and servers).

What I tend to do in my apps is search for all devices then just filter based on context.

funnel20 commented 5 years ago

Ok, I agree to use the filtering.