Update for Foxy #34
Merged
Update for Foxy #34
Conversation
This file contains 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
|
@mikaelarguedas looks like this now building agents foxy, all be it from a number of forks. Looks like the navigaiton launch files a have changed some: @SteveMacenski , which launch files should should external packages now use for navigation2? |
|
@ruffsl just remove the My next attack is going to be at the package naming with Side note: I didn't know this org existed. You may want to consider merging this in with https://github.com/ros-security so that all the security stuff is all in 1 place. |
The only nav2 package that differs there is the meta package
This org/repo predates the other buy quite a bit. Not sure why we didn't just rename this org. |
Its just an unimportant detail and adds a bunch of extra typing every time you want to launch something. I think that was initially done because OR at the time had a partially-straight port of Navigation in ROS2 so there were naming collisions. That's not been true since Bouncy. |
|
Given the flat package namespace in ros distros, I think it helps keep things organized and uniquely identifiable. Many of the existing package names would be too vague or colliding if Also, I think it helps discoverability when all related packages are similarly prefixed, like when installing: |
|
Most of these are designed to be able to be used separately so I think it sends the wrong message. Its in the navigation2 metapackage, so that makes alot of sense. But for instance specific plugins or methods aren't "core" to nav2 so the prefix seems odd. If we had 5 controller plugins, are they all the nav2 stack or are they implementations to be used by it? In that way, I think nav2 should be removed from most packages other than things that are "core" to nav2 like nav2_{planner, controller, utils, common, core, bringup}*
Anyhow this is super off topic |
There was a problem hiding this comment.
Cool, Im curious to see if we can actually run the full demo with security now that it's only 1 participant per context.
looks like this now building agents foxy
Yeah, it's still pretty early stage, hopefully once more packages are available we won't need as much handmade workarounds in the dockerfile.
For example adding the osrf apt repos seems unnecessary and we should rely on gazebo being provided in the ros repos.
If you don't need this demo now I'd encourage you to hold off until at least desktop is released and we should have quite less changes needed in this PR.
This PR removse all the non-tb3 parts of the demo, is it intentional?
Side note: I didn't know this org existed. You may want to consider merging this in with https://github.com/ros-security so that all the security stuff is all in 1 place.
Yeah as you can imagine they were not created by the same people, not at the same time, not for the same purpose. We discussed merging them back, but the approach now is that only projects maintained by the working group get moved to the ros-security org, none of the ones here currently are officially maintained by the WG.
The argument that prefixing was for avoiding collision with straight ports seems odd to me, I doubt that's the actual reason, if it were the case, you'd fine at least one distro with both repos released in it
I tend to agree that grouping helps discoverability and that e.g. I like that all parts of moveit are prefixed. I wouldn't know if something similar makes sense for navigation
I seem to recall that being the reason at the time. I did some of those ports.
Alright, seems to be a common theme, I can live with it then. I'm mostly concerned about it due to the aforementioned plugin issue about "what is navigation2" when you have many options. I'm also concerned a little about AMCL, which gives the impression that you must use lidar localization or navigation to use the stack, which is a fundamental misconception many people have that I want to aggressively dispel. So much so that I have the full intent of kicking AMCL and map server out of the stack the minute I can show and document a compelling production-ready visual or depth-based positioning demo (ros-navigation/navigation2#1342). The main blocker there is the existence / setup of an open-source production(ish) grade non-laser slam. When that happens, I'd like it not to bare the |
oh interesting, I guess thats the reason then 👍 RE:prefixing It makes a lot of sense to want to convey what are the "essential parts" and the "plug-able parts". Not sure how it could help in practice but an interesting feature brought by package format 3 is the notion of group. So maybe all navigation planners could declare in their package.xml that they are members of the "navigation_planners" group etc Currently except from cathin_pkg (so colcon) I'm not sure how many tools can leverage that information, but if index.ros.org and others (maybe bloom could add this to the deb info) could use it, then it would be a non-prefix based way to allow discovery. |
That's just temporary until these are sorted:
I wanted to simplify its focus back to sros2, plus I didn't want to maintain those parts of the demo, as they didn't seem active. Although, I've tagged a release that we can reference for posterity:
This is more or less to test the current state of foxy before anything is released, as I wanted to automate the build setup to use with SROS2 on larger ROS graphs, rather than borking my host install. |
|
I've updated this with the foxy release. It still uses an underlay as navigation2 and gazebo_ros_package haven't been released. @mikaelarguedas , if you'd like to test the Dockerfile locally, it looks like the nav2 launch files are more up-to-date than whats in the tb3 gazebo repos. E.g this works fine:
Looks like there are now a number of nodes with duplicate names :< |
as nav2 has been released in foxy
|
@mikaelarguedas I gave it a try using DDS security with RTI connext, but ran into some other QoS issues, like so
So I added a custom QoS profile with logging, but I'm still not sure why nav2 stack fails to start. The DDS log reveal little: |
|
Wow... After thinking about why the it recently dawned on me that the lengthy DDS permissions.xml file generated from the combined single default context was perhaps exceeding the max UDP packet size, and given security handshake for exchanging permissions is over a builtin topic. Per the rti docs above:
This issue probably didn't exhibit before foxy, as the size of a permissions file was relatively much smaller given the one-to-one node profile to dds permissions mapping. Thus signed permission.p7s files never encroached the ~64KB UDP limit. As a comparison, here are two examples where only the admin profiles, like in #34 (comment) is used in the default enclave, vs that currently committed that includes profiles for each node in the one default enclave. Only Admin ProfileAll Node ProfilesI am not sure if FastRTPS can fragmente the permissions.xml file for security channel write event during the Secure DDS handshake, but if it doesn't and is silently failing instead, this may explain why nav2 fails to start with the current security setup. |
|
After switching between the two enclaves above with Connext and FastRTPS, here's what I'm seeing: Only Admin Profile
All Node Profiles
|
|
Exploring the idea of reducing permissions file size by simplifying the rules of a participant by using wildcard:
Changes to permissions templatediff --git a/sros2/sros2/policy/templates/dds/permissions.xsl b/sros2/sros2/policy/templates/dds/permissions.xsl
index e99f535..d5951f1 100644
--- a/sros2/sros2/policy/templates/dds/permissions.xsl
+++ b/sros2/sros2/policy/templates/dds/permissions.xsl
@@ -194,9 +194,7 @@
<xsl:variable name="fqn">
<xsl:apply-templates select="."/>
</xsl:variable>
- <topic>rq<xsl:value-of select="$fqn"/>/_action/cancel_goalRequest</topic>
- <topic>rq<xsl:value-of select="$fqn"/>/_action/get_resultRequest</topic>
- <topic>rq<xsl:value-of select="$fqn"/>/_action/send_goalRequest</topic>
+ <topic>rq<xsl:value-of select="$fqn"/>/_action/*</topic>
</xsl:for-each>
</topics>
</publish>
@@ -206,11 +204,8 @@
<xsl:variable name="fqn">
<xsl:apply-templates select="."/>
</xsl:variable>
- <topic>rr<xsl:value-of select="$fqn"/>/_action/cancel_goalReply</topic>
- <topic>rr<xsl:value-of select="$fqn"/>/_action/get_resultReply</topic>
- <topic>rr<xsl:value-of select="$fqn"/>/_action/send_goalReply</topic>
- <topic>rt<xsl:value-of select="$fqn"/>/_action/feedback</topic>
- <topic>rt<xsl:value-of select="$fqn"/>/_action/status</topic>
+ <topic>rr<xsl:value-of select="$fqn"/>/_action/*</topic>
+ <topic>rt<xsl:value-of select="$fqn"/>/_action/*</topic>
</xsl:for-each>
</topics>
</subscribe>
@@ -223,11 +218,8 @@
<xsl:variable name="fqn">
<xsl:apply-templates select="."/>
</xsl:variable>
- <topic>rr<xsl:value-of select="$fqn"/>/_action/cancel_goalReply</topic>
- <topic>rr<xsl:value-of select="$fqn"/>/_action/get_resultReply</topic>
- <topic>rr<xsl:value-of select="$fqn"/>/_action/send_goalReply</topic>
- <topic>rt<xsl:value-of select="$fqn"/>/_action/feedback</topic>
- <topic>rt<xsl:value-of select="$fqn"/>/_action/status</topic>
+ <topic>rr<xsl:value-of select="$fqn"/>/_action/*</topic>
+ <topic>rt<xsl:value-of select="$fqn"/>/_action/*</topic>
</xsl:for-each>
</topics>
</publish>
@@ -237,9 +229,7 @@
<xsl:variable name="fqn">
<xsl:apply-templates select="."/>
</xsl:variable>
- <topic>rq<xsl:value-of select="$fqn"/>/_action/cancel_goalRequest</topic>
- <topic>rq<xsl:value-of select="$fqn"/>/_action/get_resultRequest</topic>
- <topic>rq<xsl:value-of select="$fqn"/>/_action/send_goalRequest</topic>
+ <topic>rq<xsl:value-of select="$fqn"/>/_action/*</topic>
</xsl:for-each>
</topics>
</subscribe>
Changes to "common" node permissionsdiff --git a/policies/common/lifecycle_node.xml b/policies/common/lifecycle_node.xml
index 6dbe1c0..4ee6dd2 100644
--- a/policies/common/lifecycle_node.xml
+++ b/policies/common/lifecycle_node.xml
@@ -3,14 +3,4 @@
<profile xmlns:xi="http://www.w3.org/2003/XInclude">
<xi:include href="node.xml"
xpointer="xpointer(/profile/*)"/>
- <services reply="ALLOW">
- <service>~/change_state</service>
- <service>~/get_available_states</service>
- <service>~/get_available_transitions</service>
- <service>~/get_state</service>
- <service>~/get_transition_graph</service>
- </services>
- <topics publish="ALLOW">
- <topic>~/transition_event</topic>
- </topics>
</profile>
diff --git a/policies/common/node.xml b/policies/common/node.xml
index c39e117..8e02eaf 100644
--- a/policies/common/node.xml
+++ b/policies/common/node.xml
@@ -1,5 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<profile xmlns:xi="http://www.w3.org/2003/XInclude">
+ <topics publish="ALLOW" subscribe="ALLOW" >
+ <topic>~/*</topic>
+ </topics>
+ <services reply="ALLOW" request="ALLOW" >
+ <service>~/*</service>
+ </services>
<xi:include href="node/logging.xml"
xpointer="xpointer(/profile/*)"/>
<xi:include href="node/time.xml"
diff --git a/policies/common/node/parameters.xml b/policies/common/node/parameters.xml
index dbed939..eae50f5 100644
--- a/policies/common/node/parameters.xml
+++ b/policies/common/node/parameters.xml
@@ -3,13 +3,4 @@
<topics publish="ALLOW" subscribe="ALLOW" >
<topic>parameter_events</topic>
</topics>
-
- <services reply="ALLOW" request="ALLOW" >
- <service>~/describe_parameters</service>
- <service>~/get_parameter_types</service>
- <service>~/get_parameters</service>
- <service>~/list_parameters</service>
- <service>~/set_parameters</service>
- <service>~/set_parameters_atomically</service>
- </services>
</profile>
|
FYI, here is a relevant ticket touching on the large number of nodes in nav2: ros-navigation/navigation2#816 |
|
@mikaelarguedas , touching back on this; today I rebuilt the docker image using the latest Did FastRTPS get re-released into Foxy without building with |
as nav2_bringup seems fails to load entire world
|
With 66a44ec I've added option to use
|
|
Demo now seems to work with the default rmw on foxy binaries, but only without security enabled. Will follow up on tracking down the ROS2 issues with security in separate tickets. |
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.
No description provided.