Skip to content

Commit de96db7

Browse files
committed
cyphal: encapsulate libudpard in CyphalUDPInterface (issue-61)
Encapsulate libudpard behind jarnax::cyphal::Interface so applications no longer touch udpard directly. Uses the new CyphalUDPInterface.*pp files as the implementation boundary. jarnax-cyphal-udp: - Move O1HeapPool from nucleo-cyphal app into the module, implement core::Allocator, provide UdpardMemoryResource/UdpardRxMemoryResources factories backed by a single 64 KiB arena. - Add CyphalUDPSocket abstraction (udp::Endpoint, DatagramHandler, Socket with Join/Leave/Send) and MicrosecondClock so the generic module stays host-testable without cortex headers. - Implement CyphalUDPInterface (Interface + Loopable + DatagramHandler): pooled subscriptions (8), RPC ports (8), per-port transfer-ID counters (16), remembered request TIDs (8), Nominal priority, TX queue drain via Socket, RX dispatch via udpardGather into a MaxExtent scratch buffer, and TransportStatistics. nucleo-cyphal: - Depend on jarnax-cyphal-udp instead of raw o1heap/udpard, switch CyphalApp to jarnax::cyphal::O1HeapPool. Tests/mocks: - gtest-cyphal-o1heappool: allocator and Udpard callback coverage. - gtest-cyphal-udpinterface (12 tests): Listen/Remove/IsListening lifecycle, Join-once service group, publish/request/respond, TX-error stats, subject and RPC round-trips via a local UdpardTx producer (no hand-built wire formats), unknown-group/bad-datagram handling. - MockUDPSocket (gmock) mirroring the socket abstraction. - Fix stale over-alignment expectation (o1heap serves at max_align_t). Build: on-host-native-llvm 21/21, on-host-native-clang 21/21, on-target-cortex-m4/m7 (GCC) all pass; 62/62 in jarnax-cyphal. Human work: issue #61 spec, initial O1HeapPool scaffolding, hardware requirements, review of GOTCHAS/PLAN. AI work: socket/interface design, implementation, GoogleTests/mocks, CMake wiring, verification and commit. AI: opencode/muse-spark-1.2-contributor-free (Meta Muse Spark 1.2, contributor-free tier; ~128k context). Refs #61
1 parent 69fd209 commit de96db7

16 files changed

Lines changed: 1399 additions & 138 deletions

File tree

‎GOTCHAS.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,25 @@
154154
- **Root cause:** `modules/memory/source/copy.cpp` defined `void copy(void*, void *const, size_t)` (top-level const pointer = mangles to `void*`), but the header `memory.hpp` declares `void copy(void*, void const*, size_t)` (mangles to `void const*`). The template overload had masked it for years because no caller forced the non-template `void` overload (e.g. `memory::copy(uint8_t[], char[], n)` — mismatched element types defeat template deduction and fall through to the fixed overload).
155155
- **Fix:** changed the definition's parameter to `void const *_src` to match the header. Added Catch2 tests in `modules/memory/tests/catch2-strings.cpp` using mismatched element types (`uint8_t[]` dst, `char const*` src) to force the fixed overload.
156156
- **Gotcha:** top-level `const` on a pointer parameter is dropped during C++ name mangling, but pointee-const is not — `void *const` and `void const *` are different symbols. Keep definitions byte-for-byte consistent with the header declaration.
157+
158+
## 2026-08-22 — Encapsulating libudpard in `CyphalUDPInterface` (issue-61)
159+
160+
- **Gotcha:** generic modules (`add_module` with `NO_BOARDS`) cannot include `jarnax/Ticker.hpp`
161+
because it pulls `cortex/tick.hpp`, which only the cortex *architecture* target exposes on its
162+
include line. The main `jarnax` module gets it via `ARCH cortex`; `jarnax-cyphal-udp` does not.
163+
Fix: define a tiny `MicrosecondClock` abstract interface (core-only types) and inject it instead.
164+
- **Gotcha:** `jarnax::cyphal::PortId` is **not copy-assignable** (its copy assignment is implicitly
165+
deleted, anonymous union of non-trivial-ish members). Store port identity as plain fields
166+
(type/style/value) when a struct needs to hold and overwrite one; do not keep `PortId` members.
167+
- **Gotcha:** `udpardGather` returns 0 for an empty fragment list, so an "empty payload" transfer
168+
(e.g. `uavcan.node.GetInfo` request) looks identical to a gather failure if you gate delivery on
169+
`gathered > 0`. Gate on `transfer.payload_size` instead; empty transfers are valid Cyphal transfers.
170+
- **Gotcha:** libudpard TX responses (`udpardTxRespond`) are addressed to the *client's* RPC multicast
171+
group derived from the client node-ID — not to the group the request arrived on. In tests,
172+
`sent_to == from` is wrong for responses.
173+
- **Gotcha:** o1heap does **not** reject allocation requests whose alignment exceeds max_align_t;
174+
with NDEBUG it serves the block at its own granularity. Don't write tests asserting nullptr for
175+
over-aligned requests. Fixed stale expectation in gtest-cyphal-o1heappool.cpp.
176+
- **Gotcha:** subject-IDs passed to `udpardTxPublish` must be ≤ 8191 (UDPARD_SUBJECT_ID_MAX);
177+
out-of-range IDs return -UDPARD_ERROR_ARGUMENT, which surfaces as an innocent-looking
178+
`Publish() == false` in test producers.

‎PLAN.md‎

Lines changed: 35 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,39 @@
1-
# PLAN: Add gtests for `cyphal::Node::Print` diagnostic records
1+
# PLAN: Encapsulate libudpard in a `cyphal::Interface` implementation
22

3-
Issue: #60 — branch `issue-60` (tracks `develop`; carries the user's
4-
uncommitted diagnostic Record work: `Printer` interface, `Severity` enum,
5-
`Node::Print`, `diagnostic_record_blob_`, `diagnostic_statistics_`).
3+
**Status:** In progress — steps 4–5 complete (socket abstraction + `CyphalUDPInterface` + GoogleTest suite); hypha adapter and app refactor remain.
4+
**Issue:** https://github.com/emrainey/embedded-superloop/issues/61
5+
**Branch (when started):** `issue-61`, tracking `develop`
66

77
## Summary
88

9-
Add GoogleTest coverage for the diagnostic Record publishing added by the
10-
human: `Node::Print(Severity, fmt, ...)` serializes a
11-
`uavcan.diagnostic.Record.1.1` (subject 8184) and publishes it via the
12-
interface.
13-
14-
## Changes
15-
16-
1. **`gtest-cyphal-node.cpp`** — new `TEST_F`s for `Node::Print`:
17-
- publishes a deserializable Record on `DiagnosticRecordSubjectId` with the
18-
expected severity and text,
19-
- text longer than 255 bytes is truncated to the DSDL capacity,
20-
- the `timestamp.microsecond` field reflects the timer,
21-
- `diagnostic_statistics_.passed` increments on a successful publish,
22-
- `diagnostic_statistics_.failed` increments when the interface `Send`
23-
fails.
24-
- `TestNode` accessor `GetDiagnosticStatistics()` exposing the protected
25-
`diagnostic_statistics_`.
26-
27-
## Production fixes required by the tests (all flagged/approved)
28-
29-
2. **`modules/jarnax/source/cyphal/Node.cpp`** — `Node::Print` called
30-
`jarnax::vsnprint`, which is *declared* in `jarnax/print.hpp` but has no
31-
definition anywhere in the repo. Changed to `core::vsnprint` (always linked
32-
into cyphal targets; keeps the test target lean). Approved by human.
33-
34-
3. **`modules/memory/source/copy.cpp`** — the definition was
35-
`copy(void*, void* const, size_t)` while the header declares
36-
`copy(void*, void const*, size_t)`. Different mangled symbols; the overload
37-
had never been linked before. Fixed the definition to match the header.
38-
39-
4. **`modules/memory/tests/catch2-strings.cpp`** — added 2 `TEST_CASE`s
40-
covering the fixed `copy(void*, void const*, size_t)` overload (mismatched
41-
element types to force the non-template overload, as in `Node::Print`).
42-
43-
## Verification
44-
45-
- `cmake --workflow --preset on-host-native-llvm` — 21/21 tests pass,
46-
including 5 new `Print` tests and 2 new memory `copy` tests.
47-
- `cmake --workflow --preset on-host-native-clang` — 21/21 tests pass.
48-
- Cross-builds: `on-target-cortex-m4-gcc-arm-none-eabi` and
49-
`on-target-cortex-m7-gcc-arm-none-eabi` still build clean.
50-
51-
## Notes
52-
53-
- The `Print` impl is the human's uncommitted work on this branch; the two
54-
production fixes above were the only changes needed and were approved before
55-
applying.
9+
Extract the libudpard plumbing inlined in `applications/nucleo-cyphal/source/CyphalApp.cpp` (~400 of 795 lines) into a reusable `jarnax::cyphal::CyphalUDPInterface` implementing `jarnax::cyphal::Interface`. Applications stop touching udpard directly.
10+
11+
## Estimate
12+
13+
4–7 days total including tests.
14+
15+
## Steps
16+
17+
1. Branch `issue-61` off `develop`.
18+
2. [complete] Move the shared O1Heap pool into `jarnax-cyphal-udp`, implement `core::Allocator`, and provide libudpard memory-resource factories.
19+
3. [complete] Add host-runnable GoogleTest coverage for the allocator and libudpard callbacks.
20+
4. [complete] Socket abstraction: `source/include/jarnax/services/CyphalUDPSocket.hpp` (`udp::Endpoint`, `DatagramHandler`, `Socket` with Join/Leave/Send). Time comes from an injected `MicrosecondClock` (generic modules cannot include cortex headers).
21+
5. [complete] `CyphalUDPInterface` in `source/services/CyphalUDPInterface.cpp` + internal header: all 6 `Interface` virtuals + `Loopable::Execute` TX drain + `DatagramHandler::OnDatagramReceived` RX dispatch; per-port transfer-ID counters; priority fixed Nominal; remembered request transfer-IDs for responses; fragment gather into scratch buffer; statistics.
22+
6. Hypha adapter for the socket abstraction.
23+
7. [complete] GoogleTest suite `tests/gtest-cyphal-udpinterface.cpp` with `tests/mocks/jarnax/services/MockUDPSocket.hpp`: Listen/Remove/IsListening lifecycle, Join-once semantics, Send publish/request/respond, TX error stats, RX subject round-trip, RPC request→response round-trip, unknown-group/bad-datagram handling. Valid RX datagrams are produced with a local `UdpardTx` producer (no hand-built wire formats). 62/62 pass on LLVM and AppleClang; both cross presets build.
24+
8. Refactor `CyphalApp` onto `CyphalUDPInterface`; verify on hardware via pylink/RTT + yactui.
25+
9. Run all presets and `./scripts/build-all-presets.sh`; PR against `develop`.
26+
27+
## Key design decisions
28+
29+
- Do **not** extend `Metadata` with priority/transfer-ID yet — priority is constant Nominal; per-port transfer-ID counters live inside the interface; response transfers echo the transfer-ID of the last remembered request from that client.
30+
- Constructor takes `O1HeapPool&`, node-ID, `udp::Socket&`, and `MicrosecondClock&` — no singletons reached from inside; no cortex dependency so the generic module stays host-testable.
31+
- Single redundant interface initially; `TransportStatistics` reports one interface entry.
32+
- RX transfers are gathered into a contiguous scratch buffer bounded by `MaxExtent`; empty transfers are delivered as zero-length messages.
33+
- Service ports share one RPC multicast group; Join is issued once for the first port and Leave when the last port is removed.
34+
35+
## Acceptance criteria
36+
37+
- No udpard types leak into application code.
38+
- All host unit tests pass on LLVM and AppleClang; cross builds unbroken.
39+
- nucleo-cyphal behaves identically on hardware.

‎applications/nucleo-cyphal/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ add_firmware(NAME nucleo-cyphal
33
${CMAKE_CURRENT_SOURCE_DIR}/source/GlobalContext.cpp
44
${CMAKE_CURRENT_SOURCE_DIR}/source/CyphalApp.cpp
55
${CMAKE_CURRENT_SOURCE_DIR}/source/GetInfoScanner.cpp
6-
${CMAKE_CURRENT_SOURCE_DIR}/source/O1HeapPool.cpp
76
INCLUDES
87
${CMAKE_CURRENT_SOURCE_DIR}/include
98
DEFINES
@@ -12,7 +11,8 @@ add_firmware(NAME nucleo-cyphal
1211
cyphal-dsdl
1312
hypha-ip
1413
udpard
15-
o1heap
14+
GENERIC_MODULES
15+
jarnax-cyphal-udp
1616
CONFIGURATIONS
1717
basic
1818
BOARDS

‎applications/nucleo-cyphal/include/O1HeapPool.hpp‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎applications/nucleo-cyphal/source/CyphalApp.cpp‎

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#include "CyphalApp.hpp"
22

33
#include "GetInfoScanner.hpp"
4-
#include "O1HeapPool.hpp"
54
#include "board.hpp"
65
#include "core/Conversions.hpp"
76
#include "core/vsnprint.hpp"
87
#include "hypha_ip/hypha_ip.h"
98
#include "jarnax/Assertion.hpp"
9+
#include "jarnax/cyphal/O1HeapPool.hpp"
1010
#include "memory.hpp"
1111
#include "segger/rtt.hpp"
1212
#include "stm32/h7xx/ethernet/Driver.hpp"
@@ -75,17 +75,6 @@ HyphaIpSpan_t MakeSpan(void const* data, std::size_t size) {
7575
return span;
7676
}
7777

78-
void* UdpardAlloc(void* context, size_t size) {
79-
auto* heap = static_cast<O1HeapInstance*>(context);
80-
return o1heapAllocate(heap, size);
81-
}
82-
83-
void UdpardFree(void* context, size_t size, void* pointer) {
84-
(void)size;
85-
auto* heap = static_cast<O1HeapInstance*>(context);
86-
o1heapFree(heap, pointer);
87-
}
88-
8978
bool SameIPv4Address(HyphaIpIPv4Address_t const& a, HyphaIpIPv4Address_t const& b) {
9079
static_assert(sizeof(a) == sizeof(uint32_t), "Must be exactly this size");
9180
return std::memcmp(&a, &b, sizeof(a)) == 0;
@@ -288,22 +277,13 @@ bool CyphalApp::Execute() {
288277
}
289278

290279
void CyphalApp::InitUdpard() {
291-
O1HeapInstance& heap = O1HeapPool::Instance();
280+
jarnax::cyphal::O1HeapPool& heap = jarnax::cyphal::O1HeapPool::Instance();
292281

293282
// v1.x memory resources wrap the same O1Heap-backed allocator.
294-
struct UdpardMemoryResource const memory = {
295-
.user_reference = &heap,
296-
.deallocate = &UdpardFree,
297-
.allocate = &UdpardAlloc,
298-
};
283+
UdpardMemoryResource const memory = heap.GetMemoryResource();
299284
tx_memory_ = memory;
300285

301-
rx_memory_.session = memory;
302-
rx_memory_.fragment = memory;
303-
rx_memory_.payload = {
304-
.user_reference = &heap,
305-
.deallocate = &UdpardFree,
306-
};
286+
rx_memory_ = heap.GetRxMemoryResources();
307287

308288
// TX pipeline: single (non-redundant) interface, capacity bounded by the O1Heap.
309289
int_fast8_t const tx_init = udpardTxInit(&tx_, &node_id_, 32U, tx_memory_);
@@ -328,22 +308,10 @@ void CyphalApp::InitUdpard() {
328308
}
329309

330310
void CyphalApp::ServiceDispatcherInit() {
331-
O1HeapInstance& heap = O1HeapPool::Instance();
311+
jarnax::cyphal::O1HeapPool& heap = jarnax::cyphal::O1HeapPool::Instance();
332312

333313
// The RPC dispatcher shares the same O1Heap-backed memory resources as the subjects pipeline.
334-
struct UdpardMemoryResource const memory = {
335-
.user_reference = &heap,
336-
.deallocate = &UdpardFree,
337-
.allocate = &UdpardAlloc,
338-
};
339-
struct UdpardRxMemoryResources const dispatcher_memory = {
340-
.session = memory,
341-
.fragment = memory,
342-
.payload = {
343-
.user_reference = &heap,
344-
.deallocate = &UdpardFree,
345-
},
346-
};
314+
UdpardRxMemoryResources const dispatcher_memory = heap.GetRxMemoryResources();
347315

348316
int_fast8_t const dispatcher_init = udpardRxRPCDispatcherInit(&service_dispatcher_, dispatcher_memory);
349317
if (dispatcher_init < 0) {
@@ -513,8 +481,8 @@ HyphaIpStatus_e CyphalApp::OnReceiveUdp(HyphaIpExternalContext_t context, HyphaI
513481
return HyphaIpStatusOk;
514482
}
515483

516-
O1HeapInstance& heap = O1HeapPool::Instance();
517-
void* copy = o1heapAllocate(&heap, payload_size);
484+
jarnax::cyphal::O1HeapPool& heap = jarnax::cyphal::O1HeapPool::Instance();
485+
void* copy = heap.allocate(payload_size);
518486
if (copy == nullptr) {
519487
return HyphaIpStatusOutOfMemory;
520488
}
@@ -540,7 +508,7 @@ HyphaIpStatus_e CyphalApp::OnReceiveUdp(HyphaIpExternalContext_t context, HyphaI
540508
udpardRxFragmentFree(transfer.base.payload, self->rx_memory_.fragment, self->rx_memory_.payload);
541509
}
542510
} else {
543-
o1heapFree(&heap, copy);
511+
heap.deallocate(copy, payload_size);
544512
}
545513
return HyphaIpStatusOk;
546514
}

‎applications/nucleo-cyphal/source/O1HeapPool.cpp‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎modules/jarnax/CMakeLists.txt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,15 @@ add_module(NAME jarnax-cyphal-can
162162
add_module(NAME jarnax-cyphal-udp
163163
SOURCES
164164
${CMAKE_CURRENT_SOURCE_DIR}/source/cyphal/Node.cpp
165+
${CMAKE_CURRENT_SOURCE_DIR}/source/cyphal/O1HeapPool.cpp
166+
${CMAKE_CURRENT_SOURCE_DIR}/source/services/CyphalUDPInterface.cpp
165167
INCLUDES
166168
${CMAKE_CURRENT_SOURCE_DIR}/include
167169
${CMAKE_CURRENT_SOURCE_DIR}/source/include # Internal includes
168170
DEFINES
169171
JARNAX_CYPHAL_USE_UDP
170172
LIBRARIES
171-
strict jarnax-interfaces cyphal-dsdl
173+
strict jarnax-interfaces cyphal-dsdl o1heap udpard
172174
GENERIC_MODULES
173175
memory core
174176
NO_CONFIGURATIONS
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#ifndef JARNAX_CYPHAL_O1HEAP_POOL_HPP
2+
#define JARNAX_CYPHAL_O1HEAP_POOL_HPP
3+
4+
#include <cstddef>
5+
#include <cstdint>
6+
7+
#include "core/Allocator.hpp"
8+
9+
extern "C" {
10+
#include "udpard.h"
11+
}
12+
13+
namespace jarnax {
14+
namespace cyphal {
15+
16+
/// @brief A fixed O1Heap arena shared by Cyphal/UDP allocations.
17+
class O1HeapPool final : public core::Allocator {
18+
public:
19+
static O1HeapPool& Instance();
20+
21+
static constexpr std::size_t ArenaSize{65536U};
22+
23+
void* allocate(std::size_t bytes, std::size_t alignment = alignof(std::max_align_t)) override;
24+
void deallocate(void* pointer, std::size_t bytes, std::size_t alignment = alignof(std::max_align_t)) override;
25+
26+
UdpardMemoryResource GetMemoryResource();
27+
UdpardRxMemoryResources GetRxMemoryResources();
28+
29+
private:
30+
O1HeapPool() = default;
31+
32+
static void* UdpardAllocate(void* context, std::size_t bytes);
33+
static void UdpardDeallocate(void* context, std::size_t bytes, void* pointer);
34+
};
35+
36+
} // namespace cyphal
37+
} // namespace jarnax
38+
39+
#endif // JARNAX_CYPHAL_O1HEAP_POOL_HPP

0 commit comments

Comments
 (0)