Populate message info for intra-process messages - #3271
Open
thomasmoore-torc wants to merge 3 commits into
Open
thomasmoore-torc wants to merge 3 commits into
thomasmoore-torc wants to merge 3 commits into
Conversation
(cherry picked from commit 8f255f3da12d5d0cc8aa5a49f69bb74339226820) (cherry picked from commit 94a3d5856c046e3a7b6bda942394cd8662926c28) Signed-off-by: Thomas Moore <thomas.moore@torc.ai>
(cherry picked from commit f2302935bd1b41a75f56c6a9f4ef5b7fbe4d7e9b) (cherry picked from commit 8a875ee1df60afcdb7ed68ceb27f097acdcdacf0) Signed-off-by: Thomas Moore <thomas.moore@torc.ai>
Signed-off-by: Thomas Moore <thomas.moore@torc.ai>
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.
rclcpp: Populate message info for intra-process messages
Branch:
intra-process-message-info(this repo:ros2/rclcpp)Related PR: #3272 — "Iterate over IntraProcessBuffer instead of creating a vector" (
intra-process-reduce-copies). See Landing order below — no functional dependency, but the two touch overlapping code.Summary
Subscriptions that receive a message via intra-process delivery currently get a default-constructed, empty
rmw_message_info_t— no sequence number, no publisher GID, nofrom_intra_processflag. The same subscription receiving the same message via inter-process (rmw) delivery gets a fully populated one. Anything relying onmessage_info(deduplicating by publisher GID, checkingfrom_intra_process, etc.) behaves inconsistently depending on whether a given publish happens to stay in-process or not.This populates
message_infofor intra-process messages too, so subscribers see consistent, meaningful info regardless of delivery path.Changes
Populate message info for intra-process messages— threads anrmw_message_info_talongside the message through the intra-process publish path (Publisher::do_intra_process_publish/do_intra_process_ros_message_publish_and_return_shared→IntraProcessManager→IntraProcessBuffer→ subscription).IntraProcessBufferstorage changes from a bareshared_ptr/unique_ptrto a bundledIntraProcessBufferData(message +rmw_message_info_t), added with a copy constructor that deep-copies the unique-ptr alternative (needed soget_all_data()-based paths, e.g. transient-local replay, keep working with move-only storage).do_transient_local_publishto correctly convert between shared/unique storage and shared/unique subscription requests viastd::visitinstead of assuming the two always match.Move IntraProcessBufferData to its own header— pulls the new struct out intointra_process_buffer_data.hppto keepintra_process_buffer.hppfocused and avoid a circular-ish coupling between buffer and data definitions.Update intra-process unit tests— updatestest_intra_process_buffer.cppandtest_intra_process_manager.cppfor the newData/IntraProcessBufferDataAPI and adds coverage for message-info propagation across both shared- and unique-ownership subscriptions.Compatibility
Internal to
rclcpp's intra-process implementation (rclcpp::experimental::*); no public API signature changes. Behavior change: intra-process subscribers now observe non-defaultmessage_infowhere they previously saw an empty one.Testing
Built and tested against
rollingin aros:rolling-ros-basecontainer viacolcon build/colcon testforrclcpp(3091 tests, 0 failures), plusuncrustify/cpplintlint targets. All green.Landing order
intra-process-reduce-copies(opened separately) touches the same files (intra_process_buffer.hpp,intra_process_manager.hpp, and both intra-process test files) as part of an unrelated copy-reduction optimization. The two branches are independently buildable and testable, but not free to merge in either order without a rebase — real textual conflicts exist between them (confirmed via a trial merge). No functional dependency either way; whichever of the two merges second will need to reconcile against the other.🤖 Generated with Claude Code