search

michielpost / Q42.HueApi

C# helper library to talk to the Philips Hue bridge
MIT License
412 stars 114 forks source link

Bridge locator : Add cancellationToken interface and Improve SsdpBridgeLocator #200

Closed Indigo744 closed 4 years ago

Indigo744 commented 4 years ago

Hello,

Continuing the work on #198

This PR brings two enhancements:

  1. Add a new interface to the IBridgeLocator accepting a cancellationToken. 
    Task<IEnumerable<LocatedBridge>> LocateBridgesAsync(CancellationToken cancellationToken)

    It allows users to provide a cancel mechanism different from a simple timeout. As a matter of facts, the old signature with the timeout simply use a cancellationToken with a timeout!

  2. Improve the SsdpBridgeLocator reliability on Windows After a lot of tests in our lab, it became quite clear that the SSDP protocol was not reliable. Sometimes it would find instantly a bridge, sometimes not. Using Wireshark, we found out that when the bridge wasn't found, it was because the system actually never sent the discovery packet. It was baffling. After some searching, we found someone talking about a very specific issue on Windows linked to the SSDP Discovery Service. As suggested, we tried binding the socket directly to the interface IP (instead of all interface using Any) and it worked flawlessly every time! So this is why we have to check for the OS, and perform a lookup on all private interfaces in order to use them to bind. I've also modified the discovery message by setting MX: 1 which is suppose to be the max delay to reply for the device.

As a side note, I am also working on a LocalNetworkBridgeLocator (IP lookup) for which a different PR will be provided. It will also make use of the new IPAddressExtensions.

If I find the time, I will also look into the MDNS protocol since it is the recommended approach by Philips. I'll keep you posted.

Of course, I am open to code review / remarks.

niels9001 commented 4 years ago

This is awesome :-)! Keep up the great work!

michielpost commented 4 years ago

Thanks a lot! I'm unable to discover my home bridge with the included unit test (current code and PR code), so no change there. I did discover my home bridge using the UWP sample app and the current code on master, but I'm unable to run the UWP app at the computer I'm on right now.

I can do some more testing next week.

I'll merge this PR now and I've also added you to the list of contributors. Let me know if you need a new release on NuGet that includes this PR.

Indigo744 commented 4 years ago

@michielpost interesting. Keep me posted if I can help you diagnose the issue.

I don't think a Nuget release is needed for the moment. Maybe wait for my next PR and we'll see.

Thanks.