Adding demo for Example 3 : Industrial Robots with integrated sensor #68

bailaC · 2021-02-18T17:10:53Z

No description provided.

codecov-io · 2021-02-19T14:02:24Z

Codecov Report

Merging #68 (7e53311) into master (046ce0c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #68    +/-   ##
=======================================
  Coverage    0.00%   0.00%            
=======================================
  Files           7       2     -5     
  Lines         504     163   -341     
=======================================
+ Misses        504     163   -341

Flag	Coverage Δ
unittests	`0.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...l_demo_hardware/src/rrbot_system_position_only.cpp
...l_demo_hardware/src/rrbot_system_position_only.cpp
...l_demo_hardware/src/rrbot_system_position_only.cpp
...l_demo_hardware/src/rrbot_system_position_only.cpp
...l_demo_hardware/src/rrbot_system_position_only.cpp
...l_demo_hardware/src/rrbot_system_position_only.cpp
...l_demo_hardware/src/rrbot_system_position_only.cpp
...rol_demo_hardware/src/rrbot_system_with_sensor.cpp	`0.00% <0.00%> (ø)`
...l_demo_hardware/src/rrbot_system_position_only.cpp	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 046ce0c...7e53311. Read the comment docs.

destogl

Great work!
.
Changes in short:

I saw some issues I did in Example 1 and you copied it here. After its adaption here, the have to be also addressed for the other example.

Also, it would be good to update the .ros2_control.xacro file as proposed in ros-controls/roadmap#32 and adapt the scenario to it. I would like the sensor to returns some random data than a fixed value.

ros2_control_demo_hardware/include/ros2_control_demo_hardware/rrbot_system_with_sensor.hpp

destogl · 2021-02-20T18:45:50Z

ros2_control_demo_hardware/include/ros2_control_demo_hardware/rrbot_system_with_sensor.hpp

+#include "hardware_interface/types/hardware_interface_status_values.hpp"
+#include "ros2_control_demo_hardware/visibility_control.h"
+
+using hardware_interface::return_type;


@bmagyar, @Karsten1987, @v-lopez
This is not encouraged by the ROS1 style guide and by google style.

Should we remove "using" from the official examples?

I agree (and saw you did on the PR), specially with such a generic name as return_type.

@bailaC can you also remove this using? And then adapt the files accordingly?

These names are long and mostly meaningless. The motivation was to keep them as short as possible, especially in compilation units that are very likely to be leaf nodes such as these system plugins. I personally prefer short, readable code to adhering to semi-enterprise, cover-all-cases style guides. That is only my opinion though, feel free to change it if you see fit.

ros2_control_demo_hardware/ros2_control_demo_hardware.xml

ros2_control_demo_robot/description/rrbot/rrbot_system_with_sensor.ros2_control.xacro

ros2_control_demo_robot/description/rrbot_system_with_sensor.urdf.xacro

ros2_control_demo_robot/launch/rrbot_system_with_sensor.launch.py

ros2_control_demo_hardware/include/ros2_control_demo_hardware/rrbot_system_with_sensor.hpp

ros2_control_demo_hardware/ros2_control_demo_hardware.xml

destogl · 2021-02-22T17:50:16Z

I just checked with the suggested changes. It is now failing for ['topic_name' parameter was empty]. I think I understand that adding a new parameter named 'topic_name' will resolve this issue.

Can you suggest what should be the final change here? Should I keep it as it is considering it will be corrected in the future anyway?

We have to create an official controller/publisher in ros2_controller repo. I will get to you in the next two days about this.

ros2_control_demo_hardware/src/rrbot_system_with_sensor.cpp

* Add corrections and extensions for the rrbot * Correct build and remove demo nodes from readme. * Apply suggestions from code review Co-authored-by: Bence Magyar <[email protected]> * Adjust colors of the robot * Update ros2_control_demo_robot/package.xml Co-authored-by: Victor Lopez <[email protected]> * Update ros2_control_demo_robot/launch/test_rrbot_description.launch.py Co-authored-by: Bence Magyar <[email protected]> Co-authored-by: Victor Lopez <[email protected]>

# Conflicts: # ros2_control_demo_robot/config/rrbot_with_sensor_controllers.yaml

destogl

See proposals

destogl · 2021-06-12T15:31:54Z

ros2_control_demo_robot/launch/rrbot_system_with_sensor.launch.py

+            "slowdown": slowdown,
+        }.items(),
+    )
+


Starting of FTS Broadcaster is missing:

Suggested change

robot_controller_spawner = Node(

package="controller_manager",

executable="spawner.py",

arguments=["fts_broadcaster", "-c", "/controller_manager"],

)

destogl · 2021-06-12T15:32:30Z

ros2_control_demo_robot/config/rrbot_with_sensor_controllers.yaml

+    forward_position_controller:
+      type: forward_command_controller/ForwardCommandController
+
+    fts_controller:


Suggested change

fts_controller:

fts_broadcaster:

destogl · 2021-06-12T15:32:36Z

ros2_control_demo_robot/config/rrbot_with_sensor_controllers.yaml

+      - joint2
+    interface_name: position
+
+fts_controller:


Suggested change

fts_controller:

fts_broadcaster:

bailaC added 4 commits February 18, 2021 22:36

Adding demo for Example 3 : Industrial Robots with integrated sensor

0866c42

Adding demo for Example 3 : Industrial Robots with integrated sensor

b22dd4d

Correcting directory

ba8699e

cpplint fixes

63294eb

bailaC added 2 commits February 19, 2021 19:34

cpplint fix

6310687

uncrustify fixes

a6f2052

destogl mentioned this pull request Feb 20, 2021

Update FTS tcp and return FTS state parameters ros-controls/roadmap#32

Open

destogl requested changes Feb 20, 2021

View reviewed changes

destogl mentioned this pull request Feb 20, 2021

Create descriptions for robots from examples #5

Open

3 tasks

destogl linked an issue Feb 20, 2021 that may be closed by this pull request

Create demo for example 3 : Industrial Robots with integrated sensor #67

Closed

destogl mentioned this pull request Feb 20, 2021

Correct trivial issues in implemenation of Example 1 #69

Closed

4 tasks

Addressing review comments

76d8a4f

destogl mentioned this pull request Feb 22, 2021

demo_diff_drive_controller #70

Merged

destogl requested changes Feb 22, 2021

View reviewed changes

bailaC added 4 commits February 24, 2021 18:07

addressing review comments

5d83ae1

addressing review comments

be0f534

cpplint fix

0964132

build fix

edf9b04

bmagyar reviewed Mar 5, 2021

View reviewed changes

ros2_control_demo_hardware/src/rrbot_system_with_sensor.cpp Outdated Show resolved Hide resolved

bailaC and others added 9 commits March 6, 2021 12:29

Addressing review comments

d56c698

Addressing review comments

781f388

Merge remote-tracking branch 'upstream/master'

6c5b485

# Conflicts: # ros2_control_demo_robot/config/rrbot_with_sensor_controllers.yaml

Merge remote-tracking branch 'origin/master'

7e53311

Adding changes for SR_issue_2

fbb840c

Q003 error fix

263377c

Merge branch 'master' into master

21599cd

Example-3 : Adapting changes from PR-76

60cd85a

destogl reviewed Jun 12, 2021

View reviewed changes

destogl closed this Jun 17, 2021

+    robot_controller_spawner = Node(
+        package="controller_manager",
+        executable="spawner.py",
+        arguments=["fts_broadcaster", "-c", "/controller_manager"],
+    )

Adding demo for Example 3 : Industrial Robots with integrated sensor #68

Adding demo for Example 3 : Industrial Robots with integrated sensor #68

Uh oh!

Conversation

bailaC commented Feb 18, 2021

Uh oh!

codecov-io commented Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

destogl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

destogl Feb 20, 2021

Choose a reason for hiding this comment

Uh oh!

v-lopez Feb 23, 2021

Choose a reason for hiding this comment

Uh oh!

destogl Feb 24, 2021

Choose a reason for hiding this comment

Uh oh!

bmagyar Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

destogl commented Feb 22, 2021

Uh oh!

Uh oh!

destogl left a comment

Choose a reason for hiding this comment

Uh oh!

destogl Jun 12, 2021

Choose a reason for hiding this comment

Uh oh!

destogl Jun 12, 2021

Choose a reason for hiding this comment

Uh oh!

destogl Jun 12, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-io commented Feb 19, 2021 •

edited

Loading