Renamed ign to gz #67

ahcorde · 2022-06-16T15:26:02Z

Signed-off-by: ahcorde ahcorde@gmail.com

Renamed ign to gz

README.md

gz_ros2_control/CMakeLists.txt

gz_ros2_control/include/gz_ros2_control/gz_system.hpp

gz_ros2_control/src/gz_ros2_control_plugin.cpp

chapulina

Just a few preliminary comments. I suggest setting up CI for all target versions so that it's possible to verify that they all work as intended

gz_ros2_control/CMakeLists.txt

chapulina · 2022-08-30T15:41:07Z

gz_ros2_control/include/gz_ros2_control/gz_ros2_control_plugin.hpp

+
+#ifdef GZ_HEADERS
+#include <gz/sim/System.hh>
+#define GZ_SIM_NAMESPACE gz::sim::


Soon you won't need to define this, because we're backporting the gz::sim namespace to Citadel and Fortress (gazebo-tooling/release-tools#784)

gz_ros2_control/package.xml

gz_ros2_control_demos/urdf/test_cart_velocity.xacro.urdf

ahcorde · 2022-09-07T21:56:50Z

@destogl the gripper mimic example is not working, do you mind to take a look ? Otherwise I will open an issue

ahcorde · 2022-09-08T11:38:08Z

@destogl I think I fixed the issue, do you mind to review this changes 23bf9cd

If the joint reach the limit position it's blocked

destogl · 2022-09-10T18:46:39Z

@destogl I think I fixed the issue, do you mind to review this changes 23bf9cd

If the joint reach the limit position it's blocked

This change seems OK to me. @livanov93 is fixing this in #86

ahcorde · 2022-10-05T19:29:16Z

Hi @destogl and @@livanov93,

I was debugging the issue and these are my notes:

there is an issue in dart Servo joint doesn't respect position limits dartsim/dart#1583 that might be affecting, it's related with respect positional limits. The fix should be already released in dart6.11.
I tested this plugin with another physics engine bullet-featherstone Add bullet-featherstone plugin gazebosim/gz-physics#373. It's on current development and it's working, I only found a minor issue with mimic joints (The gripper example).

I will try to follow up with a fix for dart.

azeey · 2022-11-14T19:46:25Z

Needs another round of reviews.

jlamperez · 2022-11-19T22:41:05Z

Hi, I am trying this PR with humble and garden and I am not able to load the libgz_ros2_control-system.so:

[ign gazebo-1] Library [/workspaces/robotic_arm_ros2_test_ws/install/lib/libgz_ros2_control-system.so] does not export any plugins. The symbol [IgnitionPluginHook] is missing, or it is not externally visible.
[ign gazebo-1] [Err] [SystemLoader.cc:87] Failed to load system plugin [libgz_ros2_control-system.so] : couldn't load library on path [/workspaces/robotic_arm_ros2_test_ws/install/lib/libgz_ros2_control-system.so].

Any idea how to solve this?

ahcorde · 2022-11-21T14:42:38Z

Hey Jorge! @jlamperez

jumm are you trying any of the example worlds?

I think you should update your files, I can see IgnitionPluginHook, do you have a repo that I can check?

jlamperez · 2022-11-21T18:22:11Z

@ahcorde you can reproduce it like this:

Run this docker in Dockerhub

docker run -it --rm -e DISPLAY=$DISPLAY --user ros:ros --network host -v /tmp/.X11-unix/:/tmp/.X11-unix -v /dev/shm:/dev/shm jlamperez/ros2:humble-cuda-gz-nvidia

And based on the README

sudo apt-get update 
source /opt/ros/humble/setup.bash 
mkdir -p ~/ros_ign/src 
cd ~/ros_ign/src/ 
git clone https://github.com/ros-controls/gz_ros2_control 
cd gz_ros2_control 
git checkout remotes/origin/ahcorde/rename/ign_to_gz 
cd .. 
export GZ_VERSION=garden 
rosdep update 
rosdep install -r --from-paths . --ignore-src --rosdistro $ROS_DISTRO -y 
cd ..
colcon build 
. install/local_setup.bash

Try the examples:

ros2 launch gz_ros2_control_demos cart_example_position.launch.py

or

ros2 launch gz_ros2_control_demos diff_drive_example.launch.py

I try sending commands but it is not working.

$ ros2 run gz_ros2_control_demos example_position
node created
terminate called after throwing an instance of 'std::runtime_error'
  what():  could not get action server
[ros2run]: Aborted

ros2 run gz_ros2_control_demos example_diff_drive

After a while joint_state_broadcaster and joint_trajectory_controller` die

[ros2-4] Could not contact service /controller_manager/load_controller
[ERROR] [ros2-4]: process has died [pid 2941, exit code 1, cmd 'ros2 control load_controller --set-state active joint_state_broadcaster'].
[INFO] [ros2-5]: process started with pid [3149]
[ros2-5] Could not contact service /controller_manager/load_controller
[ERROR] [ros2-5]: process has died [pid 3149, exit code 1, cmd 'ros2 control load_controller --set-state active joint_trajectory_controller'].

ahcorde · 2022-11-21T22:21:36Z

@jlamperez something is broken in ros_gz, can you compile these packages from source too ? It's working for me, I'm the maintener of ros_gz working on a fix and a release.

Thank you for reporting

ahcorde · 2022-11-21T22:36:32Z

@jlamperez the release should be out today or tomorrow https://discourse.ros.org/t/preparing-for-humble-sync-and-patch-release-2022-11-21/28234/4 with the patches

jlamperez · 2022-11-21T22:53:36Z

Yes, now when I compile ros_gz from the source in the humble branch the plugin is loaded and cart_example_position is working but I am seeing a Segmentation fault at the end. Is this correct?

SUCCEEDED result code
async_send_goal
cannot publish data, at ./src/rmw_publish.cpp:62 during '__function__'
[ERROR] [1669070763.487046078] [trajectory_test_node.rclcpp]: Error in destruction of rcl subscription handle: Failed to delete datareader, at ./src/subscription.cpp:52, at ./src/rcl/subscription.c:183
cannot publish data, at ./src/rmw_publish.cpp:62 during '__function__'
Fail in delete datareader, at ./src/rmw_service.cpp:103 during '__function__'
[ros2run]: Segmentation fault

The other examples are working well for me.

Thank you for your help!

ahcorde · 2022-11-22T09:00:35Z

@jlamperez maybe the memory/node is not destroyed properly. I will check it

azeey · 2022-12-05T20:00:37Z

Can you add me as a reviewer?

ahcorde · 2022-12-06T07:18:55Z

@azeey I can assign you to the issue, but I can't add you as a reviewer

azeey · 2023-02-03T23:41:19Z

.github/workflows/ci.yaml

      matrix:
-        version: [fortress]
+        include:
+          - docker-image: "ubuntu:22.04"
+            gz-version: "fortress"
+            ros-distro: "humble"
+          - docker-image: "ubuntu:22.04"
+            gz-version: "fortress"
+            ros-distro: "rolling"
+          - docker-image: "ubuntu:22.04"
+            gz-version: "garden"
+            ros-distro: "rolling"


I think this can be simplified as:

Suggested change

matrix:

version: [fortress]

include:

- docker-image: "ubuntu:22.04"

gz-version: "fortress"

ros-distro: "humble"

- docker-image: "ubuntu:22.04"

gz-version: "fortress"

ros-distro: "rolling"

- docker-image: "ubuntu:22.04"

gz-version: "garden"

ros-distro: "rolling"

matrix:

- docker-image: ["ubuntu:22.04"]

- gz-version: ["fortress", "garden"]

- ros-distro: ["humble", "rolling"]

azeey · 2023-02-03T23:43:48Z

.github/workflows/ci.yaml

+        sh -c 'echo "deb http://packages.osrfoundation.org/gazebo/ubuntu-stable `lsb_release -cs` main" > /etc/apt/sources.list.d/gazebo-stable.list'
+        wget https://packages.osrfoundation.org/gazebo.key -O - | apt-key add -
+        if [ "$GZ_VERSION" == "garden" ]; then
+          GZ_DEPS="libgz-sim7-dev libgz-plugin2-dev"


GZ_DEPS is set here, but I don't see where it's being used.

This is resolved now, but I don't have permissiont o resolve threads in this repo.

README.md

azeey · 2023-02-04T04:38:32Z

gz_ros2_control/package.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0"?>


Git is treating this as a new file. Can you use git mv on the ign version of this file so it's treated as a rename?

gz_ros2_control/src/gz_system.cpp

azeey · 2023-02-04T04:43:54Z

gz_ros2_control/src/gz_system.cpp

@@ -336,7 +355,6 @@ bool IgnitionSystem::initSim(
          this->dataPtr->joints_[j].joint_velocity_cmd = initial_velocity;
        }
      } else if (joint_info.command_interfaces[i].name == "effort") {
-        this->dataPtr->joints_[j].joint_control_method |= EFFORT;


Was this removed intentionally?

yes, it's intentionally

Why was this removed? I believe it makes sense, but would like to understand.

because this is not part of the registration

gz_ros2_control/gz_hardware_plugins.xml

azeey

Made some more suggestions to keep it backward compatible, i.e, you can still use names with ign in them in SDF/URDF/xacro files.

azeey · 2023-02-04T05:16:32Z

.github/workflows/ci.yaml

@@ -28,6 +39,12 @@ jobs:
        cp -r gz_ros2_control /home/ros2_ws/src/
        sh -c 'echo "deb http://packages.ros.org/ros2-testing/ubuntu `lsb_release -cs` main" > /etc/apt/sources.list.d/ros2-testing.list'
        curl -s https://raw.githubusercontent.com/ros/rosdistro/master/ros.asc | apt-key add -
+        sh -c 'echo "deb http://packages.osrfoundation.org/gazebo/ubuntu-stable `lsb_release -cs` main" > /etc/apt/sources.list.d/gazebo-stable.list'


We should only enable packages.osrfoundation.org if we're testing Garden. For Fortress, we should use what's available in packages.ros.org.

gz_ros2_control_demos/urdf/test_cart_velocity.xacro.urdf

gz_ros2_control/gz_hardware_plugins.xml

gz_ros2_control/package.xml

gz_ros2_control/src/gz_ros2_control_plugin.cpp

ahcorde · 2023-02-14T18:59:39Z

@azeey I included some of your suggestions, let me know what do you think.

azeey

The changes look good! There are still some unresolved comments, so I'll wait
for those before I approve.

azeey · 2023-02-14T19:48:44Z

.github/workflows/ci.yaml

+        sh -c 'echo "deb http://packages.osrfoundation.org/gazebo/ubuntu-stable `lsb_release -cs` main" > /etc/apt/sources.list.d/gazebo-stable.list'
+        wget https://packages.osrfoundation.org/gazebo.key -O - | apt-key add -
+        if [ "$GZ_VERSION" == "garden" ]; then
+          GZ_DEPS="libgz-sim7-dev libgz-plugin2-dev"


This is resolved now, but I don't have permissiont o resolve threads in this repo.

azeey · 2023-02-14T20:02:06Z

gz_ros2_control/include/gz_ros2_control/gz_system_interface.hpp


 #include <hardware_interface/system_interface.hpp>
 #include <hardware_interface/types/hardware_interface_type_values.hpp>

 #include <rclcpp/rclcpp.hpp>

-namespace ign_ros2_control
+namespace gz_ros2_control


So, I guess the question is do we plan to backport this to older branches? If so, existing code would fail to compile.

destogl · 2023-02-21T18:32:05Z

@ahcorde I needed this PR to get compile this package against the last released rolling. Probably we can continue with merging. What do you think? What is needed to finish this?

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

ahcorde · 2023-02-22T15:07:01Z

@azeey i used git mv, but I had to rewrite the history.

@destogl I think with the last changes it's ready for a final pass.

stale

destogl

Just a few minor comments.

.github/workflows/ci.yaml

destogl · 2023-02-22T18:09:43Z

gz_ros2_control/include/gz_ros2_control/gz_system_interface.hpp


 #include <hardware_interface/system_interface.hpp>
 #include <hardware_interface/types/hardware_interface_type_values.hpp>

 #include <rclcpp/rclcpp.hpp>

-namespace ign_ros2_control
+namespace gz_ros2_control


since there is a Humble release already, we probably can not backport anymore without breaking API. Rolling can break anyway.

destogl · 2023-02-22T18:12:20Z

gz_ros2_control/src/gz_system.cpp

@@ -336,7 +355,6 @@ bool IgnitionSystem::initSim(
          this->dataPtr->joints_[j].joint_velocity_cmd = initial_velocity;
        }
      } else if (joint_info.command_interfaces[i].name == "effort") {
-        this->dataPtr->joints_[j].joint_control_method |= EFFORT;


Why was this removed? I believe it makes sense, but would like to understand.

Co-authored-by: Dr. Denis <denis@stoglrobotics.de>

destogl · 2023-03-01T05:44:19Z

@Mergifyio backport humble

mergify · 2023-03-01T05:44:22Z

backport humble

✅ Backports have been created

#126 Renamed ign to gz (backport #67) has been created for branch humble

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com> (cherry picked from commit ab810e7) # Conflicts: # .github/workflows/ci.yaml # Dockerfile/Dockerfile # README.md # gz_ros2_control/src/gz_ros2_control_plugin.cpp # gz_ros2_control/src/gz_system.cpp # gz_ros2_control_demos/config/gripper_controller.yaml # gz_ros2_control_demos/examples/example_gripper.cpp # gz_ros2_control_demos/launch/cart_example_effort.launch.py # gz_ros2_control_demos/launch/cart_example_position.launch.py # gz_ros2_control_demos/launch/cart_example_velocity.launch.py # gz_ros2_control_demos/launch/diff_drive_example.launch.py # gz_ros2_control_demos/launch/gripper_mimic_joint_example.launch.py # gz_ros2_control_demos/launch/tricycle_drive_example.launch.py # gz_ros2_control_demos/package.xml # gz_ros2_control_demos/urdf/test_gripper_mimic_joint.xacro.urdf # ign_ros2_control/package.xml

ahcorde self-assigned this Jun 16, 2022

chapulina previously requested changes Jun 16, 2022

View reviewed changes

chapulina added the enhancement New feature or request label Jul 23, 2022

ahcorde changed the base branch from main to master August 27, 2022 09:34

ahcorde requested a review from chapulina August 29, 2022 08:21

chapulina reviewed Aug 30, 2022

View reviewed changes

graiola mentioned this pull request Sep 3, 2022

Rename ign to gz ros-controls/ign_ros_control#4

Open

ahcorde mentioned this pull request Oct 5, 2022

Mimic Joint Improvement #86

Closed

This was referenced Nov 5, 2022

Joint Controllers not working properly when joint limits are specified gazebosim/gz-sim#1684

Open

Servo joint doesn't respect position limits dartsim/dart#1583

Closed

Joint can't recover from position limits in servo mode dartsim/dart#1683

Closed

ahcorde mentioned this pull request Nov 22, 2022

Failed to load system plugin [gz_ros2_control-system] #98

Closed

ahcorde assigned azeey Dec 6, 2022

azeey reviewed Feb 4, 2023

View reviewed changes

ahcorde requested a review from chapulina February 14, 2023 18:59

ahcorde requested a review from azeey February 14, 2023 18:59

azeey reviewed Feb 14, 2023

View reviewed changes

ahcorde added 2 commits February 22, 2023 15:56

git move files

d44c148

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

Include all changes

a614678

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>

ahcorde force-pushed the ahcorde/rename/ign_to_gz branch from 97d021b to a614678 Compare February 22, 2023 15:04

chapulina removed their request for review February 22, 2023 16:53

destogl approved these changes Feb 22, 2023

View reviewed changes

Apply suggestions from code review

ca6f44a

Co-authored-by: Dr. Denis <denis@stoglrobotics.de>

ahcorde merged commit ab810e7 into master Feb 23, 2023

ahcorde deleted the ahcorde/rename/ign_to_gz branch February 23, 2023 14:40

mergify bot mentioned this pull request Mar 1, 2023

Renamed ign to gz (backport #67) #126

Open

Renamed ign to gz #67

Renamed ign to gz #67

Conversation

ahcorde commented Jun 16, 2022

chapulina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahcorde commented Sep 7, 2022

ahcorde commented Sep 8, 2022

destogl commented Sep 10, 2022

ahcorde commented Oct 5, 2022

azeey commented Nov 14, 2022

jlamperez commented Nov 19, 2022

ahcorde commented Nov 21, 2022

jlamperez commented Nov 21, 2022

ahcorde commented Nov 21, 2022

ahcorde commented Nov 21, 2022

jlamperez commented Nov 21, 2022

ahcorde commented Nov 22, 2022

azeey commented Dec 5, 2022

ahcorde commented Dec 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahcorde commented Feb 14, 2023

azeey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

destogl commented Feb 21, 2023

ahcorde commented Feb 22, 2023

destogl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

destogl commented Mar 1, 2023

mergify bot commented Mar 1, 2023 • edited Loading

✅ Backports have been created

mergify bot commented Mar 1, 2023 •

edited

Loading