From e56eed2aef5dd4b75e2c34666d0e649968592330 Mon Sep 17 00:00:00 2001 From: Khalil Estell Date: Wed, 14 Oct 2026 13:42:54 -0700 Subject: [PATCH 1/4] :sparkles: v6: Add can_interrupt_dispatcher hal::can_interrupt accepts one subscriber at a time. The dispatcher subscribes once to a real CAN receive interrupt and provides N slots. Each slot is a full can_interrupt with its own subscriber and an optional hal::can_dispatch_filter, which matches a message when its ID format (standard or extended) matches and (id & mask) == (filter.id & mask). A slot without a filter receives every message. Messages reach each matching subscriber in slot order, in interrupt context. A slot's filter can only change while it has no subscriber, so the interrupt never reads a half-written filter. operator[] is lifetimebound and deleted on a temporary dispatcher, the dispatcher cannot outlive its interrupt, and its subscription is its last member so it is removed before the slots are destroyed. Compile checks cover the same misuse as the edge-triggered dispatcher. --- v6/modules/can.cppm | 212 +++++++++++++++++- v6/tests/can.test.cpp | 200 +++++++++++++++++ .../compile_checks/dispatcher_analysis.cpp | 32 ++- .../compile_checks/dispatcher_rejects.cpp | 24 +- v6/tests/compile_checks/fixtures.hpp | 25 +++ 5 files changed, 487 insertions(+), 6 deletions(-) diff --git a/v6/modules/can.cppm b/v6/modules/can.cppm index 98d806d..c6099b6 100644 --- a/v6/modules/can.cppm +++ b/v6/modules/can.cppm @@ -15,6 +15,8 @@ module; #include +#include +#include #include #include #include @@ -146,9 +148,9 @@ export [[nodiscard]] constexpr std::uint16_t bit_width( * can bus peripheral. Returns std::nullopt if the target baud rate is not * achievable with the provided operating frequency. */ -export [[nodiscard]] std::optional -calculate_can_bus_divider(hertz p_operating_frequency, - hertz p_target_baud_rate) +export [[nodiscard]] std::optional calculate_can_bus_divider( + hertz p_operating_frequency, + hertz p_target_baud_rate) { can_bus_divider_t timing{}; @@ -301,4 +303,208 @@ private: hal::u32 m_id; std::size_t m_receive_cursor = 0; }; + +/** + * @brief Which messages a `hal::can_interrupt_dispatcher` slot receives + * @ingroup CAN_Utilities + * + * A message matches when it uses the same ID format (standard or extended) + * and `(message.id & mask) == (id & mask)`. A mask with every ID bit set + * matches one ID; a partial mask matches a block of IDs. + * + * ```cpp + * // Standard IDs 0x100 to 0x10F + * hal::can_dispatch_filter{ .id = 0x100, .mask = 0x7F0 }; + * // Only extended ID 0x18FF'0001 + * hal::can_dispatch_filter{ + * .id = 0x18FF'0001, .mask = 0x1FFF'FFFF, .extended = true + * }; + * ``` + */ +export struct can_dispatch_filter +{ + /// ID to compare against, after masking + hal::u32 id = 0; + /// ID bits that must match; zero matches every ID of the format + hal::u32 mask = 0; + /// True to match extended (29-bit) IDs, false for standard (11-bit) IDs + bool extended = false; + + /** + * @param p_message - a received message + * @return true - if the message passes this filter + */ + [[nodiscard]] constexpr bool matches( + can_message const& p_message) const noexcept + { + return p_message.extended == extended && + (p_message.id & mask) == (id & mask); + } + + constexpr bool operator==(can_dispatch_filter const&) const = default; +}; + +/** + * @brief Shares one CAN receive interrupt among several subscribers + * @ingroup CAN_Utilities + * + * `hal::can_interrupt` accepts one subscriber at a time. The dispatcher + * subscribes once to a real `hal::can_interrupt` and provides `SlotCount` + * slots. Each slot is a `hal::can_interrupt` with its own subscriber and an + * optional `hal::can_dispatch_filter`. + * + * Each received message is passed, in slot order and in interrupt context, to + * the subscriber of every slot whose filter accepts it. A slot without a + * filter receives every message. This filters in software after any hardware + * filter, so it only sees messages the hardware already accepted. + * + * Neither the dispatcher nor its slots may outlive the real interrupt, and + * the slots may not outlive the dispatcher. + * + * Usage: + * + * ```cpp + * hal::can_interrupt_dispatcher<2> dispatcher(can_interrupt); + * dispatcher[0].allow(hal::can_dispatch_filter{ .id = 0x120, .mask = 0x7FF }); + * motor_controller motor(dispatcher[0]); + * bus_logger logger(dispatcher[1]); + * ``` + * + * @tparam SlotCount - number of slots + */ +export template +class can_interrupt_dispatcher final : private can_receive_callback +{ +public: + /** + * @brief One subscriber's view of the shared CAN receive interrupt + */ + class slot final : public can_interrupt + { + public: + slot() = default; + slot(slot const&) = delete; + slot& operator=(slot const&) = delete; + slot(slot&&) = delete; + slot& operator=(slot&&) = delete; + ~slot() = default; + + /** + * @brief Choose which messages this slot receives + * + * The filter can only change while the slot has no subscriber, so the + * interrupt never sees a half-written filter. To change it, destroy the + * subscription, call allow(), and subscribe again. + * + * @param p_filter - the messages to receive; std::nullopt for every + * message (the default) + * @throws hal::subscription_busy - if the slot has a subscriber + */ + void allow(std::optional p_filter) + { + if (m_callback != nullptr) { + throw subscription_busy(this); + } + m_filter = p_filter; + } + + /// @return std::optional - the slot's filter + [[nodiscard]] std::optional filter() const noexcept + { + return m_filter; + } + + private: + friend class can_interrupt_dispatcher; + + void dispatch(can_message const& p_message) noexcept + { + // Read the callback once: a subscriber can be removed between messages. + auto* const callback = m_callback; + if (callback == nullptr) { + return; + } + if (m_filter && not m_filter->matches(p_message)) { + return; + } + callback->callback(p_message); + } + + void driver_on_receive(can_receive_callback& p_callback) override + { + if (m_callback != nullptr) { + throw subscription_busy(this); + } + m_callback = &p_callback; + } + + void driver_clear_receive() noexcept override + { + m_callback = nullptr; + } + + can_receive_callback* m_callback = nullptr; + std::optional m_filter; + }; + + /** + * @brief Subscribe to a CAN receive interrupt and provide its slots + * + * @warning Never construct the dispatcher from an interrupt service + * routine. It subscribes to the interrupt, which throws + * `hal::subscription_busy` if the interrupt already has a subscriber. + * + * @param p_interrupt - the CAN receive interrupt to share + * @throws hal::subscription_busy - if the interrupt already has a subscriber + */ + explicit can_interrupt_dispatcher(can_interrupt& p_interrupt + [[clang::lifetimebound]]) + : m_subscription(p_interrupt.on_receive(*this)) + { + } + + can_interrupt_dispatcher(can_interrupt_dispatcher const&) = delete; + can_interrupt_dispatcher& operator=(can_interrupt_dispatcher const&) = delete; + can_interrupt_dispatcher(can_interrupt_dispatcher&&) = delete; + can_interrupt_dispatcher& operator=(can_interrupt_dispatcher&&) = delete; + ~can_interrupt_dispatcher() = default; + + /** + * @brief Access a slot + * + * @param p_index - index of the slot + * @return slot& - the slot, which must not outlive the dispatcher + * @throws hal::out_of_range - if p_index is not less than SlotCount + */ + [[nodiscard]] slot& operator[](std::size_t p_index) & [[clang::lifetimebound]] + { + if (p_index >= SlotCount) { + throw hal::out_of_range(this, + { .m_index = p_index, .m_capacity = SlotCount }); + } + return m_slots[p_index]; + } + + /// A slot of a temporary dispatcher would dangle immediately. + slot& operator[](std::size_t p_index) && = delete; + + /// @return std::size_t - number of slots + [[nodiscard]] static constexpr std::size_t size() noexcept + { + return SlotCount; + } + +private: + void callback(can_message const& p_message) noexcept override + { + for (auto& slot : m_slots) { + slot.dispatch(p_message); + } + } + + std::array m_slots{}; + // Last, so the real interrupt stops calling the dispatcher before the slots + // are destroyed. + subscription m_subscription; +}; } // namespace hal::inline v6 diff --git a/v6/tests/can.test.cpp b/v6/tests/can.test.cpp index 33872ce..563b212 100644 --- a/v6/tests/can.test.cpp +++ b/v6/tests/can.test.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include @@ -306,10 +307,209 @@ void message_finder_test() }; } +struct fake_can_interrupt final : hal::can_interrupt +{ + void receive(hal::can_message const& p_message) + { + if (callback != nullptr) { + callback->callback(p_message); + } + } + + hal::can_receive_callback* callback = nullptr; + +private: + void driver_on_receive(hal::can_receive_callback& p_callback) override + { + if (callback != nullptr) { + throw hal::subscription_busy(this); + } + callback = &p_callback; + } + void driver_clear_receive() noexcept override + { + callback = nullptr; + } +}; + +struct message_recorder final : hal::can_receive_callback +{ + void callback(hal::can_message const& p_message) noexcept override + { + ids.push_back(p_message.id); + } + std::vector ids; +}; + +void can_interrupt_dispatcher_test() +{ + using namespace boost::ut; + + "subscribes to the interrupt for its lifetime"_test = []() { + // Setup + fake_can_interrupt interrupt; + + // Exercise + { + hal::can_interrupt_dispatcher<2> dispatcher(interrupt); + + // Verify + expect(interrupt.callback != nullptr); + } + + // Verify + expect(interrupt.callback == nullptr) << "destroying it unsubscribes"; + }; + + "throws when the interrupt already has a subscriber"_test = []() { + // Setup + fake_can_interrupt interrupt; + message_recorder other; + auto subscription = interrupt.on_receive(other); + + // Exercise + Verify + expect(throws( + [&]() { hal::can_interrupt_dispatcher<2> dispatcher(interrupt); })); + }; + + "forwards every message to every subscribed slot"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<3> dispatcher(interrupt); + message_recorder first; + message_recorder third; + auto first_subscription = dispatcher[0].on_receive(first); + auto third_subscription = dispatcher[2].on_receive(third); + + // Exercise + interrupt.receive({ .id = 0x111 }); + interrupt.receive({ .id = 0x222 }); + + // Verify + expect(that % first.ids == std::vector{ 0x111, 0x222 }); + expect(that % third.ids == std::vector{ 0x111, 0x222 }); + }; + + "a slot accepts one subscriber at a time"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<1> dispatcher(interrupt); + message_recorder first; + message_recorder second; + + // Exercise + Verify + { + auto subscription = dispatcher[0].on_receive(first); + expect(throws( + [&]() { auto busy = dispatcher[0].on_receive(second); })); + } + auto subscription = dispatcher[0].on_receive(second); + interrupt.receive({ .id = 0x333 }); + expect(that % first.ids.size() == 0); + expect(that % second.ids == std::vector{ 0x333 }); + }; + + "operator[] throws out_of_range past the last slot"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<2> dispatcher(interrupt); + + // Exercise + Verify + expect(throws([&]() { (void)dispatcher[2]; })); + expect(that % dispatcher.size() == 2); + }; + + "a slot only receives messages its filter accepts"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<3> dispatcher(interrupt); + message_recorder block; + message_recorder exact; + message_recorder everything; + dispatcher[0].allow(hal::can_dispatch_filter{ .id = 0x100, .mask = 0x7F0 }); + dispatcher[1].allow(hal::can_dispatch_filter{ .id = 0x123, .mask = 0x7FF }); + auto block_subscription = dispatcher[0].on_receive(block); + auto exact_subscription = dispatcher[1].on_receive(exact); + auto everything_subscription = dispatcher[2].on_receive(everything); + + // Exercise + interrupt.receive({ .id = 0x100 }); + interrupt.receive({ .id = 0x10F }); + interrupt.receive({ .id = 0x110 }); + interrupt.receive({ .id = 0x123 }); + + // Verify + expect(that % block.ids == std::vector{ 0x100, 0x10F }); + expect(that % exact.ids == std::vector{ 0x123 }); + expect(that % everything.ids == + std::vector{ 0x100, 0x10F, 0x110, 0x123 }); + }; + + "a filter only matches its own ID format"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<2> dispatcher(interrupt); + message_recorder standard; + message_recorder extended; + dispatcher[0].allow(hal::can_dispatch_filter{ .id = 0x123, .mask = 0x7FF }); + dispatcher[1].allow(hal::can_dispatch_filter{ + .id = 0x123, .mask = 0x1FFF'FFFF, .extended = true }); + auto standard_subscription = dispatcher[0].on_receive(standard); + auto extended_subscription = dispatcher[1].on_receive(extended); + + // Exercise + interrupt.receive({ .id = 0x123, .extended = false }); + interrupt.receive({ .id = 0x123, .extended = true }); + interrupt.receive({ .id = 0x1000'0123, .extended = true }); + + // Verify + expect(that % standard.ids == std::vector{ 0x123 }); + expect(that % extended.ids == std::vector{ 0x123 }) + << "the extended filter rejects a different extended ID with the same " + "low bits"; + }; + + "a zero mask matches every ID of its format"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<1> dispatcher(interrupt); + message_recorder recorder; + dispatcher[0].allow(hal::can_dispatch_filter{ .extended = true }); + auto subscription = dispatcher[0].on_receive(recorder); + + // Exercise + interrupt.receive({ .id = 0x7FF, .extended = false }); + interrupt.receive({ .id = 0x1ABC'DEF0, .extended = true }); + + // Verify + expect(that % recorder.ids == std::vector{ 0x1ABC'DEF0 }); + }; + + "the filter can only change without a subscriber"_test = []() { + // Setup + fake_can_interrupt interrupt; + hal::can_interrupt_dispatcher<1> dispatcher(interrupt); + message_recorder recorder; + auto const filter = hal::can_dispatch_filter{ .id = 0x10, .mask = 0x7FF }; + + // Exercise + Verify + expect(not dispatcher[0].filter().has_value()) << "no filter by default"; + { + auto subscription = dispatcher[0].on_receive(recorder); + expect( + throws([&]() { dispatcher[0].allow(filter); })); + } + dispatcher[0].allow(filter); + expect(dispatcher[0].filter() == filter); + dispatcher[0].allow(std::nullopt); + expect(not dispatcher[0].filter().has_value()); + }; +} } // namespace int main() { divider_test(); message_finder_test(); + can_interrupt_dispatcher_test(); } diff --git a/v6/tests/compile_checks/dispatcher_analysis.cpp b/v6/tests/compile_checks/dispatcher_analysis.cpp index a8eac50..d7798db 100644 --- a/v6/tests/compile_checks/dispatcher_analysis.cpp +++ b/v6/tests/compile_checks/dispatcher_analysis.cpp @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// edge_triggered_interrupt_dispatcher misuse that Clang's lifetime safety -// analysis reports. +// Interrupt dispatcher misuse that Clang's lifetime safety analysis reports. #include "fixtures.hpp" @@ -43,3 +42,32 @@ dispatcher const* dispatcher_outlives_pin() static dispatcher shared(pin); return &shared; } + +using can_dispatcher = hal::can_interrupt_dispatcher<2>; + +hal::can_interrupt& can_slot_outlives_dispatcher( + fake_can_interrupt& p_interrupt) +{ + can_dispatcher shared(p_interrupt); + // expected-error@+2 {{is returned}} + // expected-error@+1 {{reference to stack memory}} + return shared[0]; +} + +hal::subscription can_subscription_outlives_dispatcher( + fake_can_interrupt& p_interrupt) +{ + static message_counter c; + can_dispatcher shared(p_interrupt); + // expected-error@+2 {{is returned}} + // expected-error@+1 {{address of stack memory}} + return shared[0].on_receive(c); +} + +can_dispatcher const* can_dispatcher_outlives_interrupt() +{ + fake_can_interrupt interrupt; // forgot static + // expected-error@+1 {{which will dangle}} + static can_dispatcher shared(interrupt); + return &shared; +} diff --git a/v6/tests/compile_checks/dispatcher_rejects.cpp b/v6/tests/compile_checks/dispatcher_rejects.cpp index 37eec7e..c8d8005 100644 --- a/v6/tests/compile_checks/dispatcher_rejects.cpp +++ b/v6/tests/compile_checks/dispatcher_rejects.cpp @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// edge_triggered_interrupt_dispatcher misuse that the type system rejects. +// Interrupt dispatcher misuse that the type system rejects. #include "fixtures.hpp" @@ -40,3 +40,25 @@ void copy_dispatcher(fake_pin& p_pin) // expected-error@+1 {{call to deleted constructor}} dispatcher copy = shared; } + +using can_dispatcher = hal::can_interrupt_dispatcher<2>; + +void index_temporary_can_dispatcher(fake_can_interrupt& p_interrupt) +{ + // expected-error@+1 {{overload resolution selected deleted operator '[]'}} + auto& slot = can_dispatcher(p_interrupt)[0]; + (void)slot; +} + +void can_dispatcher_on_temporary_interrupt() +{ + // expected-error@+1 {{no matching constructor}} + can_dispatcher shared(fake_can_interrupt{}); +} + +void copy_can_dispatcher(fake_can_interrupt& p_interrupt) +{ + can_dispatcher shared(p_interrupt); + // expected-error@+1 {{call to deleted constructor}} + can_dispatcher copy = shared; +} diff --git a/v6/tests/compile_checks/fixtures.hpp b/v6/tests/compile_checks/fixtures.hpp index 73eb608..692ab72 100644 --- a/v6/tests/compile_checks/fixtures.hpp +++ b/v6/tests/compile_checks/fixtures.hpp @@ -47,3 +47,28 @@ struct counter final : hal::edge_triggered_callback { } }; + +struct fake_can_interrupt final : hal::can_interrupt +{ + hal::can_receive_callback* callback = nullptr; + +private: + void driver_on_receive(hal::can_receive_callback& p_callback) override + { + if (callback) { + throw hal::subscription_busy(this); + } + callback = &p_callback; + } + void driver_clear_receive() noexcept override + { + callback = nullptr; + } +}; + +struct message_counter final : hal::can_receive_callback +{ + void callback(hal::can_message const&) noexcept override + { + } +}; From 96eefe0c4d18f19d2e8587cceddf6b07406f80b9 Mon Sep 17 00:00:00 2001 From: Khalil Estell Date: Thu, 22 Oct 2026 07:16:45 -0700 Subject: [PATCH 2/4] Take dispatcher filters from each subscriber libhal's can_interrupt::on_receive() now takes a hal::can_receive_filter (libhal/libhal#252), so each subscriber passes the filter for the IDs it handles. A slot stores the filter its subscriber passed, and the dispatcher subscribes to the real interrupt for every message. - Drops can_dispatch_filter and slot::allow()/filter(); libhal's can_receive_filter replaces them. It compares IDs only, so one filter works for standard and extended messages. - A compiler fence keeps the filter written before the callback that makes the slot live, since the interrupt reads them in that order. - Requires libhal >= 5.0.0-alpha.5. --- v6/conanfile.py | 2 +- v6/modules/can.cppm | 96 ++++++---------------------- v6/tests/can.test.cpp | 93 ++++++++++++++------------- v6/tests/compile_checks/fixtures.hpp | 3 +- 4 files changed, 72 insertions(+), 122 deletions(-) diff --git a/v6/conanfile.py b/v6/conanfile.py index 3210277..af621ef 100644 --- a/v6/conanfile.py +++ b/v6/conanfile.py @@ -93,7 +93,7 @@ def build_requirements(self): def requirements(self): # libhal v5 is only released as pre-releases (5.0.0-alpha.N) so far. # Remove include_prerelease once 5.0.0 is published. - self.requires("libhal/[>=5.0.0-alpha.1 <6, include_prerelease]") + self.requires("libhal/[>=5.0.0-alpha.5 <6, include_prerelease]") def layout(self): build_path = Path("build") / ( diff --git a/v6/modules/can.cppm b/v6/modules/can.cppm index c6099b6..607ee8b 100644 --- a/v6/modules/can.cppm +++ b/v6/modules/can.cppm @@ -16,6 +16,7 @@ module; #include #include +#include #include #include #include @@ -304,59 +305,20 @@ private: std::size_t m_receive_cursor = 0; }; -/** - * @brief Which messages a `hal::can_interrupt_dispatcher` slot receives - * @ingroup CAN_Utilities - * - * A message matches when it uses the same ID format (standard or extended) - * and `(message.id & mask) == (id & mask)`. A mask with every ID bit set - * matches one ID; a partial mask matches a block of IDs. - * - * ```cpp - * // Standard IDs 0x100 to 0x10F - * hal::can_dispatch_filter{ .id = 0x100, .mask = 0x7F0 }; - * // Only extended ID 0x18FF'0001 - * hal::can_dispatch_filter{ - * .id = 0x18FF'0001, .mask = 0x1FFF'FFFF, .extended = true - * }; - * ``` - */ -export struct can_dispatch_filter -{ - /// ID to compare against, after masking - hal::u32 id = 0; - /// ID bits that must match; zero matches every ID of the format - hal::u32 mask = 0; - /// True to match extended (29-bit) IDs, false for standard (11-bit) IDs - bool extended = false; - - /** - * @param p_message - a received message - * @return true - if the message passes this filter - */ - [[nodiscard]] constexpr bool matches( - can_message const& p_message) const noexcept - { - return p_message.extended == extended && - (p_message.id & mask) == (id & mask); - } - - constexpr bool operator==(can_dispatch_filter const&) const = default; -}; - /** * @brief Shares one CAN receive interrupt among several subscribers * @ingroup CAN_Utilities * * `hal::can_interrupt` accepts one subscriber at a time. The dispatcher - * subscribes once to a real `hal::can_interrupt` and provides `SlotCount` - * slots. Each slot is a `hal::can_interrupt` with its own subscriber and an - * optional `hal::can_dispatch_filter`. + * subscribes once to a real `hal::can_interrupt`, for every message, and + * provides `SlotCount` slots. Each slot is a `hal::can_interrupt` for one + * subscriber, and each subscriber passes its own `hal::can_receive_filter` + * when it subscribes, the same as with a real interrupt. * * Each received message is passed, in slot order and in interrupt context, to - * the subscriber of every slot whose filter accepts it. A slot without a - * filter receives every message. This filters in software after any hardware - * filter, so it only sees messages the hardware already accepted. + * the subscriber of every slot whose filter passes it. This filters in + * software after any hardware filter, so it only sees messages the hardware + * already accepted. * * Neither the dispatcher nor its slots may outlive the real interrupt, and * the slots may not outlive the dispatcher. @@ -365,7 +327,7 @@ export struct can_dispatch_filter * * ```cpp * hal::can_interrupt_dispatcher<2> dispatcher(can_interrupt); - * dispatcher[0].allow(hal::can_dispatch_filter{ .id = 0x120, .mask = 0x7FF }); + * // Each driver subscribes with the filter for the IDs it handles. * motor_controller motor(dispatcher[0]); * bus_logger logger(dispatcher[1]); * ``` @@ -389,31 +351,6 @@ public: slot& operator=(slot&&) = delete; ~slot() = default; - /** - * @brief Choose which messages this slot receives - * - * The filter can only change while the slot has no subscriber, so the - * interrupt never sees a half-written filter. To change it, destroy the - * subscription, call allow(), and subscribe again. - * - * @param p_filter - the messages to receive; std::nullopt for every - * message (the default) - * @throws hal::subscription_busy - if the slot has a subscriber - */ - void allow(std::optional p_filter) - { - if (m_callback != nullptr) { - throw subscription_busy(this); - } - m_filter = p_filter; - } - - /// @return std::optional - the slot's filter - [[nodiscard]] std::optional filter() const noexcept - { - return m_filter; - } - private: friend class can_interrupt_dispatcher; @@ -424,17 +361,26 @@ public: if (callback == nullptr) { return; } - if (m_filter && not m_filter->matches(p_message)) { + // Pairs with the fence in driver_on_receive(): the filter is read after + // the callback, so it is never one written for an earlier subscriber. + std::atomic_signal_fence(std::memory_order_acquire); + if (not m_filter.matches(p_message)) { return; } callback->callback(p_message); } - void driver_on_receive(can_receive_callback& p_callback) override + void driver_on_receive(can_receive_callback& p_callback, + can_receive_filter p_filter) override { if (m_callback != nullptr) { throw subscription_busy(this); } + // The slot has no subscriber, so the interrupt ignores it while the + // filter is written. The fence keeps the compiler from moving that + // write after the callback store that makes the slot live. + m_filter = p_filter; + std::atomic_signal_fence(std::memory_order_release); m_callback = &p_callback; } @@ -444,7 +390,7 @@ public: } can_receive_callback* m_callback = nullptr; - std::optional m_filter; + can_receive_filter m_filter{}; }; /** diff --git a/v6/tests/can.test.cpp b/v6/tests/can.test.cpp index 563b212..92fdc6a 100644 --- a/v6/tests/can.test.cpp +++ b/v6/tests/can.test.cpp @@ -317,14 +317,17 @@ struct fake_can_interrupt final : hal::can_interrupt } hal::can_receive_callback* callback = nullptr; + hal::can_receive_filter filter{}; private: - void driver_on_receive(hal::can_receive_callback& p_callback) override + void driver_on_receive(hal::can_receive_callback& p_callback, + hal::can_receive_filter p_filter) override { if (callback != nullptr) { throw hal::subscription_busy(this); } callback = &p_callback; + filter = p_filter; } void driver_clear_receive() noexcept override { @@ -419,17 +422,29 @@ void can_interrupt_dispatcher_test() expect(that % dispatcher.size() == 2); }; - "a slot only receives messages its filter accepts"_test = []() { + "the dispatcher subscribes to every message"_test = []() { + // Setup + fake_can_interrupt interrupt; + interrupt.filter = { .id = 0x1, .mask = 0x1 }; + + // Exercise + hal::can_interrupt_dispatcher<1> dispatcher(interrupt); + + // Verify + expect(interrupt.filter == hal::can_receive_filter{}); + }; + + "a slot only receives messages its subscriber's filter passes"_test = []() { // Setup fake_can_interrupt interrupt; hal::can_interrupt_dispatcher<3> dispatcher(interrupt); message_recorder block; message_recorder exact; message_recorder everything; - dispatcher[0].allow(hal::can_dispatch_filter{ .id = 0x100, .mask = 0x7F0 }); - dispatcher[1].allow(hal::can_dispatch_filter{ .id = 0x123, .mask = 0x7FF }); - auto block_subscription = dispatcher[0].on_receive(block); - auto exact_subscription = dispatcher[1].on_receive(exact); + auto block_subscription = + dispatcher[0].on_receive(block, { .id = 0x100, .mask = 0x7F0 }); + auto exact_subscription = + dispatcher[1].on_receive(exact, { .id = 0x123, .mask = 0x1FFF'FFFF }); auto everything_subscription = dispatcher[2].on_receive(everything); // Exercise @@ -445,17 +460,16 @@ void can_interrupt_dispatcher_test() std::vector{ 0x100, 0x10F, 0x110, 0x123 }); }; - "a filter only matches its own ID format"_test = []() { + "one filter works for standard and extended IDs"_test = []() { // Setup fake_can_interrupt interrupt; hal::can_interrupt_dispatcher<2> dispatcher(interrupt); - message_recorder standard; - message_recorder extended; - dispatcher[0].allow(hal::can_dispatch_filter{ .id = 0x123, .mask = 0x7FF }); - dispatcher[1].allow(hal::can_dispatch_filter{ - .id = 0x123, .mask = 0x1FFF'FFFF, .extended = true }); - auto standard_subscription = dispatcher[0].on_receive(standard); - auto extended_subscription = dispatcher[1].on_receive(extended); + message_recorder low_bits; + message_recorder whole_id; + auto low_subscription = + dispatcher[0].on_receive(low_bits, { .id = 0x123, .mask = 0x7FF }); + auto whole_subscription = + dispatcher[1].on_receive(whole_id, { .id = 0x123, .mask = 0x1FFF'FFFF }); // Exercise interrupt.receive({ .id = 0x123, .extended = false }); @@ -463,46 +477,35 @@ void can_interrupt_dispatcher_test() interrupt.receive({ .id = 0x1000'0123, .extended = true }); // Verify - expect(that % standard.ids == std::vector{ 0x123 }); - expect(that % extended.ids == std::vector{ 0x123 }) - << "the extended filter rejects a different extended ID with the same " - "low bits"; + expect(that % low_bits.ids == + std::vector{ 0x123, 0x123, 0x1000'0123 }) + << "an 11-bit mask also passes extended IDs with the same low bits"; + expect(that % whole_id.ids == std::vector{ 0x123, 0x123 }) + << "a 29-bit mask rejects extended IDs with other upper bits"; }; - "a zero mask matches every ID of its format"_test = []() { + "a new subscriber brings its own filter"_test = []() { // Setup fake_can_interrupt interrupt; hal::can_interrupt_dispatcher<1> dispatcher(interrupt); - message_recorder recorder; - dispatcher[0].allow(hal::can_dispatch_filter{ .extended = true }); - auto subscription = dispatcher[0].on_receive(recorder); + message_recorder first; + message_recorder second; + { + auto subscription = + dispatcher[0].on_receive(first, { .id = 0x10, .mask = 0x7FF }); + interrupt.receive({ .id = 0x10 }); + interrupt.receive({ .id = 0x20 }); + } // Exercise - interrupt.receive({ .id = 0x7FF, .extended = false }); - interrupt.receive({ .id = 0x1ABC'DEF0, .extended = true }); + auto subscription = dispatcher[0].on_receive(second); + interrupt.receive({ .id = 0x10 }); + interrupt.receive({ .id = 0x20 }); // Verify - expect(that % recorder.ids == std::vector{ 0x1ABC'DEF0 }); - }; - - "the filter can only change without a subscriber"_test = []() { - // Setup - fake_can_interrupt interrupt; - hal::can_interrupt_dispatcher<1> dispatcher(interrupt); - message_recorder recorder; - auto const filter = hal::can_dispatch_filter{ .id = 0x10, .mask = 0x7FF }; - - // Exercise + Verify - expect(not dispatcher[0].filter().has_value()) << "no filter by default"; - { - auto subscription = dispatcher[0].on_receive(recorder); - expect( - throws([&]() { dispatcher[0].allow(filter); })); - } - dispatcher[0].allow(filter); - expect(dispatcher[0].filter() == filter); - dispatcher[0].allow(std::nullopt); - expect(not dispatcher[0].filter().has_value()); + expect(that % first.ids == std::vector{ 0x10 }); + expect(that % second.ids == std::vector{ 0x10, 0x20 }) + << "the second subscriber's default filter replaces the first one's"; }; } } // namespace diff --git a/v6/tests/compile_checks/fixtures.hpp b/v6/tests/compile_checks/fixtures.hpp index 692ab72..fdb5876 100644 --- a/v6/tests/compile_checks/fixtures.hpp +++ b/v6/tests/compile_checks/fixtures.hpp @@ -53,7 +53,8 @@ struct fake_can_interrupt final : hal::can_interrupt hal::can_receive_callback* callback = nullptr; private: - void driver_on_receive(hal::can_receive_callback& p_callback) override + void driver_on_receive(hal::can_receive_callback& p_callback, + hal::can_receive_filter) override { if (callback) { throw hal::subscription_busy(this); From 5aca0c4d4355f6648fa377b7a7b160f27b753f47 Mon Sep 17 00:00:00 2001 From: Khalil Estell Date: Thu, 22 Oct 2026 16:41:06 -0700 Subject: [PATCH 3/4] Share one slot type across CAN dispatcher sizes Move the nested slot out of can_interrupt_dispatcher into hal::can_interrupt_slot, so dispatchers of different sizes no longer instantiate their own slot class and vtable. Use static_cast() instead of a C-style void cast in the tests. --- v6/modules/can.cppm | 136 ++++++++++-------- v6/tests/can.test.cpp | 12 +- .../compile_checks/dispatcher_rejects.cpp | 2 +- 3 files changed, 86 insertions(+), 64 deletions(-) diff --git a/v6/modules/can.cppm b/v6/modules/can.cppm index 607ee8b..14ebc60 100644 --- a/v6/modules/can.cppm +++ b/v6/modules/can.cppm @@ -305,15 +305,80 @@ private: std::size_t m_receive_cursor = 0; }; +export template +class can_interrupt_dispatcher; + +/** + * @brief One subscriber's view of a CAN receive interrupt shared by + * `hal::can_interrupt_dispatcher` + * @ingroup CAN_Utilities + * + * Get slots from a dispatcher's `operator[]`. Every dispatcher, whatever its + * slot count, uses this one type. + */ +export class can_interrupt_slot final : public can_interrupt +{ +public: + can_interrupt_slot() = default; + can_interrupt_slot(can_interrupt_slot const&) = delete; + can_interrupt_slot& operator=(can_interrupt_slot const&) = delete; + can_interrupt_slot(can_interrupt_slot&&) = delete; + can_interrupt_slot& operator=(can_interrupt_slot&&) = delete; + ~can_interrupt_slot() = default; + +private: + template + friend class can_interrupt_dispatcher; + + void dispatch(can_message const& p_message) noexcept + { + // Read the callback once: a subscriber can be removed between messages. + auto* const callback = m_callback; + if (callback == nullptr) { + return; + } + // Pairs with the fence in driver_on_receive(): the filter is read after + // the callback, so it is never one written for an earlier subscriber. + std::atomic_signal_fence(std::memory_order_acquire); + if (not m_filter.matches(p_message)) { + return; + } + callback->callback(p_message); + } + + void driver_on_receive(can_receive_callback& p_callback, + can_receive_filter p_filter) override + { + if (m_callback != nullptr) { + throw subscription_busy(this); + } + // The slot has no subscriber, so the interrupt ignores it while the + // filter is written. The fence keeps the compiler from moving that + // write after the callback store that makes the slot live. + m_filter = p_filter; + std::atomic_signal_fence(std::memory_order_release); + m_callback = &p_callback; + } + + void driver_clear_receive() noexcept override + { + m_callback = nullptr; + } + + can_receive_callback* m_callback = nullptr; + can_receive_filter m_filter{}; +}; + /** * @brief Shares one CAN receive interrupt among several subscribers * @ingroup CAN_Utilities * * `hal::can_interrupt` accepts one subscriber at a time. The dispatcher * subscribes once to a real `hal::can_interrupt`, for every message, and - * provides `SlotCount` slots. Each slot is a `hal::can_interrupt` for one - * subscriber, and each subscriber passes its own `hal::can_receive_filter` - * when it subscribes, the same as with a real interrupt. + * provides `SlotCount` slots. Each slot is a `hal::can_interrupt_slot`, a + * `hal::can_interrupt` for one subscriber, and each subscriber passes its own + * `hal::can_receive_filter` when it subscribes, the same as with a real + * interrupt. * * Each received message is passed, in slot order and in interrupt context, to * the subscriber of every slot whose filter passes it. This filters in @@ -338,61 +403,6 @@ export template class can_interrupt_dispatcher final : private can_receive_callback { public: - /** - * @brief One subscriber's view of the shared CAN receive interrupt - */ - class slot final : public can_interrupt - { - public: - slot() = default; - slot(slot const&) = delete; - slot& operator=(slot const&) = delete; - slot(slot&&) = delete; - slot& operator=(slot&&) = delete; - ~slot() = default; - - private: - friend class can_interrupt_dispatcher; - - void dispatch(can_message const& p_message) noexcept - { - // Read the callback once: a subscriber can be removed between messages. - auto* const callback = m_callback; - if (callback == nullptr) { - return; - } - // Pairs with the fence in driver_on_receive(): the filter is read after - // the callback, so it is never one written for an earlier subscriber. - std::atomic_signal_fence(std::memory_order_acquire); - if (not m_filter.matches(p_message)) { - return; - } - callback->callback(p_message); - } - - void driver_on_receive(can_receive_callback& p_callback, - can_receive_filter p_filter) override - { - if (m_callback != nullptr) { - throw subscription_busy(this); - } - // The slot has no subscriber, so the interrupt ignores it while the - // filter is written. The fence keeps the compiler from moving that - // write after the callback store that makes the slot live. - m_filter = p_filter; - std::atomic_signal_fence(std::memory_order_release); - m_callback = &p_callback; - } - - void driver_clear_receive() noexcept override - { - m_callback = nullptr; - } - - can_receive_callback* m_callback = nullptr; - can_receive_filter m_filter{}; - }; - /** * @brief Subscribe to a CAN receive interrupt and provide its slots * @@ -419,10 +429,12 @@ public: * @brief Access a slot * * @param p_index - index of the slot - * @return slot& - the slot, which must not outlive the dispatcher + * @return can_interrupt_slot& - the slot, which must not outlive the + * dispatcher * @throws hal::out_of_range - if p_index is not less than SlotCount */ - [[nodiscard]] slot& operator[](std::size_t p_index) & [[clang::lifetimebound]] + [[nodiscard]] can_interrupt_slot& operator[]( + std::size_t p_index) & [[clang::lifetimebound]] { if (p_index >= SlotCount) { throw hal::out_of_range(this, @@ -432,7 +444,7 @@ public: } /// A slot of a temporary dispatcher would dangle immediately. - slot& operator[](std::size_t p_index) && = delete; + can_interrupt_slot& operator[](std::size_t p_index) && = delete; /// @return std::size_t - number of slots [[nodiscard]] static constexpr std::size_t size() noexcept @@ -448,7 +460,7 @@ private: } } - std::array m_slots{}; + std::array m_slots{}; // Last, so the real interrupt stops calling the dispatcher before the slots // are destroyed. subscription m_subscription; diff --git a/v6/tests/can.test.cpp b/v6/tests/can.test.cpp index 92fdc6a..0f74d8e 100644 --- a/v6/tests/can.test.cpp +++ b/v6/tests/can.test.cpp @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include #include @@ -27,6 +29,13 @@ using namespace mp_units::si::unit_symbols; namespace { +// Every dispatcher hands out the same slot type, whatever its slot count. +template +using can_slot_of = std::remove_reference_t< + decltype(std::declval&>()[0])>; +static_assert(std::is_same_v, hal::can_interrupt_slot>); +static_assert(std::is_same_v, hal::can_interrupt_slot>); + void check_validity(hal::hertz p_operating_frequency, hal::hertz p_target_baud_rate) { @@ -418,7 +427,8 @@ void can_interrupt_dispatcher_test() hal::can_interrupt_dispatcher<2> dispatcher(interrupt); // Exercise + Verify - expect(throws([&]() { (void)dispatcher[2]; })); + expect( + throws([&]() { static_cast(dispatcher[2]); })); expect(that % dispatcher.size() == 2); }; diff --git a/v6/tests/compile_checks/dispatcher_rejects.cpp b/v6/tests/compile_checks/dispatcher_rejects.cpp index c8d8005..27213ee 100644 --- a/v6/tests/compile_checks/dispatcher_rejects.cpp +++ b/v6/tests/compile_checks/dispatcher_rejects.cpp @@ -47,7 +47,7 @@ void index_temporary_can_dispatcher(fake_can_interrupt& p_interrupt) { // expected-error@+1 {{overload resolution selected deleted operator '[]'}} auto& slot = can_dispatcher(p_interrupt)[0]; - (void)slot; + static_cast(slot); } void can_dispatcher_on_temporary_interrupt() From 5c40eb713bc702832ff3cc4f95f13041fa2c91d5 Mon Sep 17 00:00:00 2001 From: Khalil Estell Date: Thu, 22 Oct 2026 16:47:48 -0700 Subject: [PATCH 4/4] Make the CAN dispatcher slot constructor private Only a dispatcher creates slots, so user code cannot make one. --- v6/modules/can.cppm | 7 ++++--- v6/tests/compile_checks/dispatcher_rejects.cpp | 7 +++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/v6/modules/can.cppm b/v6/modules/can.cppm index 14ebc60..e819529 100644 --- a/v6/modules/can.cppm +++ b/v6/modules/can.cppm @@ -313,13 +313,12 @@ class can_interrupt_dispatcher; * `hal::can_interrupt_dispatcher` * @ingroup CAN_Utilities * - * Get slots from a dispatcher's `operator[]`. Every dispatcher, whatever its - * slot count, uses this one type. + * Only a dispatcher can create slots; get them from its `operator[]`. Every + * dispatcher, whatever its slot count, uses this one type. */ export class can_interrupt_slot final : public can_interrupt { public: - can_interrupt_slot() = default; can_interrupt_slot(can_interrupt_slot const&) = delete; can_interrupt_slot& operator=(can_interrupt_slot const&) = delete; can_interrupt_slot(can_interrupt_slot&&) = delete; @@ -330,6 +329,8 @@ private: template friend class can_interrupt_dispatcher; + can_interrupt_slot() = default; + void dispatch(can_message const& p_message) noexcept { // Read the callback once: a subscriber can be removed between messages. diff --git a/v6/tests/compile_checks/dispatcher_rejects.cpp b/v6/tests/compile_checks/dispatcher_rejects.cpp index 27213ee..dd5440b 100644 --- a/v6/tests/compile_checks/dispatcher_rejects.cpp +++ b/v6/tests/compile_checks/dispatcher_rejects.cpp @@ -62,3 +62,10 @@ void copy_can_dispatcher(fake_can_interrupt& p_interrupt) // expected-error@+1 {{call to deleted constructor}} can_dispatcher copy = shared; } + +// A slot only works inside a dispatcher, so only a dispatcher creates one. +void construct_can_slot() +{ + // expected-error@+1 {{private constructor}} + hal::can_interrupt_slot slot; +}