fix: diagnostics multi device discovery by mad201802 · Pull Request #3 · Aerospace-Lab-e-V/sero

mad201802 · 2026-03-03T14:27:46Z

This pull request introduces improvements to device querying and dashboard rendering in the diagnostics scanner example, and adds support for explicit-address requests in the runtime. The changes ensure each discovered device is queried directly and promptly, resulting in more accurate and timely dashboard updates. Additionally, the codebase is refactored for clarity and robustness in service discovery and request handling.

Diagnostics scanner improvements:

Added immediate querying of newly discovered devices by calling query_device with the explicit device address, ensuring data is available quickly and dashboard updates are timely.
Changed periodic device querying and dashboard printing logic to use new global variables (g_last_scan_ms, g_dashboard_printed) for more reliable timing and single dashboard render per cycle. [1] [2] [3]
Refactored device request logic to use explicit addresses for all diagnostic requests, including DTC clearing, allowing for direct communication with each device rather than relying on service discovery alone. [1] [2]

Service discovery robustness:

Improved handling of lost diagnostics services: instead of marking all devices as lost, the code now silently re-issues find_service to keep discovery live and responsive to newly appearing devices.

Runtime API enhancement:

Added a new overload of the request method in Runtime (runtime.hpp) that allows sending requests to an explicit target address, bypassing service discovery. This is essential for scenarios with multiple providers of the same service ID.

These changes collectively make the diagnostics scanner more responsive and accurate, and extend the runtime with a useful new capability for direct device communication.

…ashboard updates

Copilot

Pull request overview

This PR improves multi-device diagnostics discovery in the diag_scanner example by querying each newly discovered device directly (via explicit address) and adds a Runtime::request(...) overload to support explicit-address requests without relying on service discovery.

Changes:

Added Runtime::request(const Addr& target, ...) to send requests to a specific peer address, bypassing SD provider selection.
Updated diag_scanner to query devices immediately on discovery and to use explicit-address requests for per-device polling and DTC clearing.
Enabled the built-in diagnostics service in desktop_peer so it can be discovered and queried by scanners.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
include/sero/runtime.hpp	Adds explicit-target `request` overload to bypass service discovery for multi-provider scenarios.
examples/diag_scanner.cpp	Refactors discovery/query loop to query each discovered device directly and adjust dashboard timing logic.
examples/desktop_peer.cpp	Enables built-in diagnostics service during startup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-03T14:33:38Z

    app.last_tick_ms       = now;
    app.last_diag_ms       = now;

+    rt.enable_diagnostics(now);


enable_diagnostics() returns bool and can fail (e.g., if service registration or offer fails). The return value is currently ignored, so the example can silently run without diagnostics being discoverable. Consider checking the return value and printing an error / exiting early if diagnostics can’t be enabled.

Suggested change

rt.enable_diagnostics(now);

if (!rt.enable_diagnostics(now)) {

std::fprintf(stderr, "[main] Failed to enable diagnostics; diagnostics will be unavailable.\n");

}

Copilot · 2026-03-03T14:33:39Z

+    // Use the explicit-address overload so each device gets its own requests,
+    // regardless of which provider the SD layer currently tracks.
+    g_rt->request(dev.addr, sero::DIAG_SERVICE_ID,
+                  static_cast<uint16_t>(DM::DIAG_GET_DEVICE_INFO),
+                  nullptr, 0, on_device_info, &dev, REQUEST_TIMEOUT_MS, now_ms);
+


query_device uses &dev (pointer to an element inside the global std::vector<DiscoveredDevice> devices) as the request callback context. If another device is discovered while requests are still pending, a subsequent devices.push_back() can reallocate and move elements, invalidating those pointers and leading to use-after-free in the response callbacks. Use stable storage (e.g., std::deque / std::list / std::vector<std::unique_ptr<...>>) or pass a stable identifier (e.g., the device Addr) and look up the device inside the callback.

Copilot · 2026-03-03T14:33:39Z

+        auto rid = request_tracker_.allocate(service_id, method_id,
+                                             timeout_ms, now_ms,
+                                             callback, user_ctx);
+        if (!rid) return std::nullopt; // table full
+
+        bool auth = should_auth_outgoing(target);
+        MessageHeader hdr = make_request_header(service_id, method_id,
+                                                 payload_length, *rid, auth);
+        hdr.sequence_counter = e2e_.next_sequence();
+        std::size_t total = build_message(hdr, payload, payload_length, auth, target);
+        if (!transport_.send(target, tx_buffer_, total)) {
+            request_tracker_.complete(*rid, ReturnCode::E_NOT_OK, nullptr, 0);
+            diag_.increment(DiagnosticCounter::DroppedMessages);
+            return std::nullopt;
+        }
+        return rid;


This new request(const Addr& target, ...) overload duplicates the send/build/tracking logic from the existing request(service_id, ...) method. To reduce the risk of future behavior drift (e.g., diagnostics counters, auth flags, header fields, failure handling), consider refactoring both overloads to share a single internal helper that takes the resolved destination address.

Copilot · 2026-03-03T14:33:39Z

+    /// Send a REQUEST to an explicit target address, bypassing SD lookup.
+    /// Use this when you manage multiple providers of the same service_id
+    /// (e.g. a diagnostic scanner querying each discovered device directly).
+    std::optional<uint32_t> request(const Addr& target,
+                                    uint16_t service_id,
+                                    uint16_t method_id,
+                                    const uint8_t* payload,
+                                    std::size_t payload_length,
+                                    RequestCallback callback,
+                                    void* user_ctx,
+                                    uint32_t timeout_ms,
+                                    uint32_t now_ms)
+    {
+        auto rid = request_tracker_.allocate(service_id, method_id,
+                                             timeout_ms, now_ms,
+                                             callback, user_ctx);
+        if (!rid) return std::nullopt; // table full
+
+        bool auth = should_auth_outgoing(target);
+        MessageHeader hdr = make_request_header(service_id, method_id,
+                                                 payload_length, *rid, auth);
+        hdr.sequence_counter = e2e_.next_sequence();
+        std::size_t total = build_message(hdr, payload, payload_length, auth, target);
+        if (!transport_.send(target, tx_buffer_, total)) {
+            request_tracker_.complete(*rid, ReturnCode::E_NOT_OK, nullptr, 0);
+            diag_.increment(DiagnosticCounter::DroppedMessages);
+            return std::nullopt;
+        }
+        return rid;
+    }


The new request(const Addr& target, ...) overload isn’t covered by existing unit tests (current test/test_runtime.cpp only exercises the SD-resolving request(service_id, ...)). Adding a test that verifies this overload sends to the provided target even when SD has no provider (and that responses complete the correct callback) would help prevent regressions.

mad201802 · 2026-03-03T14:34:49Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-03-03T14:34:59Z

@mad201802 I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * fix: address review feedback - check enable_diagnostics return, stable storage, request_impl refactor, new tests Co-authored-by: mad201802 <45396189+mad201802@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mad201802 <45396189+mad201802@users.noreply.github.com>

feat: enhance diagnostics discovery with direct device querying and d…

774c7e1

…ashboard updates

mad201802 requested a review from Copilot March 3, 2026 14:27

mad201802 self-assigned this Mar 3, 2026

Copilot started reviewing on behalf of mad201802 March 3, 2026 14:28

View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Copilot AI mentioned this pull request Mar 3, 2026

fix: address review feedback on diagnostics multi-device discovery #4

Merged

mad201802 merged commit 1846a37 into main Mar 3, 2026
11 checks passed

mad201802 deleted the fix/diagnostics_discovery branch March 17, 2026 19:58

Navigation Menu

Repository navigation

fix: diagnostics multi device discovery - #3

fix: diagnostics multi device discovery#3
mad201802 merged 2 commits into
mainfrom
fix/diagnostics_discovery

mad201802 commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

mad201802 commented Mar 3, 2026

Uh oh!

Copilot AI commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    rt.enable_diagnostics(now);
+    if (!rt.enable_diagnostics(now)) {
+        std::fprintf(stderr, "[main] Failed to enable diagnostics; diagnostics will be unavailable.\n");
+    }

Conversation

mad201802 commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

mad201802 commented Mar 3, 2026

Uh oh!

Copilot AI commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants