Add package with "Fake" components for simple integration of framework by destogl · Pull Request #323 · ros-controls/ros2_control

destogl · 2021-02-05T15:56:02Z

This package implements fake components (currently only System/Robot) which simply mirror the incoming commands to its states. It is basically a very simple simulation without a need for Gazebo. It also provides much more flexibility for integration tests than Gazebo. The use-cases are the following:

Integration/setting up of ros2_control framework for a new robot. It enables testing of ros2_control URDF, controllers' setup, and external and internal interfaces without implementing communication to the hardware.
MoveIt uses it for testing of their setup by the users. In ROS1 they were using fake_joint implementation. (@JafarAbdi)
By extending the classes from this package users can create simple simulations when they cannot or don't want to use Gazebo.

Why I propose to merge this here and not in the ros2_control_demos repository? Because this package should be released at some point and the demos repository is meant to be used always from the source for users to see the implementation examples. Also, this is not a demonstration functionality, but rather a tool for setting up and testing the ros2_control framework.

olivier-stasse · 2021-02-05T17:49:19Z

3. By extending the classes from this package users can create simple simulations when they cannot or don't want to use Gazebo.

Does that mean that we can have Gazebo examples in ros2_control_demos ?

v-lopez

In general LGTM.

I do not fully understand the need to separate the standard and other interfaces.

fake_components/include/fake_components/generic_system.hpp

hardware_interface/test/test_resource_manager.cpp

destogl · 2021-02-08T21:41:52Z

I do not fully understand the need to separate the standard and other interfaces.

Thanks for a fast review!

It is rather a convenience feature. So if one wants to extend the class and create some simple simulation for it, the interfaces relevant for the dynamic are already there, so they do not need to filter them.

The second reason is the JointStateController which is unhappy with nan position data. So I am initializing all dynamic-related data to 0 if no initial value is provided.

destogl · 2021-02-08T21:43:32Z

Does that mean that we can have Gazebo examples in ros2_control_demos ?

No, not yet, but I see you are on it :)

AndyZe · 2021-02-10T17:19:38Z

Linking to the first MoveIt PR that has tried to use this.

Notice that lines of code went up by 120 compared to fake_joint. That's not a good sign, in terms of being user-friendly and easy to configure.

AndyZe · 2021-02-10T17:24:52Z

I guess the difference is, fake_joint assumes position control for every joint.

I just want to make sure we aren't making things more complicated than they need to be.

destogl · 2021-02-11T15:52:10Z

I guess the difference is, fake_joint assumes position control for every joint.

I just want to make sure we aren't making things more complicated than they need to be.

This implementation provides a general solution where FakeSystem is adapting itself to the interfaces described with URDF. So it doesn't matter anymore which are used by your robot. In combination with at general JointTrajectoryController this works for really any set or input and output interfaces. (At least it should).

bmagyar · 2021-02-16T07:52:07Z

Notice that lines of code went up by 120 compared to fake_joint. That's not a good sign, in terms of being user-friendly and easy to configure.

I also don't like those lengthy launch files but what can we do about it? It feels that a lot of ROS2 is just dumping all the complexity on the end user in the name of freedom. Not that we should be doing this but we can't help launch files and param handling being inconvenient. Additionally, consider that if this can completely replace fake_joint, you can deduct the LoC of that too.

bmagyar

@destogl I think in general the approach is good but we shouldn't create a new package for it. I think it would fit well in hardware_interface with the fake_component namespace you are already using, it's sole purpose after all is to fake some of that away.

JafarAbdi · 2021-02-16T13:51:40Z

fake_components/src/generic_system.cpp

+PLUGINLIB_EXPORT_CLASS(fake_components::GenericSystem, hardware_interface::SystemInterface)
+PLUGINLIB_EXPORT_CLASS(fake_components::GenericRobot, hardware_interface::SystemInterface)


Out of curiosity, any reason for exporting two different names for the same library .?

Only convenience. For the most users in the most cases: System == Robot.

@bmagyar Are you also OK with this? We wanted to add this throughout the framework

That seems confusing to me. Why exactly?

Let's discuss this at the next WG meeting. If we want to revise nomenclature, that's fine, could do that for Galactic but I'd prefer to keep an official nomenclature without convenience duplications.

destogl · 2021-02-16T14:29:34Z

@destogl I think in general the approach is good but we shouldn't create a new package for it. I think it would fit well in hardware_interface with the fake_component namespace you are already using, it's sole purpose after all is to fake some of that away.

I will move this then...

Co-authored-by: Victor Lopez <3469405+v-lopez@users.noreply.github.com>

hardware_interface/test/fake_components/test_generic_system.cpp

bmagyar

I agree with @AndyZe , dual names are confusing. I suggest to remove the typedef and the duplicate pluginlib export. If we are already renaming things for convenience, let's think about renaming it all for Galactic...

ros2_control/package.xml

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

… add-fake-robot

ros-controls#323) * Generic components FakeSystem for simple testing of ros2_control framework. * Correct typo. Co-authored-by: Victor Lopez <3469405+v-lopez@users.noreply.github.com> * Update ros2_control/package.xml Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com> * Remove dual names Co-authored-by: JafarAbdi <cafer.abdi@gmail.com> Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

destogl requested review from Karsten1987, bmagyar and v-lopez February 5, 2021 15:56

destogl self-assigned this Feb 5, 2021

destogl added the enhancement label Feb 5, 2021

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

Non-Joint command interfaces #318

Closed

destogl mentioned this pull request Feb 5, 2021

Non-Joint command interfaces #318

Closed

destogl force-pushed the add-fake-robot branch from 1fcf3bd to 2d3848a Compare February 5, 2021 23:57

v-lopez reviewed Feb 6, 2021

View reviewed changes

fake_components/include/fake_components/generic_system.hpp Outdated Show resolved Hide resolved

hardware_interface/test/test_resource_manager.cpp Outdated Show resolved Hide resolved

JafarAbdi mentioned this pull request Feb 9, 2021

Use fake_components::GenericSystem from ros2_control moveit/moveit2#361

Merged

1 task

bmagyar reviewed Feb 16, 2021

View reviewed changes

JafarAbdi reviewed Feb 16, 2021

View reviewed changes

destogl and others added 10 commits February 18, 2021 15:51

Created sub-package fake_generic_robot.

e2e35fb

Added files with licence header to push directories.

cee9e81

Correct copyright

ce95265

Rename package to Fake Components.

2a53ddf

Add FakeSystem impl

154a430

Use syntax usual for ros2_control. Add first test for FakeSystem.

f61e10d

Correct some typos in other repositories and add support for FakeRobot.

16b2d5f

Added further tests

ff613e7

GenericSystem joint support for standard interfaces

9617089

Added support for other, non-standard interfaces

0abdde3

destogl and others added 9 commits February 18, 2021 15:51

FakeSystem has support for integrated sensor also

69f070d

Moved example URDFs for parser/scenario tests to assets package

240332e

Unify urdf_header and urdf_tail test constants

a752c34

Correct tests of fake system

8477de6

Use 0 as default values for standard interface types.

99df004

Correct fake components package name in metapackage

9b3a2c4

Fake components moved to hardware interface.

0f5a1c6

Use gmock instead of gtest

492aff1

Correct typo.

61dc9a2

Co-authored-by: Victor Lopez <3469405+v-lopez@users.noreply.github.com>

destogl force-pushed the add-fake-robot branch from d021d7c to 61dc9a2 Compare February 21, 2021 19:54

bmagyar reviewed Feb 22, 2021

View reviewed changes

hardware_interface/test/fake_components/test_generic_system.cpp Outdated Show resolved Hide resolved

bmagyar requested changes Feb 22, 2021

View reviewed changes

ros2_control/package.xml Outdated Show resolved Hide resolved

destogl and others added 3 commits February 22, 2021 12:23

Update ros2_control/package.xml

2a369bb

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

Remove dual names

eb79c8c

Merge branch 'add-fake-robot' of github.com:destogl/ros2_control into…

6a93d0d

… add-fake-robot

bmagyar approved these changes Feb 22, 2021

View reviewed changes

destogl merged commit 93b1578 into ros-controls:master Feb 22, 2021

destogl deleted the add-fake-robot branch February 22, 2021 12:16

destogl pushed a commit to b-robotized-forks/ros2_control that referenced this pull request Aug 11, 2022

[JTC] Variable renaming for clearer API (ros-controls#323)

98eef77

		PLUGINLIB_EXPORT_CLASS(fake_components::GenericSystem, hardware_interface::SystemInterface)
		PLUGINLIB_EXPORT_CLASS(fake_components::GenericRobot, hardware_interface::SystemInterface)

Conversation

destogl commented Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olivier-stasse commented Feb 5, 2021

Uh oh!

v-lopez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

destogl commented Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

destogl commented Feb 8, 2021

Uh oh!

AndyZe commented Feb 10, 2021

Uh oh!

AndyZe commented Feb 10, 2021

Uh oh!

destogl commented Feb 11, 2021

Uh oh!

bmagyar commented Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmagyar left a comment

Choose a reason for hiding this comment

Uh oh!

JafarAbdi Feb 16, 2021

Choose a reason for hiding this comment

Uh oh!

destogl Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndyZe Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

bmagyar Feb 22, 2021

Choose a reason for hiding this comment

Uh oh!

destogl commented Feb 16, 2021

Uh oh!

Uh oh!

bmagyar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

destogl commented Feb 5, 2021 •

edited

Loading

destogl commented Feb 8, 2021 •

edited

Loading

bmagyar commented Feb 16, 2021 •

edited

Loading

destogl Feb 16, 2021 •

edited

Loading