Add latency check to connection details

[ethical-buddy]

Closes #845.

This adds a latency row to the connection details modal. The measurement only runs when the user clicks the latency button for the selected remote address, so Sniffnet does not add extra traffic for every captured connection.

The result is cached per remote IP while the app is running and is shown as measuring, measured, or unavailable depending on the ping result.

Checks run locally:

• cargo fmt --all -- --check
• cargo build --verbose
• cargo clippy -- -D warnings
• cargo test --verbose -- --nocapture

[GyulyVGC]

Hey @ethical-buddy this would be a nice addition to the app, thanks!

The foundations look solid but:

• I'd like having cross-platform tests not only on the ping output parsing, but on the whole execution of the command from its launch to its result
• I'm not sure spawning a shell command to execute the ping is the best way to measure latency, have you checked if there's some crate doing something similar?

[ethical-buddy]

Thanks for the review! I pushed a follow-up commit that expands the latency coverage beyond just parsing ping output.

The latency code now has a small executor boundary, so the tests cover the actual command-to-result flow without touching the network: platform-specific ping args, successful stdout parsing, missing latency output, stderr from failed ping, and launch failures.

I also checked the Rust ping crate direction briefly. Crates like surge-ping / ping-rs look useful, but since they rely on ICMP sockets they can introduce extra platform and permission trade-offs. I kept the system ping approach for this follow-up so the runtime behavior stays unchanged while the test coverage is stronger.

Does this direction look good to you, or would you prefer I dig deeper into a native ICMP crate despite those permission trade-offs?

[GyulyVGC]

Right now I don't have enough knowledge to make a decision about this.
I'd prefer to look better into it myself when I have some time to take an informed choice about the command / crate / methodology to use.

Concerning the test, what's the issue are you trying to avoid with the executor?
I'd prefer executing the actual command as in use in production code, and touching the network is fine since I believe GitHub runners should be able to do it.
So I'd keep the structure more similar to the initial one without the executor complexity, and just change the test to be really end-to-end.
Let me know if you faced challenges I'm not thinking about.

[ethical-buddy]

That makes sense, thanks for clarifying.

I pushed another follow-up that removes the executor abstraction and keeps the production path direct again. The latency test now calls the real measurement function against loopback, so it runs the actual ping command end to end instead of mocking the execution layer.

The only small caveat I kept is a non-CI guard for local environments where ping cannot open sockets. I hit that in my local sandbox with Operation not permitted, but on CI the test will still expect the real command to succeed and fail if it does not.

Does this match the structure you had in mind, or would you prefer removing even that local-only permission guard?

[GyulyVGC]

Thanks it's fine for the moment.

Still I'm not sure the final version of this will spawn a ping command directly, but for now let's wait until I find what to do.

Thanks for your time!

[GyulyVGC]

Hey @ethical-buddy after some research I would definitely prefer using surge-ping:

• it's natively async and fits better with iced execution paradigm
• removes the need to parse output which can be fragile and platform dependent

At first sight, I think you should use surge-ping::Client to reuse the same connection, but I'm not sure it's better than re-pinging from scratch, so please verify.

Most importantly, as you said there's the platform permission thing to verify, but this shouldn't be a problem on macOS (already running with sudo) and Linux (running with cap_net_raw).
On Windows Sniffnet runs with no privileges so I'm not sure this is going to work there.

[ethical-buddy]

Thanks for checking this. That direction makes sense to me.

I’ll switch the implementation to surge-ping and remove the output parsing path. I’ll also look specifically at whether keeping a reusable surge-ping::Client in the app state is better than creating one per measurement, since that affects how much state we need to thread through the iced task.

The main thing I’ll verify is the Windows behavior without elevated privileges. If surge-ping needs raw socket permissions there too, I’ll report back with what I find before adding a fallback, since I don’t want to keep the old command path unless you’re okay with that trade-off.

[ethical-buddy]

I pushed the surge-ping version now.

The latency path no longer spawns the system ping command or parses command output. It uses reusable surge-ping clients instead: one for IPv4 and one for IPv6, selected by the target address. I also added a shared sequence counter so concurrent measurements do not reuse the same ping sequence.

I kept the tests end to end against loopback and updated them for the async path. Locally I verified:

• cargo fmt --check
• cargo test networking::types::latency
• cargo test -- --skip utils::check_updates::tests::fetch_latest_release_from_github

The only full-suite failure without the skip is the existing GitHub latest-release network test, unrelated to this change.

Does this look like the structure you had in mind for reusing surge_ping::Client?

[GyulyVGC]

Did you verify how it behaves on Windows without admin privileges?

[ethical-buddy]

I currently dont have any windows setup with me to test , I am still getting it arranged ,
Might take a couple days more , If you could help me with that it'll be great . else ill try to do that asap

[GyulyVGC]

Oh yeah I have a Windows VM so I can try when I have some time

[ethical-buddy]

That would be great .

[Seb6277]

Hello,

I was browsing github on windows, so if i can help... i've tested on this setup :

test-environment.txt

It works fine !

[ethical-buddy]

@Seb6277 Thanks for this ,
Looks fine by me
@GyulyVGC ?

[GyulyVGC]

Tested on macOS, Linux, and Windows and I confirm it works.
I'll do some more improvements, then merge.

[GyulyVGC]

@all-contributors please add @ethical-buddy for code.

[allcontributors]

@GyulyVGC

I've put up a pull request to add @ethical-buddy! 🎉