misc

Author: pavel-kirienko

AGENTS.md

@@ -1,14 +1,25 @@
 # LibCANard instructions for AI agents
 
-Please read `README.md` for general information about the library, and `CONTRIBUTING.md` for development-related notes.
+Read `README.md` for general information about the library, and `CONTRIBUTING.md` for development-related notes.
 
-Keep the code and comments very brief. Be sure every significant code block is preceded with a brief comment.
-
-If you need a build directory, create one in the project root named with a `build` prefix;
-you can also use existing build directories if you prefer so,
-but avoid using `cmake-build-*` because these are used by CLion.
-When building the code, don't hesitate to use multiple jobs to use all CPU cores.
+Build directories should be created in the project root named with a `build` prefix. Parallelize compilation.
 
 Run all tests in debug build to ensure that all assertion checks are enabled.
 
-It is best to use Clang-Format to format the code when done editing.
+## Style conventions
+
+Language targets: C99 for the library, C99 and C++20 for the test harness. Strict std only, compiler extensions not allowed.
+
+Naming patterns: `canard_*` functions, `canard_*_t` types, `CANARD_*` macros. Internal definitions need no prefixing. Enums and constants are `lower_snake_case`. Uppercase only for macros.
+
+Keep code compact and add brief comments before non-obvious logic.
+
+Treat warnings as errors and keep compatibility with strict warning flags.
+
+For agent-authored commits, set `GIT_AUTHOR_NAME="Agent"` and `GIT_COMMITTER_NAME="Agent"`.
+
+## Adversarial validation and verification
+
+Practice an adversarial approach to testing: the purpose of a test case is not to provide coverage, but to empirically prove correctness of the tested code. Always treat the code as suspect; you will be rewarded for pointing out flaws in it. If the code does not appear to be correct, refuse to test it and provide evidence of its defects instead of proceeding with testing.
+
+When using subagents to implement tests, always instruct them to summarize their findings concerning the correctness of the tested code and its possible limitations at the end of their run. At the end of the turn, provide a summary of the findings.

libcanard/canard.c

@@ -538,11 +538,11 @@ static tx_frame_t* tx_spool(canard_t* const            self,
                             const uint16_t             crc_seed,
                             const size_t               mtu,
                             const byte_t               transfer_id,
+                            const size_t               size,
                             const canard_bytes_chain_t payload)
 {
     bytes_chain_reader_t reader = { .cursor = &payload, .position = 0U };
     tx_frame_t*          head   = NULL;
-    const size_t         size   = bytes_chain_size(payload);
     bool                 toggle = true; // Cyphal transfers start with toggle==1, unlike legacy
     if (size < mtu) {                   // Single-frame transfer; no CRC required -- easy case.
         const size_t frame_size = tx_ceil_frame_payload_size(size + 1U);
@@ -623,10 +623,10 @@ static tx_frame_t* tx_spool(canard_t* const            self,
 static tx_frame_t* tx_spool_v0(canard_t* const            self,
                                const uint16_t             crc_seed,
                                const byte_t               transfer_id,
+                               const size_t               size,
                                const canard_bytes_chain_t payload)
 {
-    bool         toggle = false; // in v0, toggle starts with zero; that's how v0/v1 can be distinguished
-    const size_t size   = bytes_chain_size(payload);
+    bool toggle = false;                 // in v0, toggle starts with zero; that's how v0/v1 can be distinguished
     if (size < CANARD_MTU_CAN_CLASSIC) { // single-frame transfer
         tx_frame_t* const item = tx_frame_new(self, size + 1U);
         if (item != NULL) {
@@ -723,10 +723,11 @@ static bool tx_push(canard_t* const            self,
 {
     CANARD_ASSERT(tr != NULL);
     CANARD_ASSERT((!tr->fd) || !v0); // The caller must ensure this.
+    CANARD_ASSERT(iface_bitmap != 0);
 
     // Ensure the queue has enough space. v0 transfers always use Classic CAN regardless of tr->fd.
     const size_t mtu      = tr->fd ? CANARD_MTU_CAN_FD : CANARD_MTU_CAN_CLASSIC;
-    const size_t size     = bytes_chain_size(payload); // TODO: pass the precomputed size into spool functions
+    const size_t size     = bytes_chain_size(payload);
     const size_t n_frames = tx_predict_frame_count(size, mtu);
     CANARD_ASSERT(n_frames > 0);
     if (!tx_ensure_queue_space(self, n_frames)) {
@@ -737,8 +738,8 @@ static bool tx_push(canard_t* const            self,
 
     // Make a shared frame spool. Unlike the Cyphal/UDP implementation, we require all ifaces to use the same MTU.
     const size_t      queue_size_before = self->tx.queue_size;
-    tx_frame_t* const spool             = v0 ? tx_spool_v0(self, crc_seed, tr->transfer_id, payload)
-                                             : tx_spool(self, crc_seed, mtu, tr->transfer_id, payload);
+    tx_frame_t* const spool             = v0 ? tx_spool_v0(self, crc_seed, tr->transfer_id, size, payload)
+                                             : tx_spool(self, crc_seed, mtu, tr->transfer_id, size, payload);
     if (spool == NULL) {
         self->err.oom++;
         mem_free(self->mem.tx_transfer, sizeof(canard_txfer_t), tr);

tests/src/test_api_tx.cpp

@@ -34,15 +34,12 @@ static bool        mock_tx(canard_t* const,
 static bool mock_filter(canard_t* const, const size_t, const canard_filter_t*) { return true; }
 
 // Shared vtable and memory resources used by canard_new() tests.
-static const canard_vtable_t kTestVtable = { .now        = mock_now,
+static const canard_vtable_t test_vtable = { .now        = mock_now,
                                              .on_unicast = nullptr,
                                              .tx         = mock_tx,
                                              .filter     = mock_filter };
 
-static const canard_mem_vtable_t kStdMemVtable = {
-    .free  = std_free_mem,
-    .alloc = std_alloc_mem,
-};
+static const canard_mem_vtable_t std_mem_vtable = { .free = std_free_mem, .alloc = std_alloc_mem };
 
 // Captures outgoing TX callback invocations for API-level poll/unicast checks.
 struct tx_record_t
@@ -93,7 +90,7 @@ static bool capture_tx(canard_t* const self,
     return cap->accept_tx;
 }
 
-static const canard_vtable_t kCaptureVtable = {
+static const canard_vtable_t capture_vtable = {
     .now        = capture_now,
     .on_unicast = nullptr,
     .tx         = capture_tx,
@@ -102,7 +99,7 @@ static const canard_vtable_t kCaptureVtable = {
 
 static canard_mem_set_t make_std_memory()
 {
-    const canard_mem_t r = { .vtable = &kStdMemVtable, .context = nullptr };
+    const canard_mem_t r = { .vtable = &std_mem_vtable, .context = nullptr };
     return canard_mem_set_t{
         .tx_transfer = r,
         .tx_frame    = r,
@@ -117,7 +114,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, &kCaptureVtable, make_std_memory(), 16U, node_id, 1234U, 0U, nullptr));
+    TEST_ASSERT_TRUE(canard_new(self, &capture_vtable, make_std_memory(), 16U, node_id, 1234U, 0U, nullptr));
     self->user_context = capture;
 }
 
@@ -141,13 +138,13 @@ static void test_canard_new_validation()
            .rx_payload  = bad_mr,
     };
 
-    TEST_ASSERT_FALSE(canard_new(nullptr, &kTestVtable, mem, 16, 1, 1234, 0, nullptr)); // Invalid self.
+    TEST_ASSERT_FALSE(canard_new(nullptr, &test_vtable, mem, 16, 1, 1234, 0, nullptr)); // Invalid self.
     TEST_ASSERT_FALSE(canard_new(&self, nullptr, mem, 16, 1, 1234, 0, nullptr));        // Invalid vtable.
     TEST_ASSERT_FALSE(
-      canard_new(&self, &kTestVtable, mem, 16, CANARD_NODE_ID_MAX + 1U, 1234, 0, nullptr)); // Invalid node-ID.
-    TEST_ASSERT_FALSE(canard_new(&self, &kTestVtable, mem, 16, 1, 1234, 1, nullptr));       // Missing filter storage.
-    TEST_ASSERT_FALSE(canard_new(&self, &kTestVtable, bad_mem, 16, 1, 1234, 0, nullptr));   // Invalid memory callbacks.
-    TEST_ASSERT_TRUE(canard_new(&self, &kTestVtable, mem, 16, 1, 1234, 1, &filters));       // Valid constructor call.
+      canard_new(&self, &test_vtable, mem, 16, CANARD_NODE_ID_MAX + 1U, 1234, 0, nullptr)); // Invalid node-ID.
+    TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, mem, 16, 1, 1234, 1, nullptr));       // Missing filter storage.
+    TEST_ASSERT_FALSE(canard_new(&self, &test_vtable, bad_mem, 16, 1, 1234, 0, nullptr));   // Invalid memory callbacks.
+    TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, 16, 1, 1234, 1, &filters));       // Valid constructor call.
     canard_destroy(&self);
 }
 
@@ -156,7 +153,7 @@ static void test_canard_new_and_destroy()
 {
     canard_t               self = {};
     const canard_mem_set_t mem  = make_std_memory();
-    TEST_ASSERT_TRUE(canard_new(&self, &kTestVtable, mem, 16, 42, 0x0123456789ABCDEFULL, 0, nullptr));
+    TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, 16, 42, 0x0123456789ABCDEFULL, 0, nullptr));
     TEST_ASSERT_EQUAL_UINT8(42U, (uint8_t)self.node_id);
     TEST_ASSERT_TRUE(self.tx.fd);
     TEST_ASSERT_EQUAL_size_t(16U, self.tx.queue_capacity);
@@ -179,7 +176,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, &kTestVtable, mem, 16, 1, 1234, 0, nullptr));
+    TEST_ASSERT_TRUE(canard_new(&self, &test_vtable, mem, 16, 1, 1234, 0, nullptr));
 
     const canard_bytes_chain_t payload = { .bytes = { .size = 0, .data = nullptr }, .next = nullptr };
     TEST_ASSERT_EQUAL_UINT8(0U, canard_pending_ifaces(&self));

tests/src/test_intrusive_tx.c

@@ -73,7 +73,7 @@ static void test_tx_spool_single_frame(void)
     const byte_t               data[]  = { 1U, 2U, 3U, 4U };
     const canard_bytes_chain_t payload = { .bytes = { .size = sizeof(data), .data = data }, .next = NULL };
 
-    tx_frame_t* const head = tx_spool(&self, CRC_INITIAL, CANARD_MTU_CAN_CLASSIC, 7U, payload);
+    tx_frame_t* const head = tx_spool(&self, CRC_INITIAL, CANARD_MTU_CAN_CLASSIC, 7U, sizeof(data), payload);
     TEST_ASSERT_NOT_NULL(head);
     TEST_ASSERT_EQUAL_size_t(1U, count_frames(head));
     TEST_ASSERT_EQUAL_size_t(5U, canard_dlc_to_len[head->dlc]);
@@ -97,7 +97,7 @@ static void test_tx_spool_multi_frame(void)
     }
     const canard_bytes_chain_t payload = { .bytes = { .size = sizeof(data), .data = data }, .next = NULL };
 
-    tx_frame_t* const head = tx_spool(&self, CRC_INITIAL, CANARD_MTU_CAN_CLASSIC, 3U, payload);
+    tx_frame_t* const head = tx_spool(&self, CRC_INITIAL, CANARD_MTU_CAN_CLASSIC, 3U, sizeof(data), payload);
     TEST_ASSERT_NOT_NULL(head);
     TEST_ASSERT_EQUAL_size_t(2U, count_frames(head));
     TEST_ASSERT_EQUAL_HEX8(0xA3, head->data[7]);