-
Notifications
You must be signed in to change notification settings - Fork 775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pick smallest available participant ID for new paricipants (backport #3437) #4555
Conversation
* Pick smallest available participant ID for new paricipants Signed-off-by: Shane Loretz <[email protected]> * Fix style issues Signed-off-by: Shane Loretz <[email protected]> * Eliminate m_maxRTPSParticipantId Signed-off-by: Shane Loretz <[email protected]> * Clearer comments on rationale behind return value Signed-off-by: Shane Loretz <[email protected]> * static_cast to avoid warning on WIN32 Signed-off-by: Shane Loretz <[email protected]> * Use special participant id value for prefix creation. Signed-off-by: Miguel Company <[email protected]> * New reservation mechanism Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Shane Loretz <[email protected]> Signed-off-by: Miguel Company <[email protected]> Co-authored-by: Miguel Company <[email protected]> (cherry picked from commit e46e875) # Conflicts: # src/cpp/rtps/RTPSDomain.cpp # src/cpp/rtps/RTPSDomainImpl.hpp
Cherry-pick of e46e875 has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Signed-off-by: Jesus Perez <[email protected]>
Signed-off-by: Jesus Perez <[email protected]>
Signed-off-by: Jesus Perez <[email protected]>
@richiprosima please test this |
I am reordering windows and mac ci since there are more test failures than usual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mac OS github ci failed preparing the workspace when re-running the job. Nevertheless, jenkins ci was green and github failed tests on the first attempt are unrelated to this PR. Ready to merge.
Picks the smallest available participant ID.
Description
This picks the smallest available participant ID when creating new participants.
This is necessary for the out of box discovery improvements in ROS 2: ros2/rmw_fastrtps#653
The issue with picking one greater than the maximum participant ID is that it's possible for the ID to increase well beyond the number of alive participants. For example, Participant A is created with 0, and B is created with 1. A is destroyed, and C is created. C takes ID 2 even though 0 is available and there are only two participants. When using unicast discovery this can cause participants to no longer be discoverable after the maximum ID climbs past
maxInititialPeersRange
. This is currently causing thetest_launch_ros
tests to fail with the change to unicast discovery by default.Assuming the change looks ok, would it be possible to backport this to the 2.10 branch (assuming that's the branch that will be used for ROS Iron)?
Contributor Checklist
Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
Any new/modified methods have been properly documented using Doxygen.
Changes are ABI compatible.
Changes are API compatible.
N/A New feature has been added to the
versions.md
file (if applicable).N/A New feature has been documented/Current behavior is correctly described in the documentation.
N/A Applicable backports have been included in the description.
Reviewer Checklist
This is an automatic backport of pull request #3437 done by [Mergify](https://mergify.com).