diff --git a/AGENTS.md b/AGENTS.md index 07e2cd8..ba8392f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ Naming patterns: `canard_*` functions, `canard_*_t` types, `CANARD_*` macros. In Variables that are not mutated MUST be declared const, otherwise CI will reject the code. -Keep code compact and add brief comments before non-obvious logic. +DO NOT COMMENT THE CODE unless comments add critical information that is impossible to infer from reading the code (design rationale, gotchas, etc), in which case extremely terse comments are allowed. Treat warnings as errors and keep compatibility with strict warning flags. diff --git a/README.md b/README.md index 5ffd51c..152668f 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ int main(void) // Set up the local node. The node-ID will be allocated automatically. canard_t node; - if (!canard_new(&node, &vtable, mem_set, 100, UID_OR_TRUE_RANDOM_NUMBER, 0U)) { + if (!canard_new(&node, &vtable, mem_set, /*iface_bitmap=*/1, /*txq_cap=*/100, UID_OR_TRUE_RANDOM_NUMBER, 0U)) { return -1; } if (!canard_set_node_id(&node, 42U)) { diff --git a/demos/heartbeat_monitor.c b/demos/heartbeat_monitor.c index ed2f3b6..3d69c99 100644 --- a/demos/heartbeat_monitor.c +++ b/demos/heartbeat_monitor.c @@ -294,6 +294,7 @@ int main(const int argc, const char* const argv[]) .rx_payload = mem, .rx_filters = mem, }, + 0, // iface_bitmap: no TX interfaces => listen-only 0, // tx_queue_capacity: receive-only 0, // prng_seed: irrelevant for receive-only 0)) // filter_count: no HW filters diff --git a/libcanard/canard.c b/libcanard/canard.c index cb841e6..292ffa9 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -744,6 +744,12 @@ static bool tx_push(canard_t* const self, CANARD_ASSERT((!tr->fd) || !v0); // The caller must ensure this. CANARD_ASSERT(iface_bitmap != 0); + const byte_t effective = iface_bitmap & (byte_t)self->iface_bitmap; + if (effective == 0) { + mem_free(self->mem.tx_transfer, sizeof(tx_transfer_t), tr); + return false; + } + const canard_us_t now = self->vtable->now(self); // Expire old transfers first to free up queue space. @@ -775,7 +781,7 @@ static bool tx_push(canard_t* const self, (void)queue_size_before; // Adjust the spooled frame refcounts to avoid premature deallocation. - const byte_t frame_refcount_inc = (byte_t)(popcount(iface_bitmap) - 1U); + const byte_t frame_refcount_inc = (byte_t)(popcount(effective) - 1U); CANARD_ASSERT(frame_refcount_inc < CANARD_IFACE_COUNT); if (frame_refcount_inc > 0) { tx_frame_t* frame = spool; @@ -787,7 +793,7 @@ static bool tx_push(canard_t* const self, // Attach the spool. FOREACH_IFACE (i) { - if ((iface_bitmap & (1U << i)) != 0) { + if ((effective & (1U << i)) != 0) { tr->cursor[i] = spool; } } @@ -1165,12 +1171,14 @@ static byte_t rx_parse(const uint32_t can_id, // Node-ID 0 reserved for anonymous/broadcast, invalid for services. Self-addressing not allowed. is_v0 = is_v0 && (dst != 0) && (src != 0) && (src != dst); } else { - out_v0->dst = CANARD_NODE_ID_ANONYMOUS; - out_v0->port_id = (can_id >> 8U) & 0xFFFFU; - out_v0->kind = canard_kind_v0_message; + out_v0->dst = CANARD_NODE_ID_ANONYMOUS; + out_v0->kind = canard_kind_v0_message; if (src == 0) { - out_v0->src = CANARD_NODE_ID_ANONYMOUS; - is_v0 = is_v0 && start && end; // anonymous can only be single-frame + out_v0->src = CANARD_NODE_ID_ANONYMOUS; + out_v0->port_id = (can_id >> 8U) & 0x3U; // anon frame carries only the 2 low DTID bits + is_v0 = is_v0 && start && end; // anonymous can only be single-frame + } else { + out_v0->port_id = (can_id >> 8U) & 0xFFFFU; } } } @@ -1356,7 +1364,7 @@ static void rx_session_complete_slot(rx_session_t* const ses, const frame_t* con rx_slot_t* const slot = ses->slots[fr->priority]; CANARD_ASSERT(!fr->start && fr->end && (fr->port_id == sub->port_id) && (fr->kind == sub->kind)); CANARD_ASSERT((slot != NULL) && (sub->vtable->on_message != NULL)); - CANARD_ASSERT((slot->transfer_id == fr->transfer_id) && (slot->iface_index == ses->iface_index)); + CANARD_ASSERT(slot->transfer_id == fr->transfer_id); CANARD_ASSERT(fr->src != CANARD_NODE_ID_ANONYMOUS); // anons cannot be multiframe // Verify the CRC and dispatch the message if correct. The slot is consumed in either case. ses->slots[fr->priority] = NULL; // Slot memory ownership transferred to the application, or destroyed. @@ -1383,7 +1391,7 @@ static void rx_session_accept(rx_session_t* const ses, const canard_us_t ts_fram rx_slot_t* const slot = ses->slots[fr->priority]; if (slot != NULL) { CANARD_ASSERT((!fr->start || !fr->end) && (slot->expected_toggle == fr->toggle)); - CANARD_ASSERT((slot->transfer_id == fr->transfer_id) && (slot->iface_index == ses->iface_index)); + CANARD_ASSERT(slot->transfer_id == fr->transfer_id); rx_slot_advance(slot, fr->payload); // Multi-frame transfers place CRC differently in v1 and v0. // The v1 handling is trivial: simply compute the full payload CRC and ensure the residue is correct. @@ -1548,9 +1556,8 @@ static void rx_session_update(canard_subscription_t* const sub, rx_session_record_admission(ses, frame->priority, frame->transfer_id, ts, iface_index); } - // Accept the frame. + // Accept the frame. Must be last: on_message may unsubscribe, destroying ses. rx_session_accept(ses, ts, frame); - CANARD_ASSERT(!frame->end || (ses->slots[frame->priority] == NULL)); } static int32_t rx_subscription_cavl_compare(const void* const user, const canard_tree_t* const node) @@ -1605,7 +1612,9 @@ static canard_filter_t rx_filter_for_subscription(const canard_t* const self, } case canard_kind_v0_message: f.extended_can_id = (uint32_t)port_id << 8U; - f.extended_mask = 0x00FFFF80; + // A data-type-ID <= 3 can also arrive anonymously, carrying only its 2 low bits with a random + // discriminator above; a weak filter (2 low DTID bits + message flag only) admits both forms. + f.extended_mask = (port_id <= 3U) ? 0x00000380U : 0x00FFFF80U; break; case canard_kind_v0_response: case canard_kind_v0_request: { @@ -1631,11 +1640,15 @@ static canard_filter_t rx_filter_fuse(const canard_filter_t a, const canard_filt // Filter selectivity metric; see the Cyphal/CAN specification. Greater values ==> stronger filter. static byte_t rx_filter_rank(const canard_filter_t a) { return popcount(a.extended_mask); } -// Returns true if any filter in the array accepts the given extended CAN ID. -static bool rx_filter_match(const size_t count, const canard_filter_t* const filters, const uint32_t extended_can_id) +// Returns true if any filter in the array covers inner, i.e., accepts every frame that inner accepts. +// A single-point test is insufficient here because forced traffic spans all priorities and sources; a +// subscription filter that happens to match one representative may still reject other valid forced frames. +static bool rx_filter_covered(const size_t count, const canard_filter_t* const filters, const canard_filter_t inner) { for (size_t i = 0; i < count; i++) { - if ((extended_can_id & filters[i].extended_mask) == (filters[i].extended_can_id & filters[i].extended_mask)) { + CANARD_ASSERT((filters[i].extended_can_id & ~filters[i].extended_mask) == 0U); // canonical id + if (((filters[i].extended_mask & ~inner.extended_mask) == 0U) && + ((inner.extended_can_id & filters[i].extended_mask) == filters[i].extended_can_id)) { return true; } } @@ -1675,6 +1688,19 @@ static void rx_filter_coalesce_into(const size_t count, canard_filter_t* const i } } +// Append a filter, coalescing into the existing set if capacity is exhausted. +static void rx_filter_append(canard_filter_t* const into, + size_t* const n, + const size_t capacity, + const canard_filter_t f) +{ + if (*n < capacity) { + into[(*n)++] = f; + } else { + rx_filter_coalesce_into(*n, into, f); + } +} + // Recompute the filter configuration and apply. Returns true on success, false on driver error. static bool rx_filter_configure(canard_t* const self) { @@ -1698,12 +1724,7 @@ static bool rx_filter_configure(canard_t* const self) for (const canard_subscription_t* sub = (canard_subscription_t*)(void*)cavl2_min(self->rx.subscriptions[kind]); sub != NULL; sub = (canard_subscription_t*)(void*)cavl2_next_greater((canard_tree_t*)sub)) { - const canard_filter_t f = rx_filter_for_subscription(self, sub->kind, sub->port_id); - if (n < capacity) { - filters[n++] = f; - } else { - rx_filter_coalesce_into(n, filters, f); - } + rx_filter_append(filters, &n, capacity, rx_filter_for_subscription(self, sub->kind, sub->port_id)); } } CANARD_ASSERT(n <= capacity); @@ -1719,13 +1740,9 @@ static bool rx_filter_configure(canard_t* const self) { canard_kind_v0_message, 341U }, // DroneCAN NodeStatus }; for (size_t i = 0; i < sizeof(forced) / sizeof(forced[0]); i++) { - const canard_filter_t f = rx_filter_for_subscription(self, forced[i].kind, forced[i].port_id); - if (!rx_filter_match(n, filters, f.extended_can_id)) { - if (n < capacity) { - filters[n++] = f; - } else { - rx_filter_coalesce_into(n, filters, f); - } + const canard_filter_t g = rx_filter_for_subscription(self, forced[i].kind, forced[i].port_id); + if (!rx_filter_covered(n, filters, g)) { + rx_filter_append(filters, &n, capacity, g); } } CANARD_ASSERT(n <= capacity); @@ -1765,7 +1782,7 @@ static canard_subscription_t* rx_subscribe(canard_t* const &subscription->index_port_id, cavl2_trivial_factory); out = (canard_subscription_t*)(void*)existing; - self->rx.filters_dirty = (existing == &subscription->index_port_id); + self->rx.filters_dirty = self->rx.filters_dirty || (existing == &subscription->index_port_id); } return out; } @@ -1947,20 +1964,24 @@ static void node_id_occupancy_update(canard_t* const self, const byte_t src) bool canard_new(canard_t* const self, const canard_vtable_t* const vtable, const canard_mem_set_t memory, + const uint_least8_t iface_bitmap, const size_t tx_queue_capacity, const uint64_t prng_seed, const size_t filter_count) { const bool filter_ok = (filter_count == 0) || // ((vtable != NULL) && (vtable->filter != NULL) && mem_valid(memory.rx_filters)); - const bool ok = (self != NULL) && (vtable != NULL) && (vtable->now != NULL) && (vtable->tx != NULL) && + const bool iface_ok = (iface_bitmap & CANARD_IFACE_BITMAP_ALL) == iface_bitmap; // 0 => listen-only + const bool ok = (self != NULL) && (vtable != NULL) && (vtable->now != NULL) && (vtable->tx != NULL) && mem_valid(memory.tx_transfer) && mem_valid(memory.tx_frame) && mem_valid(memory.rx_session) && - mem_valid(memory.rx_payload) && filter_ok; + mem_valid(memory.rx_payload) && filter_ok && iface_ok; if (ok) { (void)memset(self, 0, sizeof(*self)); self->tx.fd = true; self->tx.queue_capacity = tx_queue_capacity; + self->iface_bitmap = iface_bitmap; self->rx.filter_count = filter_count; + self->rx.filters_dirty = filter_count > 0; // Program occupancy filters even before the first subscription. self->mem = memory; self->prng_state = prng_seed ^ (uintptr_t)self; self->vtable = vtable; diff --git a/libcanard/canard.h b/libcanard/canard.h index 5976527..9d3034a 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -227,6 +227,7 @@ struct canard_subscription_vtable_t /// A new message is received on a subscription. /// For the payload ownership notes refer to canard_payload_t. /// The timestamp is the arrival timestamp of the first frame of the transfer. + /// The callback may unsubscribe self; it must not otherwise reenter the library (no ingest/publish/poll). void (*on_message)(canard_subscription_t* self, canard_us_t timestamp, canard_prio_t priority, @@ -297,6 +298,9 @@ struct canard_t /// The node-ID can be set manually via the corresponding function. uint_least8_t node_id; + /// Bitmap of interfaces available on this node; the enqueue interface bitmap is ANDed with it. Zero is listen-only. + uint_least8_t iface_bitmap; + struct { /// By default, CAN FD mode is used; this flag can be used to change the mode to Classic CAN if needed; @@ -379,10 +383,15 @@ struct canard_t /// /// CAN FD mode is selected by default for outgoing frames; override the fd flag to change the mode if needed. /// +/// iface_bitmap is the set of interfaces available on this node; every enqueue interface bitmap is ANDed with it, +/// so a transfer targeting only unavailable interfaces is enqueued nowhere. Zero makes the node listen-only (no TX). +/// It may use only the CANARD_IFACE_COUNT least significant bits. +/// /// Returns true on success, false if any of the parameters are invalid. bool canard_new(canard_t* const self, const canard_vtable_t* const vtable, const canard_mem_set_t memory, + const uint_least8_t iface_bitmap, const size_t tx_queue_capacity, const uint64_t prng_seed, const size_t filter_count); @@ -433,6 +442,8 @@ void canard_refcount_inc(const canard_bytes_t obj); void canard_refcount_dec(canard_t* const self, const canard_bytes_t obj); /// Enqueue a message transfer on the specified interfaces. Use CANARD_IFACE_BITMAP_ALL to send on all interfaces. +/// The bitmap is ANDed with the node's available interfaces set at init; a transfer targeting no available interface +/// is enqueued nowhere and false is returned. /// Message ordering observed on the bus is guaranteed per subject as long as the priority of later messages is /// not higher (numerically not lower) than that of earlier messages. /// The context is passed into the tx() vtable function. @@ -440,8 +451,9 @@ void canard_refcount_dec(canard_t* const self, const canard_bytes_t obj); /// Cost is roughly linear in the number of emitted CAN frames plus log-time queue indexing. /// Memory use is one TX transfer object plus one shared TX frame object per emitted CAN frame. /// -/// Returns true on success; false on invalid arguments, OOM, or TX queue exhaustion. -/// See err.oom and err.tx_capacity for the enqueue failure cause. +/// Returns true on success; false on invalid arguments, OOM, TX queue exhaustion, or no available interface. +/// See err.oom and err.tx_capacity for OOM/capacity failures; a drop because no requested interface is available +/// increments no counter. bool canard_publish_16b(canard_t* const self, const canard_us_t deadline, const uint_least8_t iface_bitmap, @@ -569,7 +581,8 @@ void canard_unsubscribe(canard_t* const self, canard_subscription_t* const subsc /// /// To obtain the CRC seed, use canard_v0_crc_seed_from_data_type_signature(); if the payload does not exceed 7 bytes, /// the CRC seed can be arbitrary since it is not needed for single-frame transfers. -/// Returns true on success; false on invalid arguments, OOM, or TX queue exhaustion. +/// The interface bitmap is ANDed with the node's available interfaces set at init, as in canard_publish(). +/// Returns true on success; false on invalid arguments, OOM, TX queue exhaustion, or no available interface. /// A nonzero local node-ID is required. bool canard_v0_publish(canard_t* const self, const canard_us_t deadline, @@ -608,6 +621,7 @@ bool canard_v0_respond(canard_t* const self, /// Register a legacy v0 message subscription. /// Subscription updates are log-time; per-remote RX session state is allocated lazily on demand. /// Return semantics match canard_subscribe_16b(). +/// Anonymous v0 messages carry only the 2 low data-type-ID bits, so only data_type_id in [0,3] can receive them. canard_subscription_t* canard_v0_subscribe(canard_t* const self, canard_subscription_t* const subscription, const uint16_t data_type_id, diff --git a/tests/src/test_api_lifecycle.cpp b/tests/src/test_api_lifecycle.cpp index 9a7fbd0..9e3102a 100644 --- a/tests/src/test_api_lifecycle.cpp +++ b/tests/src/test_api_lifecycle.cpp @@ -118,7 +118,8 @@ static void init_capture(canard_t* const self, cap->now = 0; cap->accept_tx = true; cap->count = 0; - TEST_ASSERT_TRUE(canard_new(self, vtable, make_std_memory(), queue_capacity, 1234U, filter_count)); + TEST_ASSERT_TRUE( + canard_new(self, vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, queue_capacity, 1234U, filter_count)); TEST_ASSERT_TRUE(canard_set_node_id(self, node_id)); self->user_context = cap; } @@ -239,13 +240,13 @@ static uint16_t crc16_add(uint16_t crc, const void* data, const size_t size) static void test_canard_new_assigns_random_node_id() { canard_t self1 = {}; - TEST_ASSERT_TRUE(canard_new(&self1, &test_vtable, make_std_memory(), 16U, 12345U, 0U)); + TEST_ASSERT_TRUE(canard_new(&self1, &test_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 12345U, 0U)); TEST_ASSERT_TRUE(self1.node_id >= 1U); TEST_ASSERT_TRUE(self1.node_id <= 127U); canard_destroy(&self1); canard_t self2 = {}; - TEST_ASSERT_TRUE(canard_new(&self2, &test_vtable, make_std_memory(), 16U, 99999U, 0U)); + TEST_ASSERT_TRUE(canard_new(&self2, &test_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 99999U, 0U)); TEST_ASSERT_TRUE(self2.node_id >= 1U); TEST_ASSERT_TRUE(self2.node_id <= 127U); canard_destroy(&self2); @@ -258,24 +259,35 @@ static void test_canard_new_invalid_params() const canard_mem_set_t mem = make_std_memory(); // NULL self. - TEST_ASSERT_FALSE(canard_new(nullptr, &test_vtable, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(nullptr, &test_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); // NULL vtable. - TEST_ASSERT_FALSE(canard_new(&self, nullptr, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, nullptr, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); // Null vtable->now. canard_vtable_t bad_vtable = { .now = nullptr, .tx = mock_tx, .filter = nullptr }; - TEST_ASSERT_FALSE(canard_new(&self, &bad_vtable, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &bad_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); // Null vtable->tx. bad_vtable = { .now = mock_now, .tx = nullptr, .filter = nullptr }; - TEST_ASSERT_FALSE(canard_new(&self, &bad_vtable, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &bad_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); + + // Zero bitmap is valid: it declares a listen-only node. + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, 0U, 16U, 0U, 0U)); + TEST_ASSERT_EQUAL_UINT8(0U, self.iface_bitmap); + canard_destroy(&self); +#if UINT_LEAST8_MAX > CANARD_IFACE_BITMAP_ALL + TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, mem, (uint_least8_t)(CANARD_IFACE_BITMAP_ALL + 1U), 16U, 0U, 0U)); +#endif + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); + TEST_ASSERT_EQUAL_UINT8(CANARD_IFACE_BITMAP_ALL, self.iface_bitmap); + canard_destroy(&self); } // 3. set_node_id boundary values. static void test_canard_set_node_id_boundary() { canard_t self = {}; - TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 0U)); // 0 is valid per implementation (returns true). TEST_ASSERT_TRUE(canard_set_node_id(&self, 0U)); @@ -464,6 +476,52 @@ static void test_poll_filter_after_unsubscribe() canard_destroy(&self); } +// Regression (CN-01): a duplicate subscribe returning the incumbent must not clear a pending filter +// reconfiguration requested by a preceding new subscribe. +static void test_poll_filter_after_duplicate_subscribe() +{ + canard_t self = {}; + tx_capture_t cap = {}; + init_capture(&self, &cap, 42U, 16U, 4U, &capture_filter_vtable); + + rx_capture_t rx_cap = {}; + canard_subscription_t sub_a = {}; + TEST_ASSERT_EQUAL_PTR(&sub_a, canard_subscribe_16b(&self, &sub_a, 6001U, 256U, 2000000, &capture_sub_vtable)); + sub_a.user_context = &rx_cap; + canard_poll(&self, 0U); + const size_t calls_after_a = cap.filter_rec.invocation_count; + TEST_ASSERT_EQUAL_UINT64(1U, calls_after_a); + + // New subscription B sets dirty; a subsequent duplicate subscribe of A (returns incumbent) must keep it dirty. + canard_subscription_t sub_b = {}; + TEST_ASSERT_EQUAL_PTR(&sub_b, canard_subscribe_16b(&self, &sub_b, 6002U, 256U, 2000000, &capture_sub_vtable)); + sub_b.user_context = &rx_cap; + canard_subscription_t sub_dup = {}; + TEST_ASSERT_EQUAL_PTR(&sub_a, canard_subscribe_16b(&self, &sub_dup, 6001U, 256U, 2000000, &capture_sub_vtable)); + + canard_poll(&self, 0U); + TEST_ASSERT_EQUAL_UINT64(calls_after_a + 1U, cap.filter_rec.invocation_count); + + canard_unsubscribe(&self, &sub_a); + canard_unsubscribe(&self, &sub_b); + canard_destroy(&self); +} + +// Regression (review): a filter-capable instance must program its occupancy filters on the first poll even +// before any subscription or manual node-ID assignment. +static void test_poll_filter_configured_after_new() +{ + canard_t self = {}; + tx_capture_t cap = {}; + cap.accept_tx = true; + TEST_ASSERT_TRUE( + canard_new(&self, &capture_filter_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 4U)); + self.user_context = ∩ + canard_poll(&self, 0U); + TEST_ASSERT_TRUE(cap.filter_rec.invocation_count >= 1U); + canard_destroy(&self); +} + // 10. Poll cleans up stale sessions; a repeat TID is accepted after session expiry. static void test_poll_session_cleanup() { @@ -543,7 +601,7 @@ static void test_err_oom_on_publish() mem.tx_frame = instrumented_allocator_make_resource(&alloc_frame); canard_t self = {}; - TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(&self, 42U)); const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; @@ -859,56 +917,56 @@ static void test_canard_new_validation_branches() const canard_mem_set_t mem = make_std_memory(); // filter_count > 0 with vtable->filter == NULL. - TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, mem, 16U, 0U, 4U)); + TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 4U)); // filter_count > 0 with invalid memory.rx_filters (NULL vtable). { canard_mem_set_t bad_mem = mem; bad_mem.rx_filters.vtable = nullptr; - TEST_ASSERT_FALSE(canard_new(&self, &vtable_with_filter, bad_mem, 16U, 0U, 4U)); + TEST_ASSERT_FALSE(canard_new(&self, &vtable_with_filter, bad_mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 4U)); } // NULL vtable. - TEST_ASSERT_FALSE(canard_new(&self, nullptr, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, nullptr, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); // vtable->now == NULL. { const canard_vtable_t bad = { .now = nullptr, .tx = mock_tx, .filter = nullptr }; - TEST_ASSERT_FALSE(canard_new(&self, &bad, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &bad, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); } // vtable->tx == NULL. { const canard_vtable_t bad = { .now = mock_now, .tx = nullptr, .filter = nullptr }; - TEST_ASSERT_FALSE(canard_new(&self, &bad, mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &bad, mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); } // Invalid memory.tx_transfer (NULL vtable). { canard_mem_set_t bad_mem = mem; bad_mem.tx_transfer.vtable = nullptr; - TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); } // Invalid memory.tx_frame. { canard_mem_set_t bad_mem = mem; bad_mem.tx_frame.vtable = nullptr; - TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); } // Invalid memory.rx_session. { canard_mem_set_t bad_mem = mem; bad_mem.rx_session.vtable = nullptr; - TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); } // Invalid memory.rx_payload. { canard_mem_set_t bad_mem = mem; bad_mem.rx_payload.vtable = nullptr; - TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, 16U, 0U, 0U)); + TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, CANARD_IFACE_BITMAP_ALL, 16U, 0U, 0U)); } } @@ -968,6 +1026,8 @@ int main() // Poll behavior. RUN_TEST(test_poll_filter_reconfiguration); RUN_TEST(test_poll_filter_after_unsubscribe); + RUN_TEST(test_poll_filter_after_duplicate_subscribe); + RUN_TEST(test_poll_filter_configured_after_new); RUN_TEST(test_poll_session_cleanup); RUN_TEST(test_poll_deadline_then_tx); diff --git a/tests/src/test_api_roundtrip.cpp b/tests/src/test_api_roundtrip.cpp index e708e87..ed4a2b5 100644 --- a/tests/src/test_api_roundtrip.cpp +++ b/tests/src/test_api_roundtrip.cpp @@ -160,7 +160,7 @@ static void init_tx(canard_t* const tx, full_tx_capture_t* const cap, const uint *cap = full_tx_capture_t{}; cap->now = 0; cap->count = 0; - TEST_ASSERT_TRUE(canard_new(tx, &tx_vtable, make_std_memory(), 64U, 0xCAFEU, 0U)); + TEST_ASSERT_TRUE(canard_new(tx, &tx_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 64U, 0xCAFEU, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(tx, node_id)); tx->user_context = cap; } @@ -171,7 +171,7 @@ static void init_rx(canard_t* const rx, rx_context_t* const ctx, const uint_leas *ctx = rx_context_t{}; ctx->now_val = 0; ctx->capture = nullptr; - TEST_ASSERT_TRUE(canard_new(rx, &rx_vtable, make_std_memory(), 16U, 0xBEEFU, 0U)); + TEST_ASSERT_TRUE(canard_new(rx, &rx_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 0xBEEFU, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(rx, node_id)); rx->user_context = ctx; } diff --git a/tests/src/test_api_rx.cpp b/tests/src/test_api_rx.cpp index c22f987..f3ac4a5 100644 --- a/tests/src/test_api_rx.cpp +++ b/tests/src/test_api_rx.cpp @@ -39,7 +39,7 @@ static canard_mem_set_t make_std_memory() static void init_canard(canard_t* const self, canard_us_t* const now_val, const uint_least8_t node_id) { *now_val = 0; - TEST_ASSERT_TRUE(canard_new(self, &test_vtable, make_std_memory(), 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_new(self, &test_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(self, node_id)); self->user_context = now_val; } @@ -503,6 +503,58 @@ static void test_unsubscribe_cleans_up_sessions() canard_destroy(&self); } +// Regression (CN-04): unsubscribing the receiving subscription from within on_message must be safe. +static void unsubscribe_on_message(canard_subscription_t* const self, + const canard_us_t, + const canard_prio_t, + const uint_least8_t, + const uint_least8_t, + // cppcheck-suppress passedByValueCallback + const canard_payload_t payload) +{ + auto* const cap = static_cast(self->user_context); + cap->count++; + TEST_ASSERT_NULL(payload.origin.data); // single-frame transfer: no owned storage + canard_unsubscribe(self->owner, self); +} +static const canard_subscription_vtable_t unsubscribe_sub_vtable = { .on_message = unsubscribe_on_message }; + +static void test_unsubscribe_within_callback() +{ + // Instrumented session memory: fill-on-free turns a stale post-callback read into a detectable failure. + instrumented_allocator_t ses_alloc; + instrumented_allocator_t pay_alloc; + instrumented_allocator_new(&ses_alloc); + instrumented_allocator_new(&pay_alloc); + const canard_mem_t std_r = { .vtable = &std_mem_vtable, .context = nullptr }; + const canard_mem_set_t mem = { .tx_transfer = std_r, + .tx_frame = std_r, + .rx_session = instrumented_allocator_make_resource(&ses_alloc), + .rx_payload = instrumented_allocator_make_resource(&pay_alloc), + .rx_filters = std_r }; + canard_t self = {}; + canard_us_t now_val = 0; + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_set_node_id(&self, 42U)); + self.user_context = &now_val; + + rx_capture_t cap = {}; + canard_subscription_t sub = {}; + TEST_ASSERT_EQUAL_PTR(&sub, canard_subscribe_16b(&self, &sub, 601U, 64U, 2000000, &unsubscribe_sub_vtable)); + sub.user_context = (&cap); + + const uint32_t can_id = make_v1v1_msg_can_id(canard_prio_nominal, 601U, 24U); + const uint_least8_t frame[] = { 0xAAU, make_v1_single_tail(0U) }; + const canard_bytes_t can_data = { .size = sizeof(frame), .data = frame }; + now_val = 100; + TEST_ASSERT_TRUE(canard_ingest_frame(&self, 100, 0U, can_id, can_data)); + + TEST_ASSERT_EQUAL_size_t(1U, cap.count); + TEST_ASSERT_NULL(canard_find_subscription(&self, canard_kind_message_16b, 601U)); + TEST_ASSERT_EQUAL_size_t(0U, ses_alloc.allocated_fragments); + canard_destroy(&self); +} + // ------------------------------------------- v0 Argument Validation ------------------------------------------------ static void test_v0_subscribe_null_args() @@ -777,6 +829,7 @@ int main() RUN_TEST(test_unsubscribe_stops_delivery); RUN_TEST(test_multiple_subscriptions_routing); RUN_TEST(test_unsubscribe_cleans_up_sessions); + RUN_TEST(test_unsubscribe_within_callback); // v0 argument validation. RUN_TEST(test_v0_subscribe_null_args); diff --git a/tests/src/test_api_rx_edge.cpp b/tests/src/test_api_rx_edge.cpp index 93a5aed..81ad2b6 100644 --- a/tests/src/test_api_rx_edge.cpp +++ b/tests/src/test_api_rx_edge.cpp @@ -90,7 +90,7 @@ static canard_mem_set_t make_std_memory() static void init_canard(canard_t* const self, canard_us_t* const now_val, const uint_least8_t node_id) { *now_val = 0; - TEST_ASSERT_TRUE(canard_new(self, &test_vtable, make_std_memory(), 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_new(self, &test_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(self, node_id)); self->user_context = now_val; } diff --git a/tests/src/test_api_tx.cpp b/tests/src/test_api_tx.cpp index ae82ff3..89d12e4 100644 --- a/tests/src/test_api_tx.cpp +++ b/tests/src/test_api_tx.cpp @@ -99,7 +99,7 @@ static void init_with_capture_node_id(canard_t* const self, tx_capture_t* const capture->now = 0; capture->accept_tx = true; capture->count = 0; - TEST_ASSERT_TRUE(canard_new(self, &capture_vtable, make_std_memory(), 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_new(self, &capture_vtable, make_std_memory(), CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(self, node_id)); self->user_context = capture; } @@ -114,7 +114,7 @@ static void test_canard_pending_ifaces() { canard_t self = {}; const canard_mem_set_t mem = make_std_memory(); - TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, 16, 1234, 0)); + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16, 1234, 0)); const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; TEST_ASSERT_EQUAL_UINT8(0U, canard_pending_ifaces(&self)); @@ -125,6 +125,68 @@ static void test_canard_pending_ifaces() canard_destroy(&self); } +#if CANARD_IFACE_COUNT >= 2 +static void test_tx_iface_availability_masks_request() +{ + canard_t self = {}; + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), 0b01U, 16U, 1234U, 0U)); + const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; + // canard_request targets ALL ifaces internally; availability masks it down to iface 0. + TEST_ASSERT_TRUE(canard_request(&self, 1000, canard_prio_nominal, 5U, 7U, 0U, payload, nullptr)); + TEST_ASSERT_EQUAL_UINT8(0b01U, canard_pending_ifaces(&self)); + canard_destroy(&self); +} + +static void test_tx_iface_availability_partial_publish() +{ + canard_t self = {}; + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), 0b01U, 16U, 1234U, 0U)); + const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; + TEST_ASSERT_TRUE(canard_publish_16b(&self, 1000, 0b11U, canard_prio_nominal, 10U, 0U, payload, nullptr)); + TEST_ASSERT_EQUAL_UINT8(0b01U, canard_pending_ifaces(&self)); + canard_destroy(&self); +} + +static void test_tx_iface_availability_disjoint_publish() +{ + canard_t self = {}; + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), 0b10U, 16U, 1234U, 0U)); + const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; + TEST_ASSERT_FALSE(canard_publish_16b(&self, 1000, 0b01U, canard_prio_nominal, 10U, 0U, payload, nullptr)); + TEST_ASSERT_EQUAL_UINT8(0U, canard_pending_ifaces(&self)); + TEST_ASSERT_EQUAL_size_t(0U, self.tx.queue_size); + canard_destroy(&self); +} + +static void test_tx_iface_availability_masks_v0_request() +{ + canard_t self = {}; + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), 0b01U, 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_set_node_id(&self, 42U)); + const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; + // canard_v0_request targets ALL ifaces internally; availability masks it down to iface 0. + TEST_ASSERT_TRUE(canard_v0_request(&self, 1000, canard_prio_nominal, 5U, 0x1234U, 7U, 0U, payload, nullptr)); + TEST_ASSERT_EQUAL_UINT8(0b01U, canard_pending_ifaces(&self)); + canard_destroy(&self); +} +#endif + +// A listen-only node (iface_bitmap = 0) drops every enqueue as a clean no-op: nothing spooled, no tx_capacity error. +static void test_tx_iface_availability_listen_only() +{ + canard_t self = {}; + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, make_std_memory(), 0U, 16U, 1234U, 0U)); + TEST_ASSERT_TRUE(canard_set_node_id(&self, 42U)); + const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr }; + TEST_ASSERT_FALSE( + canard_publish_16b(&self, 1000, CANARD_IFACE_BITMAP_ALL, canard_prio_nominal, 10U, 0U, payload, nullptr)); + TEST_ASSERT_FALSE(canard_request(&self, 1000, canard_prio_nominal, 5U, 7U, 0U, payload, nullptr)); + TEST_ASSERT_EQUAL_UINT8(0U, canard_pending_ifaces(&self)); + TEST_ASSERT_EQUAL_size_t(0U, self.tx.queue_size); + TEST_ASSERT_EQUAL_UINT64(0U, self.err.tx_capacity); + canard_destroy(&self); +} + // Golden signatures generated from pydronecan (dronecan 1.0.24), standard uavcan.* types. static void test_canard_v0_crc_seed_from_data_type_signature_golden() { @@ -441,6 +503,13 @@ int main() // Utility API coverage. RUN_TEST(test_canard_pending_ifaces); RUN_TEST(test_canard_v0_crc_seed_from_data_type_signature_golden); +#if CANARD_IFACE_COUNT >= 2 + RUN_TEST(test_tx_iface_availability_masks_request); + RUN_TEST(test_tx_iface_availability_partial_publish); + RUN_TEST(test_tx_iface_availability_disjoint_publish); + RUN_TEST(test_tx_iface_availability_masks_v0_request); +#endif + RUN_TEST(test_tx_iface_availability_listen_only); // TX API validation checks. RUN_TEST(test_canard_publish_validation); diff --git a/tests/src/test_api_tx_queue.cpp b/tests/src/test_api_tx_queue.cpp index 7c417f8..ac5f72f 100644 --- a/tests/src/test_api_tx_queue.cpp +++ b/tests/src/test_api_tx_queue.cpp @@ -126,7 +126,8 @@ static void init_node(canard_t* const self, cap->accept_tx = true; cap->count = 0; mem_pool_new(pool); - TEST_ASSERT_TRUE(canard_new(self, &capture_vtable, mem_pool_make(pool), queue_capacity, 42U, 0U)); + TEST_ASSERT_TRUE( + canard_new(self, &capture_vtable, mem_pool_make(pool), CANARD_IFACE_BITMAP_ALL, queue_capacity, 42U, 0U)); TEST_ASSERT_TRUE(canard_set_node_id(self, node_id)); self->user_context = cap; } diff --git a/tests/src/test_intrusive_misc.c b/tests/src/test_intrusive_misc.c index 879dc4c..9408718 100644 --- a/tests/src/test_intrusive_misc.c +++ b/tests/src/test_intrusive_misc.c @@ -121,6 +121,7 @@ static void init_tx_canard(canard_t* self, memset(ctx, 0, sizeof(*ctx)); self->user_context = ctx; self->vtable = &tx_test_vtable; + self->iface_bitmap = CANARD_IFACE_BITMAP_ALL; self->tx.queue_capacity = 64U; self->tx.fd = true; self->mem.tx_transfer = instrumented_allocator_make_resource(alloc_transfer); diff --git a/tests/src/test_intrusive_rx.c b/tests/src/test_intrusive_rx.c index a59297b..c4b5a37 100644 --- a/tests/src/test_intrusive_rx.c +++ b/tests/src/test_intrusive_rx.c @@ -180,13 +180,13 @@ static void test_rx_parse_v0_message_golden(void) TEST_ASSERT_EQUAL_HEX8(1, v0.src); TEST_ASSERT_EQUAL_INT(canard_prio_nominal, v0.priority); } - // Anonymous: same frame but src=0. CAN ID = 0x13040A00. + // Anonymous: same frame but src=0. Anon frames carry only the 2 low DTID bits: 0x040A & 0x3 = 2. { const byte_t d[] = { 0xC0 }; // v0 single, tid=0 const canard_bytes_t pl = { sizeof(d), d }; TEST_ASSERT_EQUAL_UINT8(1, rx_parse(0x13040A00UL, pl, &v0, &v1)); TEST_ASSERT_EQUAL_INT(canard_kind_v0_message, v0.kind); - TEST_ASSERT_EQUAL_UINT32(0x040A, v0.port_id); + TEST_ASSERT_EQUAL_UINT32(2, v0.port_id); TEST_ASSERT_EQUAL_HEX8(0xFF, v0.src); // anonymous } // Max: prio=7, type_id=0xFFFF, src=127. CAN ID = 0x1FFFFF7F. @@ -201,6 +201,22 @@ static void test_rx_parse_v0_message_golden(void) } } +// ===================================================================================================================== +// Regression (CN-03): a v0 anonymous message carries only the 2 low DTID bits; the 14-bit discriminator +// in bits [23:10] must not leak into the routed port-ID. +static void test_rx_parse_v0_anonymous_dtid_mask(void) +{ + frame_t v0; + frame_t v1; + // 5-bit prio field=16, discriminator=0x1ABC, low DTID bits=01 (DTID 1), service=0, src=0. CAN ID = 0x106AF100. + const byte_t d[] = { 0x77, 0xC3 }; // v0 single: SOT=1 EOT=1 toggle=0 tid=3 + const canard_bytes_t pl = { sizeof(d), d }; + TEST_ASSERT_EQUAL_UINT8(1, rx_parse(0x106AF100UL, pl, &v0, &v1)); + TEST_ASSERT_EQUAL_INT(canard_kind_v0_message, v0.kind); + TEST_ASSERT_EQUAL_HEX8(0xFF, v0.src); // anonymous + TEST_ASSERT_EQUAL_UINT32(1, v0.port_id); +} + // ===================================================================================================================== // Test 6: v0.1 service golden values. // v0 service CAN ID: (((prio<<2)|3) << 24) | (type_id << 16) | (req?1<<15:0) | (dst << 8) | (1 << 7) | src @@ -1027,6 +1043,7 @@ int main(void) RUN_TEST(test_rx_parse_v1_0_message_golden); RUN_TEST(test_rx_parse_v1_0_service_golden); RUN_TEST(test_rx_parse_v0_message_golden); + RUN_TEST(test_rx_parse_v0_anonymous_dtid_mask); RUN_TEST(test_rx_parse_v0_service_golden); RUN_TEST(test_rx_parse_v1_0_reserved_bit23_reject); RUN_TEST(test_rx_parse_v0_service_zero_node_reject); diff --git a/tests/src/test_intrusive_rx_filter.c b/tests/src/test_intrusive_rx_filter.c index 78819a7..47f6758 100644 --- a/tests/src/test_intrusive_rx_filter.c +++ b/tests/src/test_intrusive_rx_filter.c @@ -134,6 +134,17 @@ static void test_rx_filter_for_subscription_v0_message_semantics(void) TEST_ASSERT_FALSE(filter_accepts(f, 0x00123485UL)); // service flag bit (7) set } +// A v0 message data-type-ID <= 3 can arrive anonymously (2 low DTID bits + random discriminator above); the +// filter is deliberately weakened to admit both anonymous and addressed frames in a single entry. +static void test_rx_filter_for_subscription_v0_message_anonymous_weak(void) +{ + const canard_filter_t f = make_filter(canard_kind_v0_message, 1U, 55U); + TEST_ASSERT_TRUE(filter_accepts(f, 0x00000105UL)); // addressed DTID=1, src=5 + TEST_ASSERT_TRUE(filter_accepts(f, 0x106AF100UL)); // anonymous DTID=1, discriminator=0x1ABC, src=0 + TEST_ASSERT_FALSE(filter_accepts(f, 0x00000205UL)); // DTID=2: different 2 low bits + TEST_ASSERT_FALSE(filter_accepts(f, 0x00000185UL)); // service flag bit (7) set +} + static void test_rx_filter_for_subscription_v0_request_semantics(void) { const canard_filter_t f = make_filter(canard_kind_v0_request, 0x5AU, 42U); @@ -893,7 +904,7 @@ static void test_rx_filter_configure_oom(void) .rx_session = real_mem, .rx_payload = real_mem, .rx_filters = oom_mem }; - TEST_ASSERT_TRUE(canard_new(&self, &test_vtable_with_filter, mem, 16U, 1234U, 4U)); + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable_with_filter, mem, CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 4U)); // Add a subscription so filtering has work to do. canard_subscription_t sub; TEST_ASSERT_EQUAL_PTR(&sub, canard_subscribe_16b(&self, &sub, 100U, 64U, 1000000, &dummy_sub_vtable)); @@ -917,7 +928,7 @@ static void test_rx_filter_configure_coalescence_overflow(void) .rx_payload = real_mem, .rx_filters = real_mem }; // Only 1 hardware filter slot but 2 subscriptions → overflow into coalescence. - TEST_ASSERT_TRUE(canard_new(&self, &test_vtable_with_filter, mem, 16U, 1234U, 1U)); + TEST_ASSERT_TRUE(canard_new(&self, &test_vtable_with_filter, mem, CANARD_IFACE_BITMAP_ALL, 16U, 1234U, 1U)); canard_subscription_t sub1; canard_subscription_t sub2; TEST_ASSERT_EQUAL_PTR(&sub1, canard_subscribe_16b(&self, &sub1, 100U, 64U, 1000000, &dummy_sub_vtable)); @@ -930,48 +941,46 @@ static void test_rx_filter_configure_coalescence_overflow(void) } // ===================================================================================================================== -// rx_filter_match() +// rx_filter_covered() -static void test_rx_filter_match_empty(void) +static void test_rx_filter_covered_empty(void) { - TEST_ASSERT_FALSE(rx_filter_match(0, NULL, 0x12345678U)); - TEST_ASSERT_FALSE(rx_filter_match(0, NULL, 0U)); + const canard_filter_t g = make_filter(canard_kind_v0_message, 341U, 42); + TEST_ASSERT_FALSE(rx_filter_covered(0, NULL, g)); } -static void test_rx_filter_match_single_hit(void) +static void test_rx_filter_covered_self_and_narrower(void) { const canard_filter_t f = make_filter(canard_kind_message_13b, 7509U, 42); - TEST_ASSERT_TRUE(rx_filter_match(1, &f, f.extended_can_id)); - // Also matches with different source node-ID (bits 6:0 are not masked). - TEST_ASSERT_TRUE(rx_filter_match(1, &f, f.extended_can_id | 1U)); - TEST_ASSERT_TRUE(rx_filter_match(1, &f, f.extended_can_id | 0x7FU)); + TEST_ASSERT_TRUE(rx_filter_covered(1, &f, f)); // a filter covers itself + // A strictly narrower inner (adds a source-node constraint) is still covered. + const canard_filter_t narrower = { .extended_can_id = f.extended_can_id | 1U, + .extended_mask = f.extended_mask | 0x7FU }; + TEST_ASSERT_TRUE(rx_filter_covered(1, &f, narrower)); } -static void test_rx_filter_match_single_miss(void) +static void test_rx_filter_covered_broader_not(void) { - const canard_filter_t f = make_filter(canard_kind_message_13b, 7509U, 42); - // A completely different subject-ID should not match. + // The v0-DTID-7509 vs Cyphal-Heartbeat alias: the v0 filter matches a single Heartbeat representative but + // does not COVER the whole Heartbeat frame set (it constrains reserved bits 22:21 Heartbeat leaves free). + const canard_filter_t v0 = make_filter(canard_kind_v0_message, 7509U, 42); + const canard_filter_t hb = make_filter(canard_kind_message_13b, 7509U, 42); + TEST_ASSERT_FALSE(rx_filter_covered(1, &v0, hb)); + // Different subject-ID under the same kind is likewise not covered. const canard_filter_t other = make_filter(canard_kind_message_13b, 100U, 42); - TEST_ASSERT_FALSE(rx_filter_match(1, &f, other.extended_can_id)); - // v0 message with same numeric ID should also not match (different mask/bits). - const canard_filter_t v0 = make_filter(canard_kind_v0_message, 341U, 42); - TEST_ASSERT_FALSE(rx_filter_match(1, &f, v0.extended_can_id)); + TEST_ASSERT_FALSE(rx_filter_covered(1, &hb, other)); } -static void test_rx_filter_match_multiple(void) +static void test_rx_filter_covered_multiple(void) { const canard_filter_t arr[] = { make_filter(canard_kind_message_16b, 100U, 42), make_filter(canard_kind_message_13b, 200U, 42), make_filter(canard_kind_v0_message, 300U, 42), }; - // Each filter's own CAN ID should be matched. - TEST_ASSERT_TRUE(rx_filter_match(3, arr, arr[0].extended_can_id)); - TEST_ASSERT_TRUE(rx_filter_match(3, arr, arr[1].extended_can_id)); - TEST_ASSERT_TRUE(rx_filter_match(3, arr, arr[2].extended_can_id)); - // Unrelated CAN ID should not match any. - const canard_filter_t unrelated = make_filter(canard_kind_message_13b, 999U, 42); - TEST_ASSERT_FALSE(rx_filter_match(3, arr, unrelated.extended_can_id)); + TEST_ASSERT_TRUE(rx_filter_covered(3, arr, arr[1])); + const canard_filter_t other = make_filter(canard_kind_message_13b, 999U, 42); + TEST_ASSERT_FALSE(rx_filter_covered(3, arr, other)); } // ===================================================================================================================== @@ -1026,7 +1035,7 @@ static canard_t make_instance(const size_t filter_count) .rx_filters = real_mem }; canard_t self; memset(&self, 0, sizeof(self)); - (void)canard_new(&self, &capturing_vtable, mem, 16U, 1234U, filter_count); + (void)canard_new(&self, &capturing_vtable, mem, CANARD_IFACE_BITMAP_ALL, 16U, 1234U, filter_count); g_cap_count = 0; memset(g_cap_filters, 0, sizeof(g_cap_filters)); return self; @@ -1095,6 +1104,68 @@ static void test_rx_filter_configure_forced_both_subscribed(void) canard_destroy(&self); } +// Regression (CN-03): a low-DTID v0 message subscription must also admit anonymous frames, whose CAN ID +// carries only the 2 low DTID bits with a random discriminator above. +static void test_rx_filter_configure_v0_anonymous_admitted(void) +{ + canard_t self = make_instance(4); + canard_subscription_t sub; + TEST_ASSERT_EQUAL_PTR(&sub, canard_v0_subscribe(&self, &sub, 1U, 0xF258U, 64U, 1000000, &dummy_sub_vtable)); + TEST_ASSERT_TRUE(rx_filter_configure(&self)); + // One weak filter for the subscription; it also subsumes the forced Heartbeat/NodeStatus (both DTID % 4 == 1). + TEST_ASSERT_EQUAL_size_t(1U, g_cap_count); + TEST_ASSERT_TRUE(captured_accepts(0x00000105UL)); // addressed DTID=1 from src=5 + TEST_ASSERT_TRUE(captured_accepts(0x106AF100UL)); // anonymous DTID=1, discriminator=0x1ABC, src=0 + TEST_ASSERT_FALSE(captured_accepts(0x106AF200UL)); // anonymous DTID=2 must not match the DTID=1 filter + canard_unsubscribe(&self, &sub); + canard_destroy(&self); +} + +// Regression (review R2): a v0 subscription whose DTID aliases the Cyphal Heartbeat subject in the raw 16-bit +// field must not suppress the forced Heartbeat occupancy filter; a real v1.0 Heartbeat (reserved bits 22:21=11) +// must still be admitted. +static void test_rx_filter_configure_forced_heartbeat_alias(void) +{ + canard_t self = make_instance(4); + canard_subscription_t sub; + TEST_ASSERT_EQUAL_PTR(&sub, + canard_v0_subscribe(&self, &sub, HEARTBEAT_SUBJECT_ID, 0, 64U, 1000000, &dummy_sub_vtable)); + TEST_ASSERT_TRUE(rx_filter_configure(&self)); + TEST_ASSERT_TRUE(captured_accepts((UINT32_C(3) << 21U) | (HEARTBEAT_SUBJECT_ID << 8U) | 1U)); // real v1.0 Heartbeat + canard_unsubscribe(&self, &sub); + canard_destroy(&self); +} + +// Regression (review R2): a Cyphal 13b subscription whose subject aliases the DroneCAN NodeStatus DTID masks +// CAN-ID bit 25, which is a v0 priority bit; it must not suppress the forced NodeStatus occupancy filter. +static void test_rx_filter_configure_forced_nodestatus_alias(void) +{ + canard_t self = make_instance(4); + canard_subscription_t sub; + TEST_ASSERT_EQUAL_PTR(&sub, + canard_subscribe_13b(&self, &sub, NODESTATUS_DTYPE_ID, 64U, 1000000, &dummy_sub_vtable)); + TEST_ASSERT_TRUE(rx_filter_configure(&self)); + // A real v0 NodeStatus whose 5-bit priority sets bit 25 (priority 2) must still be admitted. + TEST_ASSERT_TRUE(captured_accepts((UINT32_C(2) << 24U) | (NODESTATUS_DTYPE_ID << 8U) | 1U)); + canard_unsubscribe(&self, &sub); + canard_destroy(&self); +} + +// Regression (review R3): a Cyphal service-request filter can alias the raw NodeStatus probe field +// (service_id=5, dst=42 encodes the same bits as DTID 341); coverage-based dedup must still admit real +// v0 NodeStatus frames of any priority. +static void test_rx_filter_configure_forced_nodestatus_service_alias(void) +{ + canard_t self = make_instance(4); + TEST_ASSERT_TRUE(canard_set_node_id(&self, 42U)); + canard_subscription_t sub; + TEST_ASSERT_EQUAL_PTR(&sub, canard_subscribe_request(&self, &sub, 5U, 64U, 1000000, &dummy_sub_vtable)); + TEST_ASSERT_TRUE(rx_filter_configure(&self)); + TEST_ASSERT_TRUE(captured_accepts((UINT32_C(2) << 24U) | (NODESTATUS_DTYPE_ID << 8U) | 1U)); // real v0 NodeStatus + canard_unsubscribe(&self, &sub); + canard_destroy(&self); +} + static void test_rx_filter_configure_forced_capacity_1(void) { canard_t self = make_instance(1); @@ -1179,6 +1250,7 @@ int main(void) RUN_TEST(test_rx_filter_for_subscription_v1_0_request_semantics); RUN_TEST(test_rx_filter_for_subscription_v1_0_response_semantics); RUN_TEST(test_rx_filter_for_subscription_v0_message_semantics); + RUN_TEST(test_rx_filter_for_subscription_v0_message_anonymous_weak); RUN_TEST(test_rx_filter_for_subscription_v0_request_semantics); RUN_TEST(test_rx_filter_for_subscription_v0_response_semantics); @@ -1225,15 +1297,19 @@ int main(void) RUN_TEST(test_rx_filter_configure_oom); RUN_TEST(test_rx_filter_configure_coalescence_overflow); - RUN_TEST(test_rx_filter_match_empty); - RUN_TEST(test_rx_filter_match_single_hit); - RUN_TEST(test_rx_filter_match_single_miss); - RUN_TEST(test_rx_filter_match_multiple); + RUN_TEST(test_rx_filter_covered_empty); + RUN_TEST(test_rx_filter_covered_self_and_narrower); + RUN_TEST(test_rx_filter_covered_broader_not); + RUN_TEST(test_rx_filter_covered_multiple); RUN_TEST(test_rx_filter_configure_forced_no_subs); RUN_TEST(test_rx_filter_configure_forced_heartbeat_subscribed); RUN_TEST(test_rx_filter_configure_forced_nodestatus_subscribed); RUN_TEST(test_rx_filter_configure_forced_both_subscribed); + RUN_TEST(test_rx_filter_configure_v0_anonymous_admitted); + RUN_TEST(test_rx_filter_configure_forced_heartbeat_alias); + RUN_TEST(test_rx_filter_configure_forced_nodestatus_alias); + RUN_TEST(test_rx_filter_configure_forced_nodestatus_service_alias); RUN_TEST(test_rx_filter_configure_forced_capacity_1); RUN_TEST(test_rx_filter_configure_forced_capacity_2_no_subs); RUN_TEST(test_rx_filter_configure_forced_with_unrelated_subs); diff --git a/tests/src/test_intrusive_rx_session.c b/tests/src/test_intrusive_rx_session.c index 05e8dfc..0c313b5 100644 --- a/tests/src/test_intrusive_rx_session.c +++ b/tests/src/test_intrusive_rx_session.c @@ -1351,6 +1351,58 @@ static void test_preemption_independent_slots(void) fixture_check_alloc_balance(&fx); } +// Regression (CN-02): a slower multi-frame transfer's continuation on its original interface must be +// accepted after the session's interface affinity migrated elsewhere (stale&&fresh admission of an +// unrelated transfer at a different priority). Pre-fix this aborted on a false session-affinity assert. +static void test_preemption_iface_affinity_divergence(void) +{ + session_fixture_t fx; + fixture_init_v1(&fx, canard_kind_message_13b, 2222, 64); + + const byte_t f_start[7] = { 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17 }; + + // 1) Multi-frame start on iface 0, prio nominal, tid=5. Session affinity -> iface 0. + frame_t fr_start = make_start_frame( + canard_prio_nominal, canard_kind_message_13b, 2222, CANARD_NODE_ID_ANONYMOUS, 42, 5, f_start, sizeof(f_start)); + TEST_ASSERT_TRUE(feed(&fx, 1 * MEGA, &fr_start, 0)); + TEST_ASSERT_EQUAL_size_t(0, fx.capture.call_count); + + // 2) After the TID timeout, a fresh single-frame transfer on iface 1 at a different priority is admitted + // (stale && fresh) and migrates the session affinity to iface 1. + const byte_t single[] = { 0xAB }; + frame_t fr_other = make_single_frame( + canard_prio_fast, canard_kind_message_13b, 2222, CANARD_NODE_ID_ANONYMOUS, 42, 7, single, sizeof(single)); + TEST_ASSERT_TRUE(feed(&fx, (3 * MEGA) + 1, &fr_other, 1)); + TEST_ASSERT_EQUAL_size_t(1, fx.capture.call_count); + TEST_ASSERT_EQUAL_INT(canard_prio_fast, fx.capture.priority); + + // 3) The first transfer's end frame arrives on iface 0; its slot affinity (iface 0) now differs from the + // session affinity (iface 1). Must be accepted and completed without aborting. + const uint16_t crc = crc_add(CRC_INITIAL, sizeof(f_start), f_start); + byte_t f_end[2]; + f_end[0] = (byte_t)(crc >> 8U); + f_end[1] = (byte_t)(crc & 0xFF); // NOLINT(hicpp-signed-bitwise) + frame_t fr_end = make_cont_frame(canard_prio_nominal, + canard_kind_message_13b, + 2222, + CANARD_NODE_ID_ANONYMOUS, + 42, + 5, + true, + false, + f_end, + sizeof(f_end)); + TEST_ASSERT_TRUE(feed(&fx, (3 * MEGA) + 2, &fr_end, 0)); + TEST_ASSERT_EQUAL_size_t(2, fx.capture.call_count); + TEST_ASSERT_EQUAL_INT(canard_prio_nominal, fx.capture.priority); + TEST_ASSERT_EQUAL_UINT8(5, fx.capture.transfer_id); + TEST_ASSERT_EQUAL_size_t(7, fx.capture.payload.view.size); + TEST_ASSERT_EQUAL_INT(0, memcmp(fx.capture.payload.view.data, f_start, 7)); + + fixture_destroy_all_sessions(&fx); + fixture_check_alloc_balance(&fx); +} + /// A new start frame at the same priority replaces the old slot. static void test_preemption_same_priority_replaces(void) { @@ -2580,6 +2632,7 @@ int main(void) // Group 6: Priority Preemption RUN_TEST(test_preemption_independent_slots); + RUN_TEST(test_preemption_iface_affinity_divergence); RUN_TEST(test_preemption_same_priority_replaces); RUN_TEST(test_all_8_priorities); diff --git a/tests/src/test_intrusive_tx.c b/tests/src/test_intrusive_tx.c index eb00efe..6847d10 100644 --- a/tests/src/test_intrusive_tx.c +++ b/tests/src/test_intrusive_tx.c @@ -60,6 +60,7 @@ static void init_canard(canard_t* const self, memset(ctx, 0, sizeof(*ctx)); self->user_context = ctx; self->vtable = &test_vtable; + self->iface_bitmap = CANARD_IFACE_BITMAP_ALL; self->tx.queue_capacity = queue_capacity; self->mem.tx_transfer = instrumented_allocator_make_resource(alloc); self->mem.tx_frame = instrumented_allocator_make_resource(alloc); @@ -1661,6 +1662,52 @@ static void test_tx_push_refcount_multi_iface(void) TEST_ASSERT_EQUAL_size_t(0U, alloc.allocated_fragments); } +// Availability masking must keep refcount and cursors consistent: requesting both ifaces when only iface 0 is +// available yields a single-iface refcount and no cursor on iface 1 (a leak if popcount and the cursor loop diverge). +static void test_tx_push_iface_availability_partial_refcount(void) +{ + canard_t self; + test_context_t ctx; + instrumented_allocator_t alloc; + init_canard(&self, &ctx, &alloc, 8U); + self.iface_bitmap = 1U; // Only iface 0 available. + + const byte_t data[] = { 0x42U }; + const canard_bytes_chain_t payload = { .bytes = { .size = 1U, .data = data }, .next = NULL }; + tx_transfer_t* const tr = tx_transfer_new(&self, 5000, ((uint32_t)canard_prio_nominal) << PRIO_SHIFT, false, NULL); + TEST_ASSERT_NOT_NULL(tr); + TEST_ASSERT_TRUE(tx_push(&self, tr, false, 3U, 0U, payload, CRC_INITIAL)); + TEST_ASSERT_EQUAL_size_t(1U, self.tx.queue_size); + + const tx_frame_t* const frame = tr->cursor[0]; + TEST_ASSERT_NOT_NULL(frame); + TEST_ASSERT_EQUAL_size_t(1U, frame->refcount); + TEST_ASSERT_NULL(tr->cursor[1]); + + ctx.tx_budget[0] = 1U; + tx_eject_pending(&self, 0U); + TEST_ASSERT_EQUAL_size_t(0U, self.tx.queue_size); + TEST_ASSERT_EQUAL_size_t(0U, alloc.allocated_fragments); +} + +// A transfer whose requested ifaces are all unavailable is freed, not leaked, and nothing is spooled. +static void test_tx_push_iface_availability_disjoint_frees_transfer(void) +{ + canard_t self; + test_context_t ctx; + instrumented_allocator_t alloc; + init_canard(&self, &ctx, &alloc, 8U); + self.iface_bitmap = 2U; // Only iface 1 available. + + const byte_t data[] = { 0x42U }; + const canard_bytes_chain_t payload = { .bytes = { .size = 1U, .data = data }, .next = NULL }; + tx_transfer_t* const tr = tx_transfer_new(&self, 5000, ((uint32_t)canard_prio_nominal) << PRIO_SHIFT, false, NULL); + TEST_ASSERT_NOT_NULL(tr); + TEST_ASSERT_FALSE(tx_push(&self, tr, false, 1U, 0U, payload, CRC_INITIAL)); + TEST_ASSERT_EQUAL_size_t(0U, self.tx.queue_size); + TEST_ASSERT_EQUAL_size_t(0U, alloc.allocated_fragments); +} + // OOM cleanup loop in tx_spool_v0 when failure occurs after the first frame was already allocated (lines 662-668). static void test_canard_v0_spool_oom_mid_chain(void) { @@ -1872,6 +1919,8 @@ int main(void) RUN_TEST(test_tx_expire_boundary); RUN_TEST(test_tx_predict_frame_count_exhaustive); RUN_TEST(test_tx_push_refcount_multi_iface); + RUN_TEST(test_tx_push_iface_availability_partial_refcount); + RUN_TEST(test_tx_push_iface_availability_disjoint_frees_transfer); return UNITY_END(); }