Improve handling of dynamic discovery - rmw changes (Extends #338)#349
Merged
wjwwood merged 13 commits intoros2:gbiggs/discovery-peers-specificationfrom Mar 21, 2023
Merged
Conversation
arjo129
added a commit
to arjo129/rmw_fastrtps
that referenced
this pull request
Mar 2, 2023
See: * ros2/rmw#349 * ros2/rcl#1038 Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
wjwwood
requested changes
Mar 7, 2023
Member
wjwwood
left a comment
There was a problem hiding this comment.
A few style and implementation questions, as well as some naming considerations, but otherwise looks reasonable to me.
One thing I'm unsure about still, and I'm sure I'll see this elsewhere, is how the static peers are managed? Seems like there should be functions to help modify that array within the discovery params, in order to help avoid mistakes. Similar to how we have helpers for using the array types in rcutils:
https://github.com/ros2/rcutils/blob/rolling/include/rcutils/types/char_array.h#L106-L240
I don't think we need all of those, but some functions to help with resizing and adding/removing would be good I think.
Member
|
As I pointed out a few in the review, there are some linter issues that need to be addressed too: https://build.ros2.org/job/Rpr__rmw__ubuntu_jammy_amd64/93/testReport/ |
Member
|
Also we should consolidate this with #338 and address the feedback there as well. |
This was referenced Mar 9, 2023
ee413aa to
a493020
Compare
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
a574b73 to
135a742
Compare
Member
|
Aside from a conversation about strncpy, I think all my review comments were addressed. I'm going to merge this with geoff's prs and continue there. |
wjwwood
pushed a commit
to arjo129/rmw_fastrtps
that referenced
this pull request
Mar 21, 2023
See: * ros2/rmw#349 * ros2/rcl#1038 Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
wjwwood
pushed a commit
to ros2/rmw_fastrtps
that referenced
this pull request
Mar 21, 2023
See: * ros2/rmw#349 * ros2/rcl#1038 Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
sloretz
added a commit
to ros2/rmw_fastrtps
that referenced
this pull request
Apr 8, 2023
* Support specification of discovery range and static peers Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net> * Apply suggestions from eProsima Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net> * Use participant ignoring Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net> * Improve handling of aliases for hosts Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net> * Adds support for using IP addresses to specify peers This commit adds support for using IP addresses to specify peers. It also refactors out some networking function so that they can be used by other files. Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> * Remove excessive logging Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> * Add name lookup and clean up implementation. Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai> * Two more scenarios fixed. Two more to go. Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai> * Revert rmw changes Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai> * Add support for dynamic allocations See: * ros2/rmw#349 * ros2/rcl#1038 Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai> * Support new requirements Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai> * Update to latest rmw API Signed-off-by: Michael X. Grey <grey@openrobotics.org> * Update to use API on Fast-DDS master Signed-off-by: Shane Loretz <sloretz@google.com> * Update with rmw_discovery_options_t changes Signed-off-by: Shane Loretz <sloretz@google.com> * Minimize diff with rolling in custom_participant_info.hpp Signed-off-by: Shane Loretz <sloretz@google.com> * Minimize diff with rolling in participant.cpp Signed-off-by: Shane Loretz <sloretz@google.com> * Collapse lines Signed-off-by: Shane Loretz <sloretz@google.com> * conditional on one line Signed-off-by: Shane Loretz <sloretz@google.com> * return instead of setting variable Signed-off-by: Shane Loretz <sloretz@google.com> * Make log messages more informative Signed-off-by: Shane Loretz <sloretz@google.com> * Works without ignore participant change! Signed-off-by: Shane Loretz <sloretz@google.com> * Remove unused code Signed-off-by: Shane Loretz <sloretz@google.com> * Remove more unused code Signed-off-by: Shane Loretz <sloretz@google.com> * Minimize diff with rolling Signed-off-by: Shane Loretz <sloretz@google.com> * NOT_SET and SYSTEM_DEFAULT values Signed-off-by: Shane Loretz <sloretz@google.com> * OFF implementation that doesn't crash Signed-off-by: Shane Loretz <sloretz@google.com> * Set discovery range in test Signed-off-by: Shane Loretz <sloretz@google.com> * Lint Signed-off-by: Shane Loretz <sloretz@google.com> * Call rmw_discovery_options_init() Signed-off-by: Shane Loretz <sloretz@google.com> * Workaround deadlock with rclcpp global logging mutex Signed-off-by: Shane Loretz <sloretz@google.com> * Add shared memory transport for LOCALHOST traffic Signed-off-by: Shane Loretz <sloretz@google.com> * Configure max initial peers range on udp transport Signed-off-by: Shane Loretz <sloretz@google.com> * Disable built-in transports and fix lint Signed-off-by: Shane Loretz <sloretz@google.com> * Error when range is an invalid value Signed-off-by: Shane Loretz <sloretz@google.com> * undo unnecessary test change Signed-off-by: Shane Loretz <sloretz@google.com> * Document Setting range to SYSTEM_DEFAULT Signed-off-by: Shane Loretz <sloretz@google.com> * Limit participants to 1 when discover is OFF Signed-off-by: Shane Loretz <sloretz@google.com> * With SUBNET and initial peers, add default multicast address as a multicast locator Signed-off-by: Shane Loretz <sloretz@google.com> * Add multicast address to initial peer list when there are other static peers and SUBNET range Signed-off-by: Shane Loretz <sloretz@google.com> * Bump required Fast-DDS version to 2.10 Signed-off-by: Shane Loretz <sloretz@google.com> * Set maxInitialPeersRange to 32 Signed-off-by: Shane Loretz <sloretz@google.com> * Grammar Signed-off-by: Shane Loretz <sloretz@google.com> * Shorten sentence Signed-off-by: Shane Loretz <sloretz@google.com> --------- Signed-off-by: Geoffrey Biggs <gbiggs@killbots.net> Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai> Signed-off-by: Michael X. Grey <grey@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@google.com> Co-authored-by: Arjo Chakravarty <arjo@openrobotics.org> Co-authored-by: Arjo Chakravarty <arjoc@intrinsic.ai> Co-authored-by: Michael X. Grey <grey@openrobotics.org> Co-authored-by: Shane Loretz <sloretz@google.com>
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
3 participants
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 PR extends #338 to support dynamic memory allocation.