Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/desktop_peer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ int main(int argc, char* argv[]) {
app.last_tick_ms = now;
app.last_diag_ms = now;

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

// ════════════════════════════════════════════════════════════
Expand Down
93 changes: 54 additions & 39 deletions examples/diag_scanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <thread>
#include <chrono>
#include <csignal>
#include <deque>
#include <vector>

// ─── Configuration ──────────────────────────────────────────────
Expand Down Expand Up @@ -75,9 +76,11 @@ struct DiscoveredDevice {
DeviceCounters counters;
};

static std::vector<DiscoveredDevice> devices;
static std::deque<DiscoveredDevice> devices;
static Runtime* g_rt = nullptr;
static volatile std::sig_atomic_t running = 1;
static uint32_t g_last_scan_ms = 0; // last time queries were issued
static bool g_dashboard_printed = false;

// ─── Signal Handler ─────────────────────────────────────────────

Expand Down Expand Up @@ -202,7 +205,33 @@ static void on_clear_response(sero::ReturnCode rc,
}
}

// ─── SD Callback ────────────────────────────────────────────────
// ─── Query a device ─────────────────────────────────────────────

static void query_device(DiscoveredDevice& dev, uint32_t now_ms) {
using DM = sero::DiagMethod;

// 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);

Comment on lines +213 to +218

Copilot AI Mar 3, 2026

Copy link

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
g_rt->request(dev.addr, sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_SERVICE_LIST),
nullptr, 0, on_service_list, &dev, REQUEST_TIMEOUT_MS, now_ms);

g_rt->request(dev.addr, sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_DTCS),
nullptr, 0, on_dtcs, &dev, REQUEST_TIMEOUT_MS, now_ms);

g_rt->request(dev.addr, sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_COUNTERS),
nullptr, 0, on_counters, &dev, REQUEST_TIMEOUT_MS, now_ms);

dev.last_query_ms = now_ms;
}

// ─── SD Callbacks ────────────────────────────────────────────────

static void on_diag_service_found(uint16_t sid, const Addr& addr, void* /*ctx*/) {
if (sid != sero::DIAG_SERVICE_ID) return;
Expand All @@ -218,39 +247,23 @@ static void on_diag_service_found(uint16_t sid, const Addr& addr, void* /*ctx*/)
std::printf("[scanner] Discovered diagnostics device at %u.%u.%u.%u:%u\n",
addr[0], addr[1], addr[2], addr[3],
unsigned((addr[4] << 8) | addr[5]));

// Query immediately so data is available within ~500 ms of discovery
// instead of waiting up to SCAN_INTERVAL_MS for the next periodic cycle.
// Also arm the dashboard window so it prints after responses arrive.
uint32_t now = example::now_ms();
query_device(devices.back(), now);
g_last_scan_ms = now;
g_dashboard_printed = false;
}

static void on_diag_service_lost(uint16_t sid, void* /*ctx*/) {
if (sid != sero::DIAG_SERVICE_ID) return;
std::printf("[scanner] Diagnostics service 0x%04X lost\n", sid);
// Mark devices as not found (they'll be rediscovered if they come back)
for (auto& dev : devices) {
dev.found = false;
}
}

// ─── Query a device ─────────────────────────────────────────────

static void query_device(DiscoveredDevice& dev, uint32_t now_ms) {
using DM = sero::DiagMethod;

g_rt->request(sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_DEVICE_INFO),
nullptr, 0, on_device_info, &dev, REQUEST_TIMEOUT_MS, now_ms);

g_rt->request(sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_SERVICE_LIST),
nullptr, 0, on_service_list, &dev, REQUEST_TIMEOUT_MS, now_ms);

g_rt->request(sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_DTCS),
nullptr, 0, on_dtcs, &dev, REQUEST_TIMEOUT_MS, now_ms);

g_rt->request(sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(DM::DIAG_GET_COUNTERS),
nullptr, 0, on_counters, &dev, REQUEST_TIMEOUT_MS, now_ms);

dev.last_query_ms = now_ms;
// The SD layer tracks only one provider slot per service_id — this fires
// when that slot's TTL expires, not when an individual device goes offline.
// Silently re-issue find_service so the slot is refreshed on the next
// offer from any device, keeping discovery live for newly appearing devices.
g_rt->find_service(sero::DIAG_SERVICE_ID, 1, example::now_ms());
}

// ─── Print dashboard ────────────────────────────────────────────
Expand Down Expand Up @@ -389,16 +402,15 @@ int main(int argc, char* argv[]) {
std::printf("[scanner] Searching for diagnostics services (0x%04X)...\n\n",
sero::DIAG_SERVICE_ID);

uint32_t last_scan_ms = 0;

// ── Main loop ───────────────────────────────────────────────
while (running) {
now = example::now_ms();
rt.process(now);

// Query discovered devices periodically
if (now - last_scan_ms >= SCAN_INTERVAL_MS) {
last_scan_ms = now;
if (now - g_last_scan_ms >= SCAN_INTERVAL_MS) {
g_last_scan_ms = now;
g_dashboard_printed = false;

for (auto& dev : devices) {
if (dev.found) {
Expand All @@ -414,7 +426,7 @@ int main(int argc, char* argv[]) {
dev.info.client_id == clear_target_client) {
uint8_t payload[2];
sero::MessageHeader::write_u16(payload, clear_dtc_code);
rt.request(sero::DIAG_SERVICE_ID,
rt.request(dev.addr, sero::DIAG_SERVICE_ID,
static_cast<uint16_t>(sero::DiagMethod::DIAG_CLEAR_DTCS),
payload, 2, on_clear_response, nullptr,
REQUEST_TIMEOUT_MS, now);
Expand All @@ -429,10 +441,13 @@ int main(int argc, char* argv[]) {
// Print after a brief delay to let responses arrive
}

// Print dashboard shortly after query responses should have arrived
if (last_scan_ms > 0 && now - last_scan_ms > 500 &&
now - last_scan_ms < 600) {
// Print dashboard once, ~500 ms after queries were sent, to allow
// responses to arrive before rendering. The flag prevents re-printing
// on every loop iteration during that window.
if (!g_dashboard_printed && g_last_scan_ms > 0 &&
now - g_last_scan_ms >= 500) {
print_dashboard();
g_dashboard_printed = true;
}

std::this_thread::sleep_for(std::chrono::milliseconds(1));
Expand Down
64 changes: 47 additions & 17 deletions include/sero/runtime.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,24 +154,25 @@ class Runtime {
if (!sd_.get_provider_address(service_id, provider)) {
return std::nullopt; // service not found
}
return request_impl(provider, service_id, method_id, payload, payload_length,
callback, user_ctx, timeout_ms, 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(provider);
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, provider);
if (!transport_.send(provider, tx_buffer_, total)) {
// Send failed — cancel the pending request
request_tracker_.complete(*rid, ReturnCode::E_NOT_OK, nullptr, 0);
diag_.increment(DiagnosticCounter::DroppedMessages);
return std::nullopt;
}
return rid;
/// 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)
{
return request_impl(target, service_id, method_id, payload, payload_length,
callback, user_ctx, timeout_ms, now_ms);
}
Comment on lines +161 to +176

Copilot AI Mar 3, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

/// Fire-and-forget REQUEST_NO_RETURN (§5.2).
Expand Down Expand Up @@ -696,6 +697,35 @@ class Runtime {

// ── Message building ────────────────────────────────────────

/// Internal: allocate a request ID, build, and send to a known target.
std::optional<uint32_t> request_impl(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;
}

/// Build a complete message in tx_buffer_. Returns total byte count.
std::size_t build_message(const MessageHeader& hdr,
const uint8_t* payload,
Expand Down
49 changes: 49 additions & 0 deletions test/test_runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,55 @@ TEST_F(RuntimeTest, ClientRequest_ServiceFound_SendsAndTracksRequest) {
EXPECT_TRUE(callback_called);
}

// ── Client-side Explicit-Target Request ─────────────────────────

TEST_F(RuntimeTest, ClientRequest_ExplicitTarget_SendsWithoutSD) {
// SD has no provider registered — explicit-target overload must still send
Addr target = make_addr(42);

bool callback_called = false;
auto cb = [](ReturnCode, const uint8_t*, std::size_t, void* ctx) {
*static_cast<bool*>(ctx) = true;
};

auto rid = rt->request(target, 0x1000, 0x0001, nullptr, 0,
cb, &callback_called, 500, 0);
ASSERT_TRUE(rid.has_value());
ASSERT_EQ(transport.sent.size(), 1u);

// Verify the message was sent to the explicit target
EXPECT_EQ(transport.sent[0].destination, target);

MessageHeader sent_hdr = MessageHeader::deserialize(transport.sent[0].data.data());
EXPECT_EQ(sent_hdr.message_type, static_cast<uint8_t>(MessageType::REQUEST));
EXPECT_EQ(sent_hdr.service_id, 0x1000);
EXPECT_EQ(sent_hdr.method_event_id, 0x0001);
}

TEST_F(RuntimeTest, ClientRequest_ExplicitTarget_ResponseCompletesCallback) {
Addr target = make_addr(42);

bool callback_called = false;
ReturnCode received_rc = ReturnCode::E_NOT_OK;
auto cb = [](ReturnCode rc, const uint8_t*, std::size_t, void* ctx) {
auto* pair = static_cast<std::pair<bool*, ReturnCode*>*>(ctx);
*pair->first = true;
*pair->second = rc;
};

std::pair<bool*, ReturnCode*> ctx{&callback_called, &received_rc};
auto rid = rt->request(target, 0x1000, 0x0001, nullptr, 0,
cb, &ctx, 500, 100);
ASSERT_TRUE(rid.has_value());

// Inject a RESPONSE for this request_id from the explicit target
MessageHeader resp_hdr = make_response(0x1000, 0x0001, 0, *rid);
inject_and_process(resp_hdr, nullptr, 0, target, 200);

EXPECT_TRUE(callback_called);
EXPECT_EQ(received_rc, ReturnCode::E_OK);
}

// ── Client-side Fire-and-Forget ─────────────────────────────────

TEST_F(RuntimeTest, ClientFireAndForget_ServiceFound_Sends) {
Expand Down