-
Notifications
You must be signed in to change notification settings - Fork 119
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
Forward sdf ros remappings to loaded controllers #80
Conversation
@@ -200,6 +200,35 @@ void GazeboRosControlPlugin::Load(gazebo::physics::ModelPtr parent, sdf::Element | |||
// So we have to parse the plugin file manually and set it to the node's context. | |||
auto rcl_context = impl_->model_nh_->get_node_base_interface()->get_context()->get_rcl_context(); | |||
std::vector<std::string> arguments = {"--ros-args", "--params-file", impl_->param_file_.c_str()}; | |||
if (sdf->HasElement("ros")) { | |||
sdf = sdf->GetElement("ros"); | |||
} |
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.
This will allow the following
<plugin>
<namespace></namespace>
<remapping></remapping>
...
</plugin>
and
<plugin>
<ros>
<namespace></namespace>
<remapping></remapping>
...
</ros>
</plugin>
I think we should only go with the second option
Hi, and thanks.
The behavior mirrors that of gazebo_ros node.cpp, which is the file that
reads the xml structure currently. While I agree a single standard would be
better, this follows the current behaviour more closely (although it might
be an unintended feature)
Best Regards,
Jonatan
…On Fri, 11 Jun 2021, 19:59 Alejandro Hernández Cordero, < ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In gazebo_ros2_control/src/gazebo_ros2_control_plugin.cpp
<#80 (comment)>
:
> @@ -200,6 +200,35 @@ void GazeboRosControlPlugin::Load(gazebo::physics::ModelPtr parent, sdf::Element
// So we have to parse the plugin file manually and set it to the node's context.
auto rcl_context = impl_->model_nh_->get_node_base_interface()->get_context()->get_rcl_context();
std::vector<std::string> arguments = {"--ros-args", "--params-file", impl_->param_file_.c_str()};
+ if (sdf->HasElement("ros")) {
+ sdf = sdf->GetElement("ros");
+ }
This will allow the following
<plugin>
<namespace></namespace>
<remapping></remapping>
...
</plugin>
and
<plugin>
<ros>
<namespace></namespace>
<remapping></remapping>
...
</ros>
</plugin>
I think we should only go with the second option
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#80 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABQOYS2JO7YONQPABZEIGDTSJFJRANCNFSM46PLRLZA>
.
|
But yes, I could support either way
On Fri, 11 Jun 2021, 20:13 Jonatan Olofsson, ***@***.***>
wrote:
… Hi, and thanks.
The behavior mirrors that of gazebo_ros node.cpp, which is the file that
reads the xml structure currently. While I agree a single standard would be
better, this follows the current behaviour more closely (although it might
be an unintended feature)
Best Regards,
Jonatan
On Fri, 11 Jun 2021, 19:59 Alejandro Hernández Cordero, <
***@***.***> wrote:
> ***@***.**** requested changes on this pull request.
> ------------------------------
>
> In gazebo_ros2_control/src/gazebo_ros2_control_plugin.cpp
> <#80 (comment)>
> :
>
> > @@ -200,6 +200,35 @@ void GazeboRosControlPlugin::Load(gazebo::physics::ModelPtr parent, sdf::Element
> // So we have to parse the plugin file manually and set it to the node's context.
> auto rcl_context = impl_->model_nh_->get_node_base_interface()->get_context()->get_rcl_context();
> std::vector<std::string> arguments = {"--ros-args", "--params-file", impl_->param_file_.c_str()};
> + if (sdf->HasElement("ros")) {
> + sdf = sdf->GetElement("ros");
> + }
>
> This will allow the following
>
> <plugin>
> <namespace></namespace>
> <remapping></remapping>
> ...
> </plugin>
>
> and
>
> <plugin>
> <ros>
> <namespace></namespace>
> <remapping></remapping>
> ...
> </ros>
> </plugin>
>
> I think we should only go with the second option
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#80 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABQOYS2JO7YONQPABZEIGDTSJFJRANCNFSM46PLRLZA>
> .
>
|
@ahcorde Could you please advice if I should update the PR or not, based on the argument above? |
@jonatanolofsson yes, please I would prefer to update the PR |
edbdf10
to
d590ab8
Compare
@ahcorde Done! |
Hi @jonatanolofsson ,
I can manually call the service Thanks in advance for the help. |
@cco-seasony If you're using spawner.py, you can use the -c parameter to change which controller_manager it talks to. There might be a similar (same?) parameter for the ros2 control interface |
Currently, the loaded controllers do not receive the remappings and namespace info from the sdf. This commit adds this information to the generated argument list so that they are forwarded to subsequently started nodes.