Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mdns: Fix/Update Windows mdns code #837

Merged
merged 1 commit into from
Jun 29, 2022

Conversation

rgetz
Copy link
Contributor

@rgetz rgetz commented Apr 6, 2022

This syncs the application side with the updates in the mdns.h file
from: https://github.com/mjansson/mdns/
and does things according to the specs. (build up info from both A, AAAA and
SRV records), not just assuming that the DNS server is the host.

Signed-off-by: Robin Getz <[email protected]>

@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch 3 times, most recently from 1204601 to b336afb Compare April 7, 2022 00:20
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch from b336afb to a4c0551 Compare April 8, 2022 00:16
@rgetz
Copy link
Contributor Author

rgetz commented Apr 8, 2022

figured out there is still a problem on Windows/private networks - so this isn't ready to be merged yet. I must have missed something in copying things over from the other branch...

@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch 4 times, most recently from d39f914 to 1c38bb7 Compare April 24, 2022 21:53
@rgetz
Copy link
Contributor Author

rgetz commented Apr 24, 2022

Codacy complains that we are keeping track of something, and not using it (but we do use it in an IIO_DEBUG() function, it's just compiled out at this time)...

So, ignore Codacy....

@rgetz
Copy link
Contributor Author

rgetz commented Apr 24, 2022

This patch relies on #838 - once that is committed - I will rebase on master and try binaries from CI - otherwise - it is ready to go/be reviewed.

-Robin

@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch from 1c38bb7 to 905d6ca Compare April 25, 2022 12:32
@rgetz
Copy link
Contributor Author

rgetz commented Apr 25, 2022

Can confirm (finally) that this works on private/local links (phaser.local is on a cross over cable, with a unconfigured network) on master.

C:\WINDOWS\system32>iio_info -S
Library version: 0.23 (git tag: dcf65e4)
Compiled with backends: xml ip usb serial
Available contexts:
        0: 192.168.1.120 (ad7291,ad9361-phy,xadc,adf4351-udc-tx-pmod,adf4351-udc-rx-pmod,cf-ad9361-dds-core-lpc,cf-ad9361-lpc) [ip:analog.local]
        1: 2601:190:400:da:47b3:55ab:3914:bff1 (ad7291,ad9361-phy,xadc,adf4351-udc-tx-pmod,adf4351-udc-rx-pmod,cf-ad9361-dds-core-lpc,cf-ad9361-lpc) [ip:analog.local]
        2: fe80::cb1:f255:a133:e55b (cpu_thermal,rpi_volt,one-bit-adc-dac,ad7291,adf4159,adar1000_1,adar1000_0) [ip:phaser.local]

and then:

C:\WINDOWS\system32>iio_attr.exe -u ip:phaser.local -d
IIO context has 8 devices:
        hwmon0, cpu_thermal: found 0 device attributes
        hwmon1, rpi_volt: found 0 device attributes
        iio:device0, gpios: found 0 device attributes
        iio:device1, housekeeping_adc: found 0 device attributes
        iio:device2, pll0: found 0 device attributes
        iio:device3, BEAM1: found 39 device attributes
        iio:device4, BEAM0: found 39 device attributes
        iio_sysfs_trigger: found 2 device attributes

C:\WINDOWS\system32>iio_attr.exe -u ip:fe80::cb1:f255:a133:e55b -d
IIO context has 8 devices:
        hwmon0, cpu_thermal: found 0 device attributes
        hwmon1, rpi_volt: found 0 device attributes
        iio:device0, gpios: found 0 device attributes
        iio:device1, housekeeping_adc: found 0 device attributes
        iio:device2, pll0: found 0 device attributes
        iio:device3, BEAM1: found 39 device attributes
        iio:device4, BEAM0: found 39 device attributes
        iio_sysfs_trigger: found 2 device attributes

Copy link
Contributor

@pcercuei pcercuei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cosmetic changes needed.

dns_sd_windows.c Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch from 905d6ca to 34a7b2a Compare April 25, 2022 14:56
@rgetz
Copy link
Contributor Author

rgetz commented Apr 25, 2022

Updated - and still works :)

C:\WINDOWS\system32>iio_info -S
Library version: 0.23 (git tag: bcffeb5)
Compiled with backends: xml ip usb serial
Available contexts:
        0: 192.168.1.120 (ad7291,ad9361-phy,xadc,adf4351-udc-tx-pmod,adf4351-udc-rx-pmod,cf-ad9361-dds-core-lpc,cf-ad9361-lpc) [ip:analog.local]
        1: 2601:190:400:da:47b3:55ab:3914:bff1 (ad7291,ad9361-phy,xadc,adf4351-udc-tx-pmod,adf4351-udc-rx-pmod,cf-ad9361-dds-core-lpc,cf-ad9361-lpc) [ip:analog.local]
        2: fe80::cb1:f255:a133:e55b (cpu_thermal,rpi_volt,one-bit-adc-dac,ad7291,adf4159,adar1000_1,adar1000_0) [ip:phaser.local]

dns_sd_windows.c Outdated Show resolved Hide resolved
dns_sd_windows.c Outdated Show resolved Hide resolved
@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch from 34a7b2a to 5540e28 Compare April 25, 2022 15:32
@rgetz
Copy link
Contributor Author

rgetz commented Apr 25, 2022

Ok - still a bug if a device is serving up different ports... (currently sees only one).

digging into that now.

dns_sd_windows.c Outdated
@@ -20,6 +20,10 @@
#include "iio-private.h"
#include "deps/mdns/mdns.h"

#define _STRINGIFY(x) #x
#define STRINGIFY(x) _STRINGIFY(x)
#define MDNS_PORT_STR _STRINGIFY(MDNS_PORT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a few days to debug... But the fix is literally removing one character:

#define MDNS_PORT_STR STRINGIFY(MDNS_PORT)

One thing I wonder, is the usefulness to list scanned contexts on both IPv6 and IPv4; because afterwards, there is no way to specify which to use (they both would list the URI as ip:analog.local).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed on the branch, just testing to see if this resolves things.

@rgetz
Copy link
Contributor Author

rgetz commented May 31, 2022

What does that “fix”? I can see that’s a problem - and needs to be updated - but I don’t think that will fix the problem I was seeing….

As for ipv4 vs ipv6 - yes, if you use them by name, it doesn’t help - which is why it always displays the IP number at the same time. It has always been this way, and doesn’t have anything to do with the window implementation.

@pcercuei
Copy link
Contributor

@rgetz MDNS_PORT_STR evaluated to "MDNS_PORT" instead of "5353".

@rgetz
Copy link
Contributor Author

rgetz commented May 31, 2022

Yes - like I said - that needs to be fixed, but I don't think that solves the problem I was seeing (ipv4 and ipv6 with multiple instances of iiod running on different ports).

@pcercuei
Copy link
Contributor

Well I can assure you it does. I obviously tested it.

@rgetz
Copy link
Contributor Author

rgetz commented May 31, 2022

Paul:

on private network (cross over cable, no IP config) or on DHCP configured network? With and without VPN turned on? Getting all things working was the part I was having issues with.

@pcercuei
Copy link
Contributor

On a DHCP configured network with VPN turned off.

@pcercuei
Copy link
Contributor

It works with ADI's VPN turned ON as well.

@rgetz
Copy link
Contributor Author

rgetz commented May 31, 2022

And private networks? cross over cable to RPi (That is where I was having most issues).

@pcercuei
Copy link
Contributor

My Windows laptop has no Ethernet port unfortunately :(

@rgetz
Copy link
Contributor Author

rgetz commented May 31, 2022

I was using usb <-> Ethernet dongle to test.

This syncs the application side with the updates in the mdns.h file
from: https://github.com/mjansson/mdns/
and does things according to the specs. (build up info from both A, AAAA and
SRV records), not just assuming that the DNS server is the host.

Signed-off-by: Robin Getz <[email protected]>
@rgetz rgetz force-pushed the rgetz-make-win-mdns-work-again branch from 5540e28 to 8453d92 Compare June 6, 2022 13:10
@pcercuei
Copy link
Contributor

@rgetz ready to merge?

@mhennerich
Copy link
Contributor

@rgetz ready to merge?

Robin is on vacation and might not respond.
I think this is better what we ever had before. And this PR is holding off the release for so long now.
I'll merge it and we do the release.
In case there is still something to fix/enhance we can do another minor follow up release.
Or we address it in libiio1.0

-Michael

@mhennerich mhennerich merged commit 3db19bc into master Jun 29, 2022
@mhennerich mhennerich deleted the rgetz-make-win-mdns-work-again branch June 29, 2022 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants