diff --git a/firmware/lib/test_handlers.cpp b/firmware/lib/test_handlers.cpp index f732668..e756ae7 100644 --- a/firmware/lib/test_handlers.cpp +++ b/firmware/lib/test_handlers.cpp @@ -10,8 +10,6 @@ #include "parse_buffer.h" #include "prepend_buffer.h" -// Echo IDs are just tags used to match our ping replies; constants suffice -// since tests are one-at-a-time. static constexpr uint16_t PING_ECHO_ID = 0x1234; static constexpr uint16_t PING_RATE_ECHO_ID = 0x5678; @@ -20,16 +18,11 @@ struct peer_info { ipv4::ip4_addr ip; }; -// Shared sub-struct types. Used as fields inside per-test-command structs -// when two tests happen to carry the same shape of data. - -// State for the discover_peer primitive. struct discovery_data { void (*on_found)(const peer_info&) = nullptr; void (*on_timeout)() = nullptr; }; -// Rolling state for a rate-testing ping run. struct ping_rate_data { peer_info peer; uint16_t target; @@ -40,8 +33,6 @@ struct ping_rate_data { uint32_t start_us; }; -// One struct per test command. Empty structs (where a test command has no -// extra state beyond the common fields) are kept for symmetry/documentation. struct discovery_igmp_test {}; struct discovery_info_test { discovery_data discovery; @@ -57,23 +48,17 @@ struct byte_rate_test { ping_rate_data rate; }; -// All test state lives in this single instance. The protocol is one-test-at- -// a-time: handle_test rejects a new request while in_flight is set. Only the -// fields for the running test are populated; the rest are wasted bytes traded -// for clarity. +// One test runs at a time; in_flight gates that. active_* let shared +// primitive callbacks find the running test's sub-state. struct test_state { - // Common to every async test. bool in_flight = false; responder resp; timer_handle timer = nullptr; frame_cb_handle frame_cb = nullptr; - // Views into the running test's shared sub-state so the shared primitive - // callbacks (discover_peer, ping_rate) know where to find it. discovery_data* active_discovery = nullptr; ping_rate_data* active_rate = nullptr; - // Per-test-command storage. discovery_igmp_test discovery_igmp; discovery_info_test discovery_info; ping_subnet_test ping_subnet; @@ -93,13 +78,9 @@ static void test_end(const ResponseTest& result) { ts.in_flight = false; } -// ----- discover_peer (shared building block) ----- - -// Note on frame/timer handle lifetimes: when a callback fires, the dispatcher -// (net or timer_queue) has already taken ownership of the node. Callbacks that -// self-consume (frame returning true, or any timer fire) must null the -// matching `ts.` handle before calling test_end, so test_end doesn't try to -// cancel a stale handle. +// When a callback fires, its dispatcher (net or timer_queue) has already +// removed the node; the matching ts.timer / ts.frame_cb is stale. Callbacks +// that self-consume must null that handle before calling test_end. static bool discover_reply_cb(std::span frame) { parse_buffer pb(frame); @@ -114,7 +95,7 @@ static bool discover_reply_cb(std::span frame) { if (ip->src == net_get_state().ip) return false; dispatch_cancel_timer(ts.timer); ts.timer = nullptr; - ts.frame_cb = nullptr; // self-consumed via `return true` below + ts.frame_cb = nullptr; auto cont = ts.active_discovery ? ts.active_discovery->on_found : nullptr; ts.active_discovery = nullptr; peer_info peer{eth_hdr->src, ip->src}; @@ -161,19 +142,17 @@ static void discover_peer(discovery_data& d, net_send_raw(buf.span()); } -// ----- discovery_igmp ----- - static bool igmp_report_cb(std::span frame) { ipv4::ip4_addr group; if (!igmp::parse_report(frame, group)) return false; if (group != igmp::PICOMAP_DISCOVERY_GROUP) return false; - ts.frame_cb = nullptr; // self-consumed via `return true` + ts.frame_cb = nullptr; test_end({true, {"got IGMP report for " + ipv4::to_string(group)}}); return true; } static void igmp_timeout_cb() { - ts.timer = nullptr; // already fired + ts.timer = nullptr; test_end({false, {"no IGMP report within 5s"}}); } @@ -188,8 +167,6 @@ static void test_discovery_igmp() { net_send_raw(buf.span()); } -// ----- discovery_info ----- - static void info_found(const peer_info& peer) { test_end({true, {"got info response from " + ipv4::to_string(peer.ip)}}); } @@ -202,12 +179,10 @@ static void test_discovery_info() { discover_peer(ts.discovery_info.discovery, info_found, info_timeout); } -// ----- ping_subnet / ping_global ----- - static bool ping_reply_cb(std::span frame) { ipv4::ip4_addr src_ip; if (!icmp::parse_echo_reply(frame, src_ip, PING_ECHO_ID)) return false; - ts.frame_cb = nullptr; // self-consumed via `return true` + ts.frame_cb = nullptr; if (src_ip == net_get_state().ip) test_end({false, {"got reply from self: " + ipv4::to_string(src_ip)}}); else @@ -216,7 +191,7 @@ static bool ping_reply_cb(std::span frame) { } static void ping_timeout_cb() { - ts.timer = nullptr; // already fired + ts.timer = nullptr; test_end({false, {"no reply from non-self host within 5s"}}); } @@ -233,8 +208,6 @@ static void start_ping(ipv4::ip4_addr dst_ip) { static void test_ping_subnet() { start_ping({169, 254, 255, 255}); } static void test_ping_global() { start_ping({255, 255, 255, 255}); } -// ----- packet_rate / byte_rate ----- - static size_t ping_rate_frame_size() { return sizeof(eth::header) + sizeof(ipv4::header) + sizeof(icmp::echo) + ts.active_rate->payload_len; @@ -274,7 +247,7 @@ static bool ping_rate_reply_cb(std::span frame) { static_cast(pps), static_cast(total_bytes), static_cast(kbps)); - ts.frame_cb = nullptr; // self-consumed via `return true` + ts.frame_cb = nullptr; test_end({true, {msg}}); return true; } @@ -285,7 +258,7 @@ static bool ping_rate_reply_cb(std::span frame) { } static void ping_rate_timeout_cb() { - ts.timer = nullptr; // already fired + ts.timer = nullptr; auto& r = *ts.active_rate; uint32_t elapsed_us = time_us_32() - r.start_us; char msg[64]; @@ -329,8 +302,6 @@ static void test_byte_rate() { start_ping_rate(ts.byte_rate.discovery, ts.byte_rate.rate, 2048, 1400, 8); } -// ----- registry ----- - using sync_test_fn = ResponseTest (*)(); using async_test_fn = void (*)();