Feature/mailbox cleanup#2
Draft
mattih11 wants to merge 44 commits into
Draft
Conversation
added 30 commits
February 20, 2026 11:53
- Tests 1,4,6: Verify TiMS collision detection works correctly - Tests 2,3,5,7,8: Verify no collisions for different types/instances - All tests now include consumers to verify data flow - Removed emoji characters from output - Exposed data flow issue (consumers not receiving data) for investigation Test results: 3/8 passing (collision tests work, data flow issue exists)
Design Phase: Document new mailbox architecture before implementation Motivation: - Current system has 4+ mailbox types (BlankMailbox, TypedMailbox, RegistryMailbox, HistoricalMailbox) with unclear separation of concerns - HistoricalMailbox mixes storage (RingBuffer) with communication (Mailbox) - Unclear ownership and lifecycle management Inspiration: Architecture inspired by RACK framework (github.com/smolorz/RACK) by Sebastian Smolorz. RACK's clean separation between data storage (buffers) and communication (mailboxes) provides solid foundation for real-time systems. New Architecture Principles: 1. Single mailbox type: TypedMailbox<MessageTypes...> 2. Mailboxes for communication ONLY 3. Buffers for storage ONLY (separate concern) 4. Clear ownership at module/input/output level Per-Module Components: - ONE shared work_mbx for subscription protocol (all inputs reference it) Per-Output Components: - RingBuffer<Type, SLOTS> for data storage (output-side only) - TypedMailbox<SystemCommands, UserCommands<Type>> cmd_mailbox * Receives: SubscribeRequest, UnsubscribeRequest, user commands * Sends: SubscriptionAck to work_mbx, data to consumers' data_mbx Per-Input Components: - TypedMailbox<Type> data_mbx for receiving data - TypedMailbox<SubscriptionAck>& work_mbx reference (shared module-level) * Input sends: SubscribeRequest to producer's cmd_mailbox * Module receives: SubscriptionAck from producer Migration Strategy: Phase 1: TypedMailbox unification (merge RegistryMailbox) Phase 2: Remove HistoricalMailbox Phase 3: Implement ModuleInput/ModuleOutput fully Phase 4: Update Module framework to use new classes Benefits: - Single mailbox concept to learn - Clear separation: storage vs communication - Type safety via variadic templates - Simpler mental model Files: - docs/work/MAILBOX_CLEANUP_DESIGN.md: Full architecture document - include/commrat/module/io/module_output.hpp: Prototype showing RingBuffer + CommandMailbox - include/commrat/module/io/module_input.hpp: Prototype showing DataMailbox + work_mbx reference Next: Implement Phase 1 (TypedMailbox unification)
Mark cmd_mailbox per-output question as RESOLVED. Rationale: - TiMS addresses are type-dependent: calculate_address(sys_id, inst_id, type_id) - Each output type MUST have unique mailbox address for subscription protocol - Output-specific commands (SetSensorRate, SetFilter, etc.) are type-dependent - Cannot share command mailbox across different output types Result: Each ModuleOutput has its own CommandMailbox<SystemCommands, UserCommands<Type>>
Split ModuleInput into three distinct classes with clear responsibilities: 1. CmdInput (Base class): - Command-only interface to remote module - No data reception, no process() participation - send_command() to producer's command mailbox - Stores producer address calculation 2. ContinuousInput : public CmdInput - Push model: receives continuous data stream - Has data_mbx for receiving published data - subscribe/unsubscribe via work_mbx reference (module-level shared) - poll_data() → process() (blocking receive) - NO local buffer (producer stores in RingBuffer) 3. SyncedInput : public CmdInput - Pull model: get_data(timestamp) queries producer's buffer - RACK-style getData for multi-rate sensor fusion - NO data_mbx (only RPC to producer) - Optional work_mbx (may not need subscription for getData) - Tolerance and interpolation mode configuration - Tracks validity and freshness of last getData Benefits: - Clear separation of communication patterns - No enable_if complexity - Type-safe APIs (can't call poll_data on SyncedInput) - Follows RACK proxy pattern for different data flows - CmdInput allows control-only connections Architecture: - All inputs can send commands (inherited from CmdInput) - ContinuousInput adds subscription + streaming - SyncedInput adds timestamp synchronization + buffered access - Module-level work_mbx shared across all inputs (reference) Files: - include/commrat/module/io/input/cmd_input.hpp - include/commrat/module/io/input/continuous_input.hpp - include/commrat/module/io/input/synced_input.hpp - include/commrat/module/io/module_input.hpp (now exports all three) Next: Update ModuleOutput and implement subscription/getData protocols
Single output type incorporating TimestampedRingBuffer functionality: BufferedOutput<CommratApp, T, SLOTS>: - Command mailbox: Receives Subscribe/Unsubscribe + user commands - TimestampedRingBuffer: Stores recent data with timestamp indexing - publish(data, timestamp): Store in buffer + send to continuous subscribers - getData(timestamp, tolerance, mode): RACK-style synchronized access - No tuple complexity - strongly typed for single type T Moved buffer handling from HistoricalMailbox to BufferedOutput: - Clean separation: storage (output) vs communication (mailboxes) - TimestampedRingBuffer provides optimized timestamp search - Supports both ContinuousInput (publish) and SyncedInput (getData) - Default tolerance configuration per output Removed: - ModuleOutput base class (not needed - only one output type) - StreamingOutput (all outputs are buffered, supports both use cases) Benefits: - Single output concept - always buffered - No inheritance complexity - Direct access to TimestampedRingBuffer features - Follows RACK: buffers in data provider (output side only) Architecture: - Each output type T has own BufferedOutput instance - Address depends on type_id (per-output command mailbox required) - Buffer stores TimsMessage<T> with header.timestamp indexing - Framework uses publish() for data flow, getData() for sync Files: - include/commrat/module/io/output/buffered_output.hpp - include/commrat/module/io/module_output.hpp (exports BufferedOutput) Next: Update design doc, mark HistoricalMailbox for removal
Document completed Phase 2 work: Input Class Hierarchy (DONE): - CmdInput: Base class for command-only interface - ContinuousInput: Push model with data_mbx and subscribe() - SyncedInput: Pull model with getData(timestamp) Output Class (DONE): - BufferedOutput: Single class with TimestampedRingBuffer + cmd_mbx - Moved buffer functionality from HistoricalMailbox - No tuple complexity - strongly typed per output type T Key Achievements: - Clean separation: storage (BufferedOutput) vs communication (mailboxes) - Three input patterns for different communication needs - Work mailbox shared at module level (inputs reference it) - Command mailbox per-output (required by TiMS addressing) - TimestampedRingBuffer supports both publish() and getData() Updated Status: - Phase 1: TypedMailbox unification (PENDING) - Phase 2: I/O classes (IN PROGRESS - classes done, integration pending) - Phase 3: Module framework integration (PENDING) - Phase 4: Cleanup (PENDING) Next Steps: - Remove HistoricalMailbox class - Update existing modules to use new I/O classes - Wire new classes into Module framework - Implement subscription protocol and getData RPC Files marked for removal: - include/commrat/mailbox/historical_mailbox.hpp (replaced by BufferedOutput) - include/commrat/mailbox/registry_mailbox.hpp (merge into TypedMailbox)
RACK-inspired I/O architecture with clean separation of concerns: New I/O Classes (module/io/input/, module/io/output/): - CmdInput: Command-only communication (no data) - ContinuousInput: Push model with blocking receive() - SyncedInput: Pull model with getData(timestamp, tolerance) - BufferedOutput: Storage with TimestampedRingBuffer Configuration (module/io/config/): - input_config.hpp: CmdInputConfig, ContinuousInputConfig, SyncedInputConfig - output_config.hpp: BufferedOutputConfig - All I/O classes have ConfigType aliases Type System (module/io/): - io_traits.hpp: Compile-time type introspection * find_continuous_input_index_v<Tuple> * count_continuous_inputs_v<Tuple> * ValidInputTuple concept (at most 1 ContinuousInput) * consteval validation functions - io_spec.hpp: Clean I/O specifications * Output<T>, Outputs<Ts...>, NoOutput * Input<T>, Inputs<Ts...>, CmdOnlyInput<T> * InputSpecOrVoid concept (allows omitting input spec) Infrastructure (module/io/): - input_infrastructure.hpp: Unified single/multi input handling - output_infrastructure.hpp: Unified single/multi output handling - input_processor.hpp: Input synchronization (renamed from multi_*) - output_manager.hpp: Output management (renamed from multi_*) Three Execution Models: 1. Input-driven: Has ContinuousInput (blocked on receive) 2. Periodic: No ContinuousInput + config.period set 3. Free-running: No ContinuousInput + config.period = std::nullopt Module Config Changes: - period: std::chrono::milliseconds -> std::optional<std::chrono::milliseconds> - Default: 100ms (backward compatible) - std::nullopt: Free-running loop (no sleep) Removed: - historical_mailbox.hpp (functionality moved to BufferedOutput) - multi_* files (renamed to unified input/output_*) - PeriodicInput/LoopInput tags (use InputSpecOrVoid concept + optional period) Key Design Principles: - Compile-time validation (constexpr, consteval, concepts) - Zero runtime overhead (all type dispatch at compile time) - Unified single/multi approach (tuple size 1 vs N) - Type-safe primary input detection (no hardcoded index 0) - RACK separation: Mailboxes for communication, buffers for storage
Simplifies config generation using std::optional instead of complex template selection:
- UniversalModuleConfig: Single struct with all possible I/O fields as optional
- Fields only appear in JSON when set (rfl::json handles std::optional)
- No template metaprogramming needed - Module just sets relevant fields
Benefits:
- Simpler: One config type instead of complex template selection
- Cleaner: Unused fields don't appear in serialized JSON
- Easier: Same type for all Module<OutputSpec, InputSpec> combinations
- Flexible: Module initialization sets which optional fields are present
Example configs:
- Module<Output<T>>: Sets system_id, instance_id
- Module<Output<T>, Input<U>>: Adds input{producer, timeout}
- Module<Outputs<...>>: Sets outputs vector
- Module<Inputs<...>>: Sets primary_input, synced_inputs, sync_tolerance
- Module<CmdOnlyInput<T>>: Sets command_input
Uses reflect-cpp processors (rfl::SnakeCaseToCamelCase) for clean JSON output.
Simplified mailbox configuration by removing custom mailbox_name field. Mailbox names now auto-generated from mailbox_id: 'mailbox_<id>'. Changes: - Removed mailbox_name field from MailboxConfig - Auto-generate name in create_tims_config() - Updated examples and tests to not specify mailbox_name - Minor cleanups in test files
New utility files for type introspection and mailbox categorization: mailbox_type.hpp: - MailboxType enum (CMD, WORK, DATA0, DATA1, etc.) - Used for RACK-style mailbox categorization - Enables compile-time mailbox name generation type_name.hpp: - TypeName<T>: Compile-time type name extraction using rfl - TypeNames<Ts...>: Unified max-size type names - EnumName<E>: Enum-to-string conversion - format_mailbox_name_ct<>: Compile-time mailbox name formatting - MailboxNameBuilder: Efficient runtime name generation with compile-time prefix/suffix test_type_name.cpp: - Tests for type name extraction and formatting - Validates compile-time string manipulation These utilities support the new I/O infrastructure and config generation.
- test_multi_input.cpp: Rewritten to use Module2 with Input<T> + SyncedInput<T> * Tests multi-input compilation with Synced<T> wrapper * Validates metadata accessors and type safety * Supports 1 primary + N secondary inputs (5/5 tests pass) - test_timestamp_logic.cpp: Updated to use modern metadata accessor API * Replaced get_input_metadata<N>() with get_input_timestamp<N>() * Uses has_new_data<N>() and is_input_valid<N>() * Tests automatic timestamp generation and propagation - continuous_input.hpp: Added unified metadata API methods * Added is_valid() method (alias for has_data()) * Added is_fresh() method (always true for continuous input) * Provides consistent API matching SyncedInput All modernized tests compile and validate current Module2 architecture.
Core Module2 Infrastructure: - module2.hpp: New Module2 base class with I/O tuple architecture - module/services/io_handler.hpp: I/O service layer for input/output management - module/services/command_handler.hpp: Command dispatch infrastructure I/O Components: - module/io/synced.hpp: Synced<T> wrapper for multi-input synchronization - module/io/output/module_output.hpp: ModuleOutput with timestamped buffering - module/io/multi_input_infrastructure.hpp: Multi-input support infrastructure Messaging System: - messaging/data_with_commands.hpp: DataWithCommands message type - messaging/registry_utils.hpp: Registry utility functions and type extraction - messaging/system/data_request_messages.hpp: GetData/GetNextData protocol - module/helpers/command_extraction.hpp: Command type extraction helpers Examples: - examples/module2_simple_example.cpp: Module2 usage patterns - examples/command_association_example.hpp: Command association patterns Tests (10 new test files): - test_auto_id_validation.cpp: AUTO_ID message ID validation - test_getdata_registration.cpp: GetData message auto-registration - test_module_output_getdata.cpp: ModuleOutput GetData handler tests - test_registry_utils.cpp: Registry utility function tests - test_registry_utils_simple.cpp: Simple registry utility tests - test_subscription_protocol.cpp: Subscribe/Unsubscribe protocol tests - test_synced_wrapper.cpp: Synced<T> wrapper API tests - test_system_messages.cpp: System message validation - test_workmailbox.cpp: Work mailbox functionality tests - debug_registry.cpp: Registry introspection debugging tool Documentation (8 new design documents): - docs/work/MODULE_REFACTOR_STRATEGY.md: Module2 refactoring roadmap - docs/work/MODULE_REFACTOR_QUICKSTART.md: Quick reference guide - docs/work/MODULE2_PRODUCTION_ROADMAP.md: Production readiness plan - docs/work/ZERO_COPY_ARCHITECTURE.md: Zero-copy design documentation - docs/work/SERVICE_ARCHITECTURE.md: Service layer architecture - docs/work/REGISTRY_CONSOLIDATION.md: Registry cleanup and utilities - docs/work/COMMAND_ASSOCIATION.md: Command association design - docs/work/KNOWN_ISSUES.md: Active issues and workarounds This commit establishes the foundation for Module2 with: - Zero-copy workspace API for inputs/outputs - Compile-time I/O tuple construction - Multi-input synchronization with Synced<T> - Producer-side GetData request handling - Unified metadata accessor API - Command dispatch infrastructure
…ments Message Registry: - Enhanced MessageRegistry with system message auto-inclusion - Added registry utility type extraction (subscription_messages_t, data_messages_t) - Improved compile-time message ID calculation and validation - Added support for AUTO_ID (0) with automatic assignment starting from 1 - MAX_MESSAGE_ID validation (0x7FFF for non-reply messages) Message ID System: - Request/reply protocol with automatic reply ID generation (-request_id) - Sign bit reserved for reply messages (negative IDs) - Compile-time validation for allowed reply categories - Improved message ID formatting and prefix/subprefix handling Message Helpers: - Added utility functions for message type extraction - Improved type introspection for MessageDefinition - Enhanced compile-time type checking and validation System Messages: - Integrated subscription protocol messages into registry - Automatic inclusion of system messages in all registries - Simplified system message registration and access Changes: +361 insertions, -107 deletions across 5 files
Major Changes: - Deleted registry_mailbox.hpp (249 lines) - functionality integrated elsewhere - Simplified typed_mailbox.hpp (net -411 lines of complexity) - Improved mailbox.hpp with cleaner API (restructured 276 lines) - Enhanced timestamped_ring_buffer.hpp with better serialization support TypedMailbox: - Removed redundant abstractions and template bloat - Streamlined type-safe send/receive operations - Better integration with MessageRegistry - Cleaner separation of concerns TimestampedRingBuffer: - Improved serialization support for GetData protocol - Better timestamp-based lookups (NEAREST interpolation) - Enhanced const-correctness and API clarity - Optimized buffer management Mailbox Core: - Cleaner separation between typed and untyped operations - Improved error handling and timeout support - Better TiMS integration with reduced boilerplate Net change: -413 lines (365 insertions, 778 deletions) This is a significant simplification while maintaining functionality.
Major Architectural Changes: - io_spec.hpp: Complete rewrite with I/O tuple specification system (+623 lines) * Output<T>, Input<T>, SyncedInput<T>, Period<Duration> specifications * BuildIOTuple with compile-time I/O construction * IO::Meta struct with comprehensive I/O metadata * Support for multi-input (1 primary + N synced secondary inputs) Input Infrastructure: - cmd_input.hpp: Enhanced command/request handling (+158 lines) - synced_input.hpp: Major improvements to synchronized input (+276 lines) * GetData/GetNextData protocol integration * Synced<T> wrapper support for fresh/stale/invalid states * Automatic timestamp synchronization with primary input - Deleted input_infrastructure.hpp (134 lines) - integrated into io_spec.hpp - Deleted io_traits.hpp (195 lines) - replaced by IO::Meta system Output Infrastructure: - buffered_output.hpp: Enhanced buffering support (+110 lines) - output_infrastructure.hpp: Improved structured output access (+244 lines) * Named field access via rfl::NamedTuple * Zero-copy workspace API integration * Multi-output support improvements Configuration: - input_config.hpp, output_config.hpp: Simplified config structures - module_output.hpp, module_input.hpp: Header includes updated Net change: +578 lines (1182 insertions, 604 deletions) This establishes the foundation for Module2's I/O tuple architecture.
Module Helpers & Mailbox: - module/helpers/address_helpers.hpp: Updated address calculation for Module2 - module/mailbox/mailbox_set.hpp: Mailbox set management updates Module Lifecycle: - module/lifecycle/loop_executor.hpp: Loop executor for Module2 data loop Module Metadata: - module/metadata/input_metadata.hpp: Input metadata structure - module/metadata/input_metadata_accessors.hpp: Metadata accessor functions - module/metadata/input_metadata_manager.hpp: Metadata management layer Module Configuration: - module/module_config.hpp: Module configuration with SimpleOutputConfig support Module Traits: - module/traits/module_types.hpp: Type traits for Module2 - module/traits/processor_bases.hpp: Processor base class helpers - module/traits/type_extraction.hpp: Type extraction utilities These updates provide the foundation for Module2's I/O tuple architecture, enabling compile-time type extraction and metadata access.
Platform Threading: - platform/threading.hpp: Thread, Mutex, Lock abstractions for portability * Ensures consistent threading API across platforms * Required by Module2 for input/output thread management Platform TiMS Integration: - platform/tims_wrapper.hpp: Updated TiMS wrapper interface - src/tims_wrapper.cpp: TiMS wrapper implementation updates * Mailbox creation and management * Message send/receive operations These platform abstractions provide the foundation for Module2's multi-threaded architecture while maintaining portability.
Core Application Header: - commrat.hpp: Updated CommRaT<> template to support Module2 * Includes module2.hpp for new module architecture * Maintains backward compatibility with existing code Message System: - messages.hpp: Core message type definitions * Message::Data, Message::Command, Message::Event * Integration with Module2 I/O specifications Module Main Wrapper: - module_main.hpp: Application entry point helpers * Module lifecycle management * Configuration parsing and validation Registry Module: - registry_module.hpp: Module registry and introspection * Module discovery and type information * Runtime module management These core headers integrate Module2 into the main CommRaT framework while preserving the existing API for backward compatibility.
Module Examples: - clean_interface_example.cpp: Clean API demonstration - command_example.cpp: Command handling patterns - continuous_input_example.cpp: Continuous data subscription - loop_mode_example.cpp: Free-running loop mode - module_main_basic.cpp: Basic module_main usage - module_main_multiformat.cpp: Multi-format message handling - module_main_with_config.cpp: Configuration-driven modules - multi_output_runtime.cpp: Runtime multi-output configuration - multi_output_sensor_fusion.cpp: Multi-output sensor fusion example Message Definitions: - messages/common_messages.hpp: Common message types - messages/messages.hpp: Example message definitions All examples updated to demonstrate Module2 features: - Zero-copy workspace API - I/O tuple specifications - Multi-input/multi-output patterns - Modern metadata accessors
Multi-Input Tests: - test_3input_fusion.cpp: Triple-input synchronization test - test_multi_input_compilation.cpp: Multi-input compilation validation - test_multi_input_module.cpp: Multi-input module runtime tests Multi-Output Tests: - test_multi_output.cpp: Multi-output module validation Module Infrastructure Tests: - test_historical_mailbox.cpp: ModuleOutput timestamped buffering tests - test_process_signature.cpp: Process signature generation tests - test_timestamped_ring_buffer.cpp: RingBuffer serialization tests All tests updated to use Module2 API: - Modern metadata accessors (get_input_timestamp<N>(), etc.) - I/O tuple specifications (Output<T>, Input<T>, SyncedInput<T>) - Synced<T> wrapper for multi-input synchronization - Zero-copy workspace patterns
User-Facing Documentation: - docs/README.md: Updated overview and Module2 introduction - docs/API_REFERENCE.md: Module2 API documentation - docs/USER_GUIDE.md: Module2 user guide and examples - docs/ROADMAP.md: Module2 roadmap and future plans Architecture Documentation: - docs/ARCHITECTURE.md: Module2 architecture overview - docs/DOCUMENTATION_STRATEGY.md: Documentation organization Issue Tracking: - docs/KNOWN_ISSUES.md: Current known issues and workarounds Example Documentation: - docs/examples/01_hello_commrat/hello_commrat.cpp: Updated hello world - docs/examples/02_multi_output/README.md: Multi-output guide - docs/examples/02_multi_output/multi_output_example.cpp: Updated example - docs/examples/03_multi_input_fusion/README.md: Multi-input guide - docs/examples/03_multi_input_fusion/multi_input_fusion.cpp: Updated example Internal Documentation: - docs/internal/README.md: Internal docs index - docs/internal/design_decisions/: Design decision documents - docs/internal/refactoring/: Refactoring strategy docs Archive: - docs/archive/MODULE_MODERNIZATION_STRATEGY.md: Historical context All documentation updated to reflect: - Module2 I/O tuple architecture - Zero-copy workspace API - Multi-input synchronization patterns - Modern metadata accessor API - Current best practices and examples
Build System: - CMakeLists.txt: Added Module2 test targets and examples * test_auto_id_validation, test_getdata_registration, etc. * module2_simple_example and other Module2 examples * Updated include paths and dependencies Project Documentation: - README.md: Updated project overview with Module2 * Module2 feature overview * Quick start guide updates * Architecture highlights Development Guidelines: - .github/copilot-instructions.md: Module2 coding guidelines * Module2 patterns and best practices * Zero-copy API guidelines * Multi-input synchronization patterns * Updated examples and constraints Build system now supports full Module2 test suite and examples.
- Fixed Thread constructor lambda capture to copy config by value - Lambda now captures by value and uses static thread_function_static - Multiple io_instances_ initialization fixes across input/output types - Added 5-second timeout to example_commands test to prevent indefinite runs - Test pass rate improved: 40% -> 50% -> 53% (16/30 tests passing) The Thread lambda bug was causing segfaults when ThreadConfig went out of scope before the thread started, as the lambda captured config by reference. Now uses [cfg = config, f = std::forward<Func>(func)] for safe captures.
- ContinuousInput: owns TypedMailbox<Registry, T> instead of generic pointer - SyncedInput: no data mailbox (RPC only), has last_message_ storage - CmdInput: uses WorkMailbox* reference instead of generic MailboxFor* - IOHandler: type-aware initialize_input(), start_inputs()/start_outputs() - Module2::start(): proper lifecycle - create then activate mailboxes - Fix PUBLISH mailbox index: WORK_MBX_BASE(1) -> PUBLISH_MBX_BASE(2) - Fix DATA mailbox index: dynamic calc -> DATA_MBX_BASE(3) constant - Fix WORK mailbox type_id: void(0) -> primary output type (prevents different module types at same sys/inst from colliding) - Fix IOHandler::initialize_input for MultiOutputConfig consumers - Fix data_loop: skip process() when primary input times out (fixes zero timestamp assertion in test_timestamp_logic) - Fix test_3input_fusion: guard Synced<T>::operator-> with is_valid() check Remaining known limitations: - test_address_collisions Test 8: TiMS resource limit with 10 concurrent modules - example_commands: timeout expected (command protocol not yet implemented)
Build fixes: - message_id.hpp: add uint8_t passthrough in subprefix lambda (AutoAssignIDsProcess was silently reclassifying Commands as Data when re-instantiating MessageDefinition) - message_registry.hpp: preserve ReplyPayload through auto-ID assignment via new GetReplyPayload<> helper; Command messages no longer lose their reply type - test_system_messages.cpp: instantiate GetDataRequest<T,ID>/GetNextDataRequest<T,ID> with concrete type; fix test 6 prefix assertions (UserDefined not System) - Result: 94% pass rate (31/33); same 2 known-unfixable failures remain Dead code removal: - registry_module.hpp, module_core/mixins/services.hpp, services/publishing.hpp - module/traits/multi_input_resolver.hpp, module_types.hpp - module/io: module_input/output.hpp, multi_input_infrastructure.hpp, input_processor.hpp, output_manager.hpp, output/buffered_output.hpp - module/io_spec.hpp (old path; canonical is module/io/io_spec.hpp) - module/lifecycle/, module/metadata/, module/mailbox/ (entire directories) - test/test_io_spec.cpp, test/test_primary_input.cpp (tested removed old API) Docs cleanup: - Delete: CLEANUP_source_primary_output_type_id.md, work/KNOWN_ISSUES.md - Archive: MAILBOX_CLEANUP_DESIGN.md, INTROSPECTION_INTEGRATION_PLAN.md, REGISTRY_MODULE_REFACTORING.md, MODULE_BASE_REFACTORING_PROPOSAL.md, ADDRESSING_IMPLEMENTATION_PLAN.md - Rewrite DOCUMENTATION_TODO.md (removed 175 lines of duplication) - Update KNOWN_ISSUES.md: fix stale refs, add AutoAssignIDs bug resolution
Root causes fixed (5 issues across 7 framework files): 1. command_handler.hpp: GetDataRequestPayload<T> not recognized as system command - added template trait detection alongside tuple membership check 2. command_handler.hpp: Type mismatch guard for multi-data registries - GetData handler now checks request DataType matches output Type 3. synced_input.hpp: Wrong TimsHeader field names (src_address/dest_address -> src/dest) and incorrect designated initializer order 4. io_handler.hpp: fetch_inputs never called get_data() on synced inputs - added fetch_synced_if_needed<InputIndex>(primary_ts) dispatch 5. mailbox.hpp: Mailbox::receive<T>() did not populate header.src/dest from TiMS transport metadata, causing send_command() source filter to reject every reply (header.src was always 0) Additional fixes: - commrat.hpp: Changed WorkMailbox from restricted TypedMailbox (Subscribe/ Unsubscribe only) to unrestricted MailboxFor<Registry> to allow GetData payloads through the WORK mailbox - module2.hpp: Use Registry::max_message_size instead of WorkMailbox member - synced.hpp: Added using value_type = T for type extraction in templates - Example: Set sync_tolerance=150ms (>= GPS 100ms period) and improved GPS stale/invalid messaging (not an error, expected for multi-rate fusion) Test results: 31/33 pass (94%, unchanged from before)
Dead code removal: - messages.hpp: Remove 6 legacy payloads, type_hash, serialize/deserialize wrappers - system_registry.hpp: Remove deprecated Registry<>, CombinedRegistry<> - timestamp.hpp: Remove ~45 lines of commented-out operator overloads - address_helpers.hpp: Remove deprecated createWorkMailboxConfig() - mailbox.hpp: Remove unused includes (<expected>, <queue>), legacy aliases - continuous_input.hpp: Remove deprecated poll_data() overload Real-time safety fixes: - tims_wrapper.hpp: std::string -> sertial::fixed_string<32> for mailbox_name - mailbox.hpp: Replace std::to_string with stack-allocated digit loop - mailbox.hpp, tims_wrapper.cpp: Remove std::cerr from send hot paths Bug fixes: - continuous_input.hpp: Fix get_sequence_number (header.seq_number) - continuous_input.hpp: Fix get_message_type (header.msg_type) - message_registry.hpp: serialize_message -> serialize - tims_wrapper.hpp: serialize_message/deserialize_message -> serialize/deserialize Deduplication: - registry_utils.hpp: Remove custom TupleSize, replace with std::tuple_size_v - typed_mailbox.hpp: Replace 30-line HasRegistryMember with MailboxFor<Registry> Documentation trimming (~750 lines removed): - commrat.hpp, io_spec.hpp, module2.hpp, timestamped_ring_buffer.hpp - typed_mailbox.hpp, type_name.hpp: Consolidate repeated TODO comments 17 files changed, 107 insertions(+), 861 deletions(-)
…3 obsolete docs - Fix 100+ wrong API references across all docs: Module<> -> Module2<>, PeriodicInput -> Period<>, LoopInput -> loop mode, Outputs<> -> multiple Output<>, Inputs<> -> Input<>+SyncedInput<>, PrimaryInput<> -> auto-primary, process_continuous() -> process(), ModuleConfig -> SimpleOutputConfig/MultiOutputConfig, PUBLISH mailbox -> 3-mailbox (CMD/WORK/DATA) - Rewrite USER_GUIDE.md: 2623 -> 1009 lines (61% reduction) - Rewrite GETTING_STARTED.md with correct Module2 API - Rewrite docs/README.md: 500 -> 130 lines - Rewrite all example READMEs (docs/examples/01-03, examples/, messages/) - Fix API_REFERENCE.md with correct types throughout - Fix ARCHITECTURE.md (verified correct, minor tweaks) - Fix KNOWN_ISSUES.md (merge duplicates, update status) - Fix ROADMAP.md (mark completed items) - Fix copilot-instructions.md (std::optional->Synced<T>, archived doc refs) - Fix docs/internal/README.md (update phase descriptions) - Archive 13 obsolete docs to docs/archive/
…moval, 33/33 tests - Wire user command reply sending in visit_user_command() via CMD mailbox (command_handler.hpp + module2.hpp: pass output ref, call send_reply) - Remove dead legacy subscription.hpp (353 lines, RT violations, unused) - Fix example_commands timeout: auto-stop after 2s instead of Ctrl+C wait - Fix test_address_collisions: reduce stress test from 10 to 9 instances (stay under TiMS 32-connection limit), trim sleeps, add 45s CTest timeout - Harden run_module_main_example.sh: shorter timeout, --kill-after fallback - Update KNOWN_ISSUES.md: mark Issues 2+3 as RESOLVED - Update ROADMAP.md: subscription protocol moved to Completed Features - Update copilot-instructions.md: current status and completed items All 33 tests pass (100% pass rate)
added 9 commits
April 13, 2026 14:07
New files: - platform/duration.hpp: Custom Duration class (structural type, NTTP-compatible) - int64_t ns_ storage, constexpr everything, C++20 spaceship operator - Free function constructors: Milliseconds(), Seconds(), Microseconds(), etc. - Chrono/timespec interop, UDL support (_ns, _ms, _s) - platform/platform.hpp: Compile-time backend selection (STD/EVL) - platform/std/threading_impl.hpp: Thread, Mutex, SharedMutex, ConditionVariable - platform/std/timestamp_impl.hpp: Time::now(), sleep(), sleep_until(), yield() - platform/evl/: Skeleton backends for libevl/Xenomai 4 - docs/work/EVL_API_REFERENCE.md: libevl function reference Refactored: - threading.hpp: Thin wrapper with common types + backend dispatch - timestamp.hpp: Thin wrapper with Timestamp typedef + backend dispatch Migrated all bypass sites: - Core library: mailbox.hpp, typed_mailbox.hpp, timestamped_ring_buffer.hpp, module_config.hpp, module2.hpp, io_spec.hpp, module_output.hpp, io_handler.hpp, continuous_input.hpp, synced_input.hpp, cmd_input.hpp, tims_wrapper.hpp/.cpp - Tests: all std::this_thread::sleep_for -> Time::sleep(), Milliseconds type -> Duration - Examples: all sleep calls migrated, stale #include <thread> removed - Config structs retain std::chrono::milliseconds for rfl serialization 33/33 tests pass
- API_REFERENCE.md: Version 2.1.0, new Duration section, updated Time/Threading sections with Duration-based API, CvStatus enum, Platform Selection section - ARCHITECTURE.md: Platform Abstraction Layer section with backend table, Duration type overview, file structure; updated Threading Model - USER_GUIDE.md: Migrated std::this_thread::sleep_for to Time::sleep(), added platform abstraction note to Threading Model section - ROADMAP.md: Marked Platform Abstraction as COMPLETED, added to completed list - README.md (docs): Added Platform Abstraction to Current Features, updated date
- Replace hard-coded RACK paths with find_package(RACK REQUIRED) - Use RACK::rack imported target (includes propagate transitively) - Use find_package(Threads REQUIRED) + Threads::Threads instead of raw pthread - Use add_compile_options() instead of string-appending on CMAKE_CXX_FLAGS - Split examples and tests into examples/CMakeLists.txt and test/CMakeLists.txt - Add COMMRAT_BUILD_EXAMPLES and COMMRAT_BUILD_TESTS options (default ON) - Remove run_tests custom target (superseded by ctest) - Remove dead commented-out code and stale phase milestone comments
- typed_mailbox.hpp: initialize missing TimsHeader fields (dest, src, flags) in both send() overloads - tims_helpers.hpp: initialize missing TimsHeader fields (dest, src, flags) - module_output.hpp: initialize missing TimsHeader fields (msg_size, dest, src, flags) - continuous_input.hpp: initialize missing TimsHeader fields (msg_size, dest, flags) in UnsubscribeRequest - synced_input.hpp: initialize missing TimsHeader fields (msg_size, flags) in GetDataRequest and GetNextDataRequest - module2.hpp: remove unused 'result' variable from receive_any_for() - test_module_output_getdata.cpp: fix narrowing int -> uint32_t conversion - test_multi_input_compilation.cpp: suppress unused parameter warnings with (void) - test_process_signature.cpp: suppress unused local typedef warnings with sizeof() - test_type_name.cpp: use anonymous parameters in main() signature - test_auto_id_validation.cpp: suppress unused typedef warning with sizeof()
- COMMRAT_PLATFORM cache variable (STD/EVL) with set_property validation - EVL: auto-discovers libevl + evl/thread.h, creates EVL::evl imported target - .github/workflows/ci.yml: 3-job pipeline - build-std: build + ctest in ratos-dev-image container - build-evl-compile: EVL compile-check (continue-on-error, #error stubs pending) - test-evl-runtime: QEMU boot from RaTOS wic image + SSH rsync + ctest - scripts/ci/run-evl-tests.sh: rsync + cmake + ctest helper
- COMMRAT_PLATFORM cmake variable documented throughout (was raw compile defines) - ARCHITECTURE.md: platform table shows STD/EVL option with cmake examples + EVL stub status - API_REFERENCE.md: Platform Selection section shows cmake variable, not raw defines - GETTING_STARTED.md: build section shows optional -DCOMMRAT_PLATFORM=EVL - README.md: quick start shows both STD and EVL build commands - ROADMAP.md: CI pipeline added as completed, EVL backend status clarified - docs/README.md: platform abstraction line updated to cmake variable syntax - copilot-instructions.md: status, platform section, directory structure, completed features all updated
- .github/skills/doc-checkup/SKILL.md: pre-commit documentation review skill Covers all doc files, per-category change detection, checklist for platform option naming, emoji policy, dependency install method, EVL stub status - docs/ROADMAP.md: update Last Updated date to April 22, 2026
- docs/work/EVL_API_REFERENCE.md: replace outdated pkg_check_modules cmake snippet with current COMMRAT_PLATFORM=EVL cmake variable usage - .github/skills/doc-checkup/SKILL.md: add docs/work/ entries to file table
- .github/skills/pre-commit-test/SKILL.md: category-based test selection Maps changed files to test categories (build+ctest, cmake-reconfigure, evl-compile, yaml-lint, shell-syntax, doxygen) so only relevant tests run - Notes tims_router_tcp prerequisite for all ctest runs - Includes pgrep check, background start pattern, and pass criteria per category
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors CommRaT’s mailbox and messaging infrastructure toward the new Module2 + service-oriented design, while updating examples/docs to the new I/O spec API and adding request/reply + get_data protocol message support.
Changes:
- Added new input/config helpers (CmdInput, ContinuousInput, config generator structs) and mailbox typing utilities.
- Introduced request/reply pairing in
MessageDefinition, auto-expansion of reply + get_data messages inMessageRegistry, and new system/user protocol message templates. - Updated examples/docs/CI to reflect Module2 API, new timing abstractions, and revised messaging conventions; removed legacy mailbox wrappers.
Reviewed changes
Copilot reviewed 83 out of 162 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| include/commrat/module/io/input/continuous_input.hpp | Adds ContinuousInput push-stream input with subscribe/unsubscribe and polling. |
| include/commrat/module/io/input/cmd_input.hpp | Adds CmdInput RPC helper for request/reply commands via work mailbox. |
| include/commrat/module/io/config/output_config.hpp | Introduces output buffering configuration struct. |
| include/commrat/module/io/config/module_config_generator.hpp | Adds reflect-cpp based universal module config generator. |
| include/commrat/module/io/config/input_config.hpp | Adds config structs for command/continuous/synced inputs. |
| include/commrat/module/helpers/tims_helpers.hpp | Initializes new TiMS header fields (dest/src). |
| include/commrat/module/helpers/command_extraction.hpp | Adds tuple-membership + user-command extraction utilities. |
| include/commrat/module/helpers/address_helpers.hpp | Adds mailbox index constants + address helpers related tweaks. |
| include/commrat/messaging/system/system_registry.hpp | Adjusts framework system registry composition and includes get_data messages. |
| include/commrat/messaging/system/subscription_messages.hpp | Switches subscription messages to request/reply pairing model and payload fields. |
| include/commrat/messaging/system/data_request_messages.hpp | Adds generic get_data / get_next_data request/reply message templates. |
| include/commrat/messaging/message_registry.hpp | Adds reply expansion + get_data protocol expansion and new registry plumbing. |
| include/commrat/messaging/message_id.hpp | Extends subprefixes, adds request/reply pairing, and changes AUTO_ID marker semantics. |
| include/commrat/messaging/message_helpers.hpp | Updates Message helper aliases; adds DataWith wrapper; adds GetData protocol support. |
| include/commrat/messaging/data_with_commands.hpp | Adds DataWithCommands definitions and extraction/flattening helpers. |
| include/commrat/messages.hpp | Extends TiMS header (dest/src) and removes legacy example payloads/helpers. |
| include/commrat/mailbox/registry_mailbox.hpp | Removes legacy RegistryMailbox wrapper. |
| include/commrat/mailbox/mailbox_type.hpp | Adds MailboxType enum describing mailbox roles/offsets. |
| include/commrat/mailbox/historical_mailbox.hpp | Removes legacy HistoricalMailbox wrapper. |
| examples/multi_output_sensor_fusion.cpp | Migrates example to Module2 and commrat::Time sleep. |
| examples/multi_output_runtime.cpp | Migrates example to Module2 and commrat::Time sleep. |
| examples/module_main_with_config.cpp | Migrates module_main example to Module2 macro signature. |
| examples/module_main_multiformat.cpp | Migrates multi-input example to Input + SyncedInput + Synced API. |
| examples/module_main_basic.cpp | Migrates basic module_main example to Module2 + Period<>. |
| examples/module2_simple_example.cpp | Adds new Module2 usage showcase. |
| examples/messages/messages.hpp | Updates re-exports to new I/O spec API. |
| examples/messages/common_messages.hpp | Updates re-exports to new I/O spec API. |
| examples/messages/README.md | Updates docs to Module2 API and new I/O spec table. |
| examples/loop_mode_example.cpp | Migrates loop mode example to Module2. |
| examples/continuous_input_example.cpp | Migrates continuous input example to Module2 + commrat::Time sleep. |
| examples/command_example.cpp | Reworks command example around DataWithCommands and Module2. |
| examples/command_association_example.hpp | Adds command association usage documentation header. |
| examples/clean_interface_example.cpp | Migrates clean interface example to Module2 + Period<>. |
| examples/README.md | Updates examples index to new API terminology and patterns. |
| examples/CMakeLists.txt | Adds/updates example build targets. |
| docs/work/SERVICE_ARCHITECTURE.md | Documents Module2 service decomposition (IOHandler/CommandHandler/etc.). |
| docs/work/PHASE_7.0_MODULE_MAIN_WRAPPER.md | Updates getData naming references to get_data. |
| docs/work/COMMAND_ASSOCIATION.md | Documents DataWithCommands design and integration. |
| docs/internal/refactoring/ARCHITECTURE_REFACTOR_PLAN.md | Updates getData naming references. |
| docs/internal/design_decisions/SERTIAL_RINGBUFFER_REQUEST.md | Updates getData naming references. |
| docs/internal/design_decisions/RACK_ANALYSIS.md | Updates getData naming references throughout. |
| docs/internal/README.md | Updates phase summaries to current Module2 + synced input model. |
| docs/examples/README.md | Updates examples index structure and topics. |
| docs/examples/02_multi_output/multi_output_example.cpp | Migrates docs example to Module2 and commrat::Time sleep. |
| docs/examples/01_hello_commrat/hello_commrat.cpp | Migrates hello example to Module2 and commrat::Time sleep. |
| docs/examples/01_hello_commrat/README.md | Updates hello example docs to Module2 API and new patterns. |
| docs/archive/TIMESTAMP_ACCESSOR_STRATEGY.md | Updates getData naming references. |
| docs/archive/SUBSCRIPTION_ADDRESSING_STRATEGY.md | Updates getData naming references. |
| docs/archive/MODULE_REFACTOR_QUICKSTART.md | Adds/updates refactor quickstart archive doc. |
| docs/archive/MODULE_MODERNIZATION_STRATEGY.md | Updates getData naming references. |
| docs/archive/MAILBOX_CLEANUP_DESIGN.md | Adds mailbox cleanup design archive doc. |
| docs/archive/IO_SYNC_STRATEGY.md | Updates getData naming references. |
| docs/archive/DOCUMENTATION_TODO.md | Adds documentation todo archive doc. |
| docs/archive/DOCUMENTATION_STRATEGY.md | Updates getData naming references. |
| docs/archive/ADDRESSING_IMPLEMENTATION_PLAN.md | Updates getData naming references. |
| docs/ROADMAP.md | Updates roadmap for Module2-era status. |
| docs/KNOWN_ISSUES.md | Updates known issues to reflect new architecture/status. |
| docs/DOCUMENTATION_TODO.md | Rewrites documentation TODO for Module2 migration. |
| docs/CLEANUP_source_primary_output_type_id.md | Removes obsolete cleanup document. |
| README.md | Updates top-level README to Module2 API and platform builds. |
| .github/workflows/ci.yml | Adds CI workflow for STD + EVL compile + EVL-kernel runtime tests. |
| .github/skills/pre-commit-test/SKILL.md | Adds pre-commit testing skill doc. |
| .github/skills/doc-checkup/SKILL.md | Adds documentation checkup skill doc. |
Comment on lines
+62
to
+63
| template<typename MessageDef> | ||
| inline constexpr bool has_user_commands_v = std::tuple_size_v<ExtractUserCommands_t<MessageDef>> > 0; |
There was a problem hiding this comment.
has_user_commands_v has a syntax error due to an extra > which will prevent compilation. Change the expression to compare the tuple size against 0 without the extra closing angle bracket.
Comment on lines
+28
to
+31
| constexpr uint8_t CMD_MBX_BASE = 0; // CMD mailboxes have index 0 | ||
| constexpr uint8_t WORK_MBX_BASE = 1; // WORK mailboxes have index 1 | ||
| constexpr uint8_t PUBLISH_MBX_BASE = 2; // PUBLISH mailboxes have index 2 | ||
| constexpr uint8_t DATA_MBX_BASE = 3; // DATA mailboxes start at index 3 |
There was a problem hiding this comment.
The mailbox index constants (WORK_MBX_BASE = 1, etc.) conflict with MailboxType and extract_mailbox_type(), which treat indices as 0/16/32/48 offsets. This mismatch will cause incorrect address encoding/decoding and routing. Align the constants and mapping to the same index scheme (either use 0/16/32/48 everywhere, or update MailboxType/mapping to match 0/1/2/3).
Suggested change
| constexpr uint8_t CMD_MBX_BASE = 0; // CMD mailboxes have index 0 | |
| constexpr uint8_t WORK_MBX_BASE = 1; // WORK mailboxes have index 1 | |
| constexpr uint8_t PUBLISH_MBX_BASE = 2; // PUBLISH mailboxes have index 2 | |
| constexpr uint8_t DATA_MBX_BASE = 3; // DATA mailboxes start at index 3 | |
| constexpr uint8_t CMD_MBX_BASE = 0; // CMD mailbox offset | |
| constexpr uint8_t WORK_MBX_BASE = 16; // WORK mailbox offset | |
| constexpr uint8_t PUBLISH_MBX_BASE = 32; // PUBLISH mailbox offset | |
| constexpr uint8_t DATA_MBX_BASE = 48; // DATA mailboxes start at offset 48 |
Comment on lines
+68
to
+75
| constexpr MailboxType extract_mailbox_type(uint32_t addr) { | ||
| uint8_t index = extract_mailbox_index(addr); | ||
| // Map index back to enum (CMD=0, WORK=16, PUBLISH=32, DATA=48+) | ||
| if (index == 0) return MailboxType::CMD; | ||
| if (index == 16) return MailboxType::WORK; | ||
| if (index == 32) return MailboxType::PUBLISH; | ||
| return MailboxType::DATA; // Any other index is DATA | ||
| } |
There was a problem hiding this comment.
The mailbox index constants (WORK_MBX_BASE = 1, etc.) conflict with MailboxType and extract_mailbox_type(), which treat indices as 0/16/32/48 offsets. This mismatch will cause incorrect address encoding/decoding and routing. Align the constants and mapping to the same index scheme (either use 0/16/32/48 everywhere, or update MailboxType/mapping to match 0/1/2/3).
Comment on lines
+128
to
+135
| MailboxFor<Registry>& data_mbx, | ||
| uint8_t producer_system_id, | ||
| uint8_t producer_instance_id, | ||
| Duration requested_period = Duration::zero(), | ||
| Duration poll_timeout = Milliseconds(100), | ||
| Duration cmd_timeout = Milliseconds(1000)) | ||
| : CmdInput<Registry, OutputType>(work_mbx, producer_system_id, producer_instance_id, cmd_timeout) | ||
| , data_mbx_(&data_mbx) |
There was a problem hiding this comment.
data_mbx_ is declared as std::optional<DataMailbox>, but this constructor initializes it with &data_mbx (a pointer), which is not compatible and is very likely a compile error (or constructs DataMailbox from an unintended pointer conversion if such a constructor exists). If you need a non-owning mailbox mode, consider changing the member to something like std::optional<std::reference_wrapper<DataMailbox>> or introducing a separate DataMailbox*/variant to represent owned vs non-owned mailbox storage.
Suggested change
| MailboxFor<Registry>& data_mbx, | |
| uint8_t producer_system_id, | |
| uint8_t producer_instance_id, | |
| Duration requested_period = Duration::zero(), | |
| Duration poll_timeout = Milliseconds(100), | |
| Duration cmd_timeout = Milliseconds(1000)) | |
| : CmdInput<Registry, OutputType>(work_mbx, producer_system_id, producer_instance_id, cmd_timeout) | |
| , data_mbx_(&data_mbx) | |
| DataMailbox& data_mbx, | |
| uint8_t producer_system_id, | |
| uint8_t producer_instance_id, | |
| Duration requested_period = Duration::zero(), | |
| Duration poll_timeout = Milliseconds(100), | |
| Duration cmd_timeout = Milliseconds(1000)) | |
| : CmdInput<Registry, OutputType>(work_mbx, producer_system_id, producer_instance_id, cmd_timeout) | |
| , data_mbx_(data_mbx) |
|
|
||
| // Re-export base members | ||
| using Payload = typename Base::Payload; | ||
| static constexpr uint32_t message_id = Base::message_id; |
There was a problem hiding this comment.
MessageDefinition (as shown in the diff) does not define a message_id member, so Base::message_id will not compile. Either remove this re-export from DataMessageDef, or add a well-defined static constexpr uint32_t message_id to MessageDefinition (if that’s intended to be a compile-time ID independent of registry assignment).
Suggested change
| static constexpr uint32_t message_id = Base::message_id; |
Comment on lines
+55
to
+58
| template<typename... MessageDefs> | ||
| struct ExtractMailboxType<MessageRegistry<MessageDefs...>, std::enable_if_t<!HasRegistryMember<MessageRegistry<MessageDefs...>>::value>> { | ||
| using type = Mailbox<MessageDefs...>; | ||
| }; |
There was a problem hiding this comment.
MailboxFor<Registry> and System::WorkMailbox are currently based on the unexpanded MessageDefs..., but MessageRegistry now expands message defs to include replies and GetData/GetNextData messages (ProcessedDefs). That means the derived mailbox types may not be able to send/receive expanded message types (e.g., reply payloads) even though the registry claims they are registered. Consider basing ExtractMailboxType< MessageRegistry<...> > and System::WorkMailbox on the processed/expanded list (e.g., ProcessedDefs / MessageDefsTuple) so the mailbox’s allowed types match what the registry produces.
Comment on lines
+735
to
+738
| struct System { | ||
| // Generic WorkMailbox - unrestricted (sends/receives any registered type) | ||
| using WorkMailbox = Mailbox<MessageDefs...>; | ||
| }; |
There was a problem hiding this comment.
MailboxFor<Registry> and System::WorkMailbox are currently based on the unexpanded MessageDefs..., but MessageRegistry now expands message defs to include replies and GetData/GetNextData messages (ProcessedDefs). That means the derived mailbox types may not be able to send/receive expanded message types (e.g., reply payloads) even though the registry claims they are registered. Consider basing ExtractMailboxType< MessageRegistry<...> > and System::WorkMailbox on the processed/expanded list (e.g., ProcessedDefs / MessageDefsTuple) so the mailbox’s allowed types match what the registry produces.
| @@ -163,6 +359,9 @@ class MessageRegistry { | |||
| static constexpr bool collisions_checked = CheckCollisions<MessageDefs...>::check(); | |||
There was a problem hiding this comment.
Collision checking is performed only on the original MessageDefs..., but the registry now auto-generates additional message definitions (replies + GetData/GetNextData). This can miss collisions introduced by expansion. Move collision checking to operate on the final expanded tuple (ProcessedDefs / MessageDefsTuple) to ensure the actual registered message set is validated.
Comment on lines
+11
to
+13
| #include "commrat/module2.hpp" | ||
| #include "commrat/messaging/message_registry.hpp" | ||
| #include <iostream> |
There was a problem hiding this comment.
The comment on the period is inconsistent: Milliseconds(100) is 100ms (10Hz), not “100Hz = 10ms”. Also, this example uses types like std::array, std::tuple, and std::optional later in the file but only includes <iostream> explicitly; consider adding the missing standard headers in this example to avoid relying on transitive includes.
Comment on lines
+50
to
+53
| class PeriodicSensor : public ExampleApp::Module2< | ||
| Output<SensorPayload>, | ||
| Period<Milliseconds(100)> // 100Hz = 10ms period | ||
| > { |
There was a problem hiding this comment.
The comment on the period is inconsistent: Milliseconds(100) is 100ms (10Hz), not “100Hz = 10ms”. Also, this example uses types like std::array, std::tuple, and std::optional later in the file but only includes <iostream> explicitly; consider adding the missing standard headers in this example to avoid relying on transitive includes.
added 5 commits
April 22, 2026 00:42
GitHub Actions rejects empty strings in container.credentials at template validation time. vars.* evaluate to '' when not set, causing the error: 'Unexpected value (empty string)' secrets.* are not validated for emptiness at parse time, so moving RATOS_IMAGE_REF and RATOS_REGISTRY_USER from vars to secrets fixes the template validation error while keeping the same runtime behaviour.
GitHub Actions context availability rules: - container.image: vars only (secrets context not available here) - container.credentials.username/password: secrets allowed RATOS_IMAGE_REF -> vars (container.image requirement) RATOS_REGISTRY_USER -> secrets (credentials.username supports it) RATOS_REGISTRY_TOKEN -> secrets (credentials.password, unchanged) The original 'Unexpected value empty string' error was because vars.RATOS_IMAGE_REF was not yet set in the repo, not a code bug.
- CMakePresets.json: default/debug/evl presets (cmake --preset <name>) Used by CI, VS Code cmake-tools, CLion, and scripts — single config source. Requires cmake 3.21+; bump cmake_minimum_required accordingly. - .commrat.env: non-secret defaults (RATOS_IMAGE_REF, RATOS_RELEASE_REPO, QEMU_MEMORY/CPUS/PORT). Sourced by scripts; .commrat.env.local is gitignored for machine-specific overrides. - .devcontainer/devcontainer.json: VS Code Dev Container using ratos-dev-image. Opens with full IntelliSense and cmake-tools pre-configured. - CMakeLists.txt: add commrat-shell / commrat-test-container / commrat-evl-compile custom targets (docker/podman). Auto-skipped inside a container environment. - ci.yml: hardcode public image (no credentials or GitHub variables needed), switch to cmake presets, use dawidd6/action-download-artifact to fetch ratos-evl-artifacts (works on every main push, not just release tags). Only RATOS_RELEASE_TOKEN secret required. - scripts/ci/run-evl-tests.sh: SSH_KEY/SSH_PORT now env-configurable (no hardcoded ci_key path or symlink hack). - scripts/run-local-evl-tests.sh: sources .commrat.env, defaults to gh run download from latest successful main build (fixes the 'no release assets' error when using a non-vX.Y.Z tag). Supports --run-id / --tag flags. - docs: update build commands to cmake --preset throughout; update CI Pipeline entry in ROADMAP.md; update directory structure and completed features in copilot-instructions.md.
scripts/run-in-container.sh: build with any CMake preset and/or open an
interactive shell inside the RaTOS dev container. Bind-mounts the source
tree so build outputs (build/<preset>/) persist on the host.
scripts/run-in-container.sh # interactive shell
scripts/run-in-container.sh --build evl # EVL build then shell
scripts/run-in-container.sh --build evl --no-shell
scripts/run-in-container.sh -- ./build/evl/examples/example_commands
CMakeLists.txt:
- Add commrat-evl-dev target (build EVL preset then open shell)
- Add --network host to all container targets so tims_router_tcp on the
host is reachable from the container
- Fix find_program to use NAMES keyword (without it, cmake treats
subsequent args as path hints, not names; podman was silently missed)
Phase 4 migration: CommRaT is now a header-only (INTERFACE) library that depends on CoreRaT for platform abstraction, wire types, IPC mailbox, and TiMS backend. Deleted files (replaced by CoreRaT equivalents): - src/tims_wrapper.cpp - include/commrat/platform/ (entire directory) - include/commrat/messages.hpp - include/commrat/messaging/message_id.hpp - include/commrat/mailbox/mailbox.hpp - scripts/ci/run-evl-tests.sh - scripts/run-in-container.sh - scripts/run-local-evl-tests.sh Updated files: - CMakeLists.txt: INTERFACE library, find_package(CoreRaT), no RACK/EVL - CMakePresets.json: removed COMMRAT_PLATFORM cache variables - messaging/registry_utils.hpp: 590->128 lines, delegates to corerat::registry - messaging/message_registry.hpp: 834->758 lines, reuses corerat ExpandReplies/ GetReplyPayload/CheckCollisions - All internal headers: include corerat/ directly, using declarations in namespace commrat blocks CI: start corerat-router-tcp before ctest (replaces tims_router_tcp) Docs: update all router references to corerat-router-tcp All 33 tests pass with corerat-router-tcp.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a large PR. We didnt split it correctly, so it will need some documentation and naming reworks: @copilot!
Also have a look at the labels, milestones and other repository recommendations you would make for github structure (linking issues that are currently only tracked in the md, etc)! @copilot