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

Send SearchStarted events with addrs as ip (intf-name) #245

Merged
merged 5 commits into from
Aug 21, 2024

Conversation

hrzlgnm
Copy link
Contributor

@hrzlgnm hrzlgnm commented Aug 20, 2024

Otherwise the output gets pretty cluttered when running the query example

With the change to Map(Interface, Socket) in #242
The outpout looks a bit odd and too verbose when expecting addrs:

cargo run --example=query _services._dns-sd._udp
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.31s
     Running `target/debug/examples/query _services._dns-sd._udp`
At 1.320972ms : SearchStarted("_services._dns-sd._udp.local. on addrs [Interface { name: \"br-1a301212c996\", addr: V4(Ifv4Addr { ip: 192.168.49.1, netmask: 255.255.255.0, broadcast: Some(192.168.49.255) }), index: Some(3) }, Interface { name: \"ens33\", addr: V4(Ifv4Addr { ip: 192.168.178.80, netmask: 255.255.255.0, broadcast: Some(192.168.178.255) }), index: Some(2) }, Interface { name: \"ens33\", addr: V6(Ifv6Addr { ip: fe80::c9f4:d01d:e4ca:1296, netmask: ffff:ffff:ffff:ffff::, broadcast: None }), index: Some(2) }, Interface { name: \"docker0\", addr: V4(Ifv4Addr { ip: 172.17.0.1, netmask: 255.255.0.0, broadcast: Some(172.17.255.255) }), index: Some(4) }, Interface { name: \"ens33\", addr: V6(Ifv6Addr { ip: 2003:e8:bf4e:c900:9553:51cc:c0bc:34b9, netmask: ffff:ffff:ffff:ffff::, broadcast: None }), index: Some(2) }, Interface { name: \"ens33\", addr: V6(Ifv6Addr { ip: 2003:e8:bf4e:c900:f1a9:80a6:bdf6:5311, netmask: ffff:ffff:ffff:ffff::, broadcast: None }), index: Some(2) }]")

This change brings back the output like it was before:

cargo run --example=query _services._dns-sd._udp
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/examples/query _services._dns-sd._udp`
At 1.107118ms : SearchStarted("_services._dns-sd._udp.local. on addrs [172.17.0.1, 192.168.178.80, 2003:e8:bf4e:c900:9553:51cc:c0bc:34b9, 192.168.49.1, 2003:e8:bf4e:c900:f1a9:80a6:bdf6:5311, fe80::c9f4:d01d:e4ca:1296]")

Update to <ip> (<intf-name>), where i also aligned the query example output of other events with ServiceResolved()

At 1.074132ms: SearchStarted("_fbox._tcp.local. on addrs [192.168.73.130 (eno16777736), 192.168.178.76 (eno33554984), fe80::e389:cd5c:6aa4:50e3 (eno33554984), fe80::f3fc:8e58:38e3:ff2b (eno16777736), 2003:e8:bf4e:c900:4f05:cc9:27e5:48f3 (eno33554984)]")

Otherwise the output gets pretty cluttered when running the query example
@hrzlgnm hrzlgnm changed the title Send SearchStarted events with ip addrs again Send SearchStarted events with ip addrs Aug 20, 2024
@hrzlgnm
Copy link
Contributor Author

hrzlgnm commented Aug 20, 2024

An alternative would be to print interface names. But therefore we would need to update the if_addrs crate. As the currently used version only would print he GUID instead of the friendly name of the interface when running on windows. That change was introduced with messense/if-addrs#34 which is available since https://crates.io/crates/if-addrs/0.12.0, we are using https://crates.io/crates/if-addrs/0.10.2

@keepsimple1
Copy link
Owner

keepsimple1 commented Aug 20, 2024

Thanks for this PR!

An alternative would be to print interface names. But therefore we would need to update the if_addrs crate. As the currently used version only would print he GUID instead of the friendly name of the interface when running on windows.

In this example code, I think maybe we could print both IP address and the friendly name (just in case two interfaces share the same IP) ? For example a format like : 192.168.0.10 (wlan0)

That change was introduced with messense/if-addrs#34 which is available since https://crates.io/crates/if-addrs/0.12.0, we are using https://crates.io/crates/if-addrs/0.10.2

A good finding! Would you please update if-addrs version so we can use this?

@@ -2038,7 +2038,7 @@ impl Zeroconf {
next_delay: u32,
listener: Sender<ServiceEvent>,
) {
let addr_list: Vec<_> = self.intf_socks.keys().collect();
let addr_list: Vec<_> = self.intf_socks.keys().map(|itf| itf.addr.ip()).collect();
Copy link
Owner

Choose a reason for hiding this comment

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

Btw, you can just call itf.ip() .

@@ -2038,7 +2038,7 @@ impl Zeroconf {
next_delay: u32,
listener: Sender<ServiceEvent>,
) {
let addr_list: Vec<_> = self.intf_socks.keys().collect();
let addr_list: Vec<_> = self.intf_socks.keys().map(|itf| itf.addr.ip()).collect();
Copy link
Owner

Choose a reason for hiding this comment

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

Related to the general comment I gave, maybe we could print both IP and name by doing something like:

let addr_list: Vec<_> = self.intf_socks.keys().map(|itf| (itf.ip(), itf.name)).collect();

(I didn't try locally, so just a thought)

Copy link
Contributor Author

@hrzlgnm hrzlgnm Aug 21, 2024

Choose a reason for hiding this comment

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

Yes it's simpler, but I personally find the <ip> (<itf-name>) more pleasant to read :)

@hrzlgnm
Copy link
Contributor Author

hrzlgnm commented Aug 21, 2024

Thanks for this PR!

An alternative would be to print interface names. But therefore we would need to update the if_addrs crate. As the currently used version only would print he GUID instead of the friendly name of the interface when running on windows.

In this example code, I think maybe we could print both IP address and the friendly name (just in case two interfaces share the same IP) ? For example a format like : 192.168.0.10 (wlan0)

That change was introduced with messense/if-addrs#34 which is available since https://crates.io/crates/if-addrs/0.12.0, we are using https://crates.io/crates/if-addrs/0.10.2

A good finding! Would you please update if-addrs version so we can use this?

Yes will do, without the update of the crate if_addrs on windows that would look like this:

At 6.6554ms: SearchStarted("_http._tcp.local. on addrs [192.168.73.1 ({B44B9330-91A8-4ACF-985D-96270A5423AA}), fe80::2b19:d118:3bdc:d9e8 ({649FE08F-C011-4CFD-9E21-F3688EFF4B5C}), fe80::e5b8:2dca:ee23:5df4 ({B44B9330-91A8-4ACF-985D-96270A5423AA}), 192.168.178.25 ({361919C0-6530-43E9-B3E9-9DF8111EFF9F}), 192.168.49.1 ({649FE08F-C011-4CFD-9E21-F3688EFF4B5C}), fe80::f900:996d:50d1:8349 ({361919C0-6530-43E9-B3E9-9DF8111EFF9F}), 2003:e8:bf4e:c900:860d:98d5:d90b:7d ({361919C0-6530-43E9-B3E9-9DF8111EFF9F}), 2003:e8:bf4e:c900:ac70:4a66:970:79d2 ({361919C0-6530-43E9-B3E9-9DF8111EFF9F})]")

With the crate updated:

At 6.549ms: SearchStarted("_http._tcp.local. on addrs [fe80::f900:996d:50d1:8349 (Ethernet), fe80::e5b8:2dca:ee23:5df4 (VMware Network Adapter VMnet8), 2003:e8:bf4e:c900:ac70:4a66:970:79d2 (Ethernet), 192.168.49.1 (VMware Network Adapter VMnet1), 192.168.73.1 (VMware Network Adapter VMnet8), 2003:e8:bf4e:c900:860d:98d5:d90b:7d (Ethernet), 192.168.178.25 (Ethernet), fe80::2b19:d118:3bdc:d9e8 (VMware Network Adapter VMnet1)]")

@hrzlgnm hrzlgnm changed the title Send SearchStarted events with ip addrs Send SearchStarted events with addrs as ip (name) Aug 21, 2024
@hrzlgnm hrzlgnm changed the title Send SearchStarted events with addrs as ip (name) Send SearchStarted events with addrs as ip (intf-name) Aug 21, 2024
Copy link
Owner

@keepsimple1 keepsimple1 left a comment

Choose a reason for hiding this comment

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

The new print output looks really nice! Thanks!

@keepsimple1 keepsimple1 merged commit 5567c1f into keepsimple1:main Aug 21, 2024
3 checks passed
@hrzlgnm hrzlgnm deleted the search-started-on-addrs branch August 22, 2024 04:09
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.

2 participants