Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Indicate missing support for unique network flows#484

Merged
wjwwood merged 4 commits intoros2:masterfrom
Ericsson:unique_network_flows
Apr 5, 2021
Merged

Indicate missing support for unique network flows#484
wjwwood merged 4 commits intoros2:masterfrom
Ericsson:unique_network_flows

Conversation

@anamud
Copy link
Copy Markdown
Contributor

@anamud anamud commented Feb 9, 2021

This PR indicates missing support in rmw_connext for unique network flows for publishers and subscribers in communicating nodes. Such indication is required for graceful error handling in the ROS2 RMW and above layers.

Associated pull-requests:

Initial contributions stem from Ericsson and eProsima.

Signed-off-by: Ananya Muddukrishna ananya.x.muddukrishna@ericsson.com

Signed-off-by: Ananya Muddukrishna <ananya.x.muddukrishna@ericsson.com>
Ananya Muddukrishna added 3 commits February 25, 2021 14:21
Signed-off-by: Ananya Muddukrishna <ananya.x.muddukrishna@ericsson.com>
Signed-off-by: Ananya Muddukrishna <ananya.x.muddukrishna@ericsson.com>
Signed-off-by: Ananya Muddukrishna <ananya.x.muddukrishna@ericsson.com>
@wjwwood wjwwood merged commit 48617c5 into ros2:master Apr 5, 2021
// limitations under the License.

#include "rmw/error_handling.h"
#include "rmw/get_network_flow_endpoint.h"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include "rmw/get_network_flow_endpoint.h"
#include "rmw/get_network_flow_endpoints.h"

The name of the file looks wrong
https://github.com/Ericsson/ros2-rmw/blob/c1c25072c199c3177121768a612243386b84b054/rmw/include/rmw/get_network_flow_endpoints.h#L1 (?)

Copy link
Copy Markdown
Contributor Author

@anamud anamud Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivanpauno Thanks for noticing the mistake. I understood that this repository is going to be deprecated so did not include it in my integration recent tests with Rolling Ridley. Also, the ros2.repos file for Rolling Ridley does not include this repository.

Since this PR got merged, do I go ahead and create a new PR to fix the error?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this PR got merged, do I go ahead and create a new PR to fix the error?

Yes, thanks!
We don't need to continue adding new features here, but if we do we shouldn't break it.
(I know that there are people still using this package).

Copy link
Copy Markdown
Contributor Author

@anamud anamud Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I submitted a PR with corrections. Sorry about overlooking this.

Please ensure that it passes compile-time checks and does not break existing functionality. I am unable to test fully due to lack of a licensed RTI Connext system at my side.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants