Skip to content

Add targeted internet speed test - #50

Merged
Boussetta merged 6 commits into
mainfrom
feat/internet-speed-test
Sep 15, 2026
Merged

Boussetta merged 6 commits into
mainfrom
feat/internet-speed-test

Conversation

@Boussetta

Copy link
Copy Markdown
Owner

Summary

  • Add a targeted internet speed-test API and Windows implementation.
  • Add a speed-test meter opened from the selected gateway card.
  • Run the test against the selected router and return latency, download, and upload measurements.
  • Keep the test asynchronous so the tray UI remains responsive.

Validation

  • MSVC Debug build
  • CTest: 6/6 passing

The speed-test work is based on merged PR #49 and is isolated from the per-network device-store changes.

Copilot AI lite review requested due to automatic review settings September 15, 2026 11:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Final review identified a critical payload-bounds issue and multiple moderate functional, cancellation, and lifecycle issues.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a Windows asynchronous internet speed-test API and popup meter integrated with gateway selection and tray routing.

Changes:

  • Adds portable speed-test results and quality grading.
  • Implements asynchronous WinHTTP latency, download, and upload measurements.
  • Integrates gateway selection, meter UI, cancellation, and build targets.
File summaries
File Summary and final findings
src/ui/tray_win32.c Integrates popup routing. Critical, 2 votes: WM_COPYDATA handling can read past an unterminated payload; validate bounds and require an in-range terminator.
src/ui/speed_meter_win32.h Declares the meter API. Moderate, 1 vote: showing the meter does not start the fresh measurement promised by the contract.
src/ui/speed_meter_win32.c Implements the meter and worker lifecycle. Moderate, 2 votes: destruction can block the UI thread during network calls. Moderate, 3 votes: replacing router metadata can leave an old worker and progress snapshot active.
src/ui/network_map_win32.h Adds router-selection messaging.
src/ui/network_map_win32.c Handles gateway-node selection.
src/platform/win32/speedtest_win32.c Implements WinHTTP measurements. Moderate, 1 vote: failed or rejected upload responses can still mark uploads valid. Moderate, 2 votes: the selected router is not used for the connection. Moderate, 1 vote: download reads do not poll cancellation between chunks. Moderate, 1 vote: the upload loop does not poll cancellation between writes. Moderate, 1 vote: the final response wait is not skipped after cancellation.
src/core/speedtest.c Implements quality grading. Nit, 2 votes: threshold, weakest-dimension, and partial/unknown-result cases lack test coverage.
include/linkpulse/speedtest.h Defines the public speed-test API.
CMakeLists.txt Adds the new core, platform, and UI sources to the build.
Review details

Suppressed comments (5)

src/platform/win32/speedtest_win32.c:327

  • If WinHttpReceiveResponse fails, or if it returns a non-200 status, this path still reaches the accounting below with total_bytes > 0 and marks the upload as valid. That reports an upload speed even though the endpoint never accepted the request; treat either response failure or rejection as LP_ERR_IO before setting has_upload.
        if (remaining == 0 && WinHttpReceiveResponse(response, NULL)) {
            const DWORD http_status = response_status_code(response);
            if (http_status != 200) {
                LP_WARN("speed test upload rejected: http_status=%lu", http_status);
                WinHttpCloseHandle(response);

src/platform/win32/speedtest_win32.c:211

  • Cancellation is checked only by the outer loop. Once a 25 MB response is open, this inner loop can continue reading the entire body before observing cancel_flag; on a slow link, stop_test can therefore wait for the remainder of the transfer instead of stopping between chunks as the public API promises. Poll is_cancelled(request) before each WinHttpReadData call.
        for (;;) {
            DWORD read = 0;
            if (!WinHttpReadData(response, buffer, (DWORD)sizeof(buffer), &read)) {

src/platform/win32/speedtest_win32.c:323

  • After the final upload chunk, this unconditional WinHttpReceiveResponse can still wait for the server even when cancellation was requested. Since stop_test joins this worker, backing out or shutting down can block unnecessarily; skip the response wait when cancel_flag is set.
        if (remaining == 0 && WinHttpReceiveResponse(response, NULL)) {

src/platform/win32/speedtest_win32.c:301

  • The upload loop only checks cancel_flag in the outer loop. Once this 10 MB request is open, it can keep calling WinHttpWriteData until the entire request is sent even after the user cancels, contrary to the API's between-chunk cancellation contract.
        while (remaining > 0) {
            const DWORD chunk = remaining < sizeof(buffer) ? (DWORD)remaining
                                                           : (DWORD)sizeof(buffer);
            DWORD written = 0;
            if (!WinHttpWriteData(response, buffer, chunk, &written) || written == 0) {

src/ui/speed_meter_win32.h:16

  • This public contract says showing the meter starts a fresh measurement, but lp_speed_meter_show only updates the labels, displays the window, and invalidates it; start_test is called only from the Run test button handler. Opening a gateway therefore leaves the meter at “Ready to test” instead of starting the promised run. Either start the worker from the show path or revise the contract and UI flow to make the extra click explicit.
/* Shows the meter for one router and starts a fresh measurement run. */
void lp_speed_meter_show(HWND window, const char *router_label, const char *router_ip,
                         const char *isp, bool use_bits);
  • Files reviewed: 9/9 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ui/tray_win32.c Outdated
Comment thread src/platform/win32/speedtest_win32.c
Comment thread src/ui/speed_meter_win32.c
Comment thread src/ui/speed_meter_win32.c
Comment thread src/core/speedtest.c
Boussetta and others added 2 commits September 15, 2026 12:44
Refactor IP address handling to ensure safe copying and null termination.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI and others added 2 commits September 15, 2026 11:48
Co-authored-by: Boussetta <4375228+Boussetta@users.noreply.github.com>
Co-authored-by: Boussetta <4375228+Boussetta@users.noreply.github.com>
Co-authored-by: Boussetta <4375228+Boussetta@users.noreply.github.com>
@Boussetta
Boussetta merged commit 8796aff into main Sep 15, 2026
2 checks passed
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