Skip to content
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

Create a demo that uses transmissions #226

Merged
merged 27 commits into from
Feb 15, 2023

Conversation

jordan-palacios
Copy link
Member

@jordan-palacios jordan-palacios commented Jan 24, 2023

Based on the base rrbot_system_position_only demo but with transmissions. The goal is to have a system with two simple transmissions that connect the joint1 with the actuator1 and the joint2 with the actuator2. joint1 has a reduction with value 2 and joint2 a reduction with value 4.

The behavior can be easily tracked in the log while the demo is running. Here is a sample:

[ros2_control_node-1] [INFO] [1674563263.738955303] [RRBotTransmissionsSystemPositionOnlyHardware]: Command data:
[ros2_control_node-1] 	joint1: 2.5 --> transmission1(R=2) --> actuator1: 5
[ros2_control_node-1] 	joint2: 5 --> transmission2(R=4) --> actuator2: 20
[ros2_control_node-1] [INFO] [1674563264.838728177] [RRBotTransmissionsSystemPositionOnlyHardware]: State data:
[ros2_control_node-1] 	joint1: 2.49911 <-- transmission1(R=2) <-- actuator1: 4.99822
[ros2_control_node-1] 	joint2: 4.99822 <-- transmission2(R=4) <-- actuator2: 19.9929

TODOs:

  • Document the demo in the README.

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see this example, I've got only some minor comments..

<?xml version="1.0"?>
<robot xmlns:xacro="http://www.ros.org/wiki/xacro">

<xacro:macro name="rrbot_transmissions_system_position_only" params="name prefix use_gazebo:=^|false use_fake_hardware:=^|true mock_sensor_commands:=^|false slowdown:=2.0">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can further simplify this by deleting use_gazebo, use_fake_hardware, mock_sensor_comands here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this breaks the launch, since we use rrbot_base.launch.py as base. Maybe something that could be addressed in #231.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it? removing them here and lines 34+35 in rrbot_transmissions_system_position_only.urdf.xacro works for me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the interest of merging this prior to the restructure, I'll open a general cleanup issue for xacro parameters

#240

@jordan-palacios
Copy link
Member Author

@bmagyar As far as I can tell, the failing checks in the CI are not related to the contents of this PR.

@bmagyar
Copy link
Member

bmagyar commented Feb 14, 2023

@christophfroehlich happy for me to merge this prior to the restructure PRs? May cause a little trouble there

@christophfroehlich
Copy link
Contributor

I'd suggest to merge this now in master, and then merge this into the restructuring branch to restructure that example.

bmagyar
bmagyar previously approved these changes Feb 15, 2023
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this, only left some small comments

<?xml version="1.0"?>
<robot xmlns:xacro="http://www.ros.org/wiki/xacro">

<xacro:macro name="rrbot_transmissions_system_position_only" params="name prefix use_gazebo:=^|false use_fake_hardware:=^|true mock_sensor_commands:=^|false slowdown:=2.0">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the interest of merging this prior to the restructure, I'll open a general cleanup issue for xacro parameters

#240

std::string name_;
double command_;
double state_;
double transmission_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is quite confusing. Could you rename this variable please to clarify it's meaning/content?
What does it mean in the joint and in the actuator context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the sink of the Joint/Actuator handle that lives inside the transmission. transmission_passthrough_ maybe?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you rename plus add this comment about the sink we are done ;)

Co-authored-by: Bence Magyar <[email protected]>
@bmagyar bmagyar merged commit 1007b1d into ros-controls:master Feb 15, 2023
@bmagyar
Copy link
Member

bmagyar commented Feb 15, 2023

@Mergifyio backport to new-structure-of-master

mergify bot pushed a commit that referenced this pull request Feb 15, 2023
* add copy for transmission demo and set in the urdf

* Renamed to rrbot_transmissions

* Changed reduction value of transmission2

* Added the rrbot_transmissions_system_position_only

Exact copy of rrbot_system_position_only for now. Using it
from the transmission demo

* Moved logger into its own variable

* Uncrustify

* Renamed hw interface to joint. Added actuator interfaces

* Formatting. Remove comments

* Using transmissions for joint and actuator interfaces

Added a helper object to manage the joint to transmissions to actuator
data handling

* Propagating data between all the interfaces

* Cleanup. Added some consts

* Simulate motor motion

* Added logging to periodically show interface data

* Refactored the parameters a bit. Removed unused ones

* Added try/catches where needed

Checking joint consistency in the description not required. Already done
by the parser

* Added transmission demo to README

* Use clang-format instead

* rename transmissions ros2_control macro

* Remove unnecessary actuator parameters

* Typo

* Removed gazebo, mock hardware and fake sensors from the transmission
demo

* Missing 'explicit' from constructor

* Further cleanup of gazebo, mock hardware and fake sensors from the transmission demo

* branding fix

* Remove pointer definitions

Co-authored-by: Bence Magyar <[email protected]>

* Fixed comment about reserve

Co-authored-by: Bence Magyar <[email protected]>

* Renamed member variable and added description

---------

Co-authored-by: Noel Jimenez <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
(cherry picked from commit 1007b1d)
@mergify
Copy link
Contributor

mergify bot commented Feb 15, 2023

backport to new-structure-of-master

✅ Backports have been created

bmagyar added a commit that referenced this pull request Feb 16, 2023
* Create a demo that uses transmissions (#226)

* add copy for transmission demo and set in the urdf

* Renamed to rrbot_transmissions

* Changed reduction value of transmission2

* Added the rrbot_transmissions_system_position_only

Exact copy of rrbot_system_position_only for now. Using it
from the transmission demo

* Moved logger into its own variable

* Uncrustify

* Renamed hw interface to joint. Added actuator interfaces

* Formatting. Remove comments

* Using transmissions for joint and actuator interfaces

Added a helper object to manage the joint to transmissions to actuator
data handling

* Propagating data between all the interfaces

* Cleanup. Added some consts

* Simulate motor motion

* Added logging to periodically show interface data

* Refactored the parameters a bit. Removed unused ones

* Added try/catches where needed

Checking joint consistency in the description not required. Already done
by the parser

* Added transmission demo to README

* Use clang-format instead

* rename transmissions ros2_control macro

* Remove unnecessary actuator parameters

* Typo

* Removed gazebo, mock hardware and fake sensors from the transmission
demo

* Missing 'explicit' from constructor

* Further cleanup of gazebo, mock hardware and fake sensors from the transmission demo

* branding fix

* Remove pointer definitions

Co-authored-by: Bence Magyar <[email protected]>

* Fixed comment about reserve

Co-authored-by: Bence Magyar <[email protected]>

* Renamed member variable and added description

---------

Co-authored-by: Noel Jimenez <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
(cherry picked from commit 1007b1d)

* Fix preformatted block

* fix header guard

---------

Co-authored-by: Jordan Palacios <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
destogl pushed a commit that referenced this pull request Mar 8, 2023
* Create a demo that uses transmissions (#226)

* add copy for transmission demo and set in the urdf

* Renamed to rrbot_transmissions

* Changed reduction value of transmission2

* Added the rrbot_transmissions_system_position_only

Exact copy of rrbot_system_position_only for now. Using it
from the transmission demo

* Moved logger into its own variable

* Uncrustify

* Renamed hw interface to joint. Added actuator interfaces

* Formatting. Remove comments

* Using transmissions for joint and actuator interfaces

Added a helper object to manage the joint to transmissions to actuator
data handling

* Propagating data between all the interfaces

* Cleanup. Added some consts

* Simulate motor motion

* Added logging to periodically show interface data

* Refactored the parameters a bit. Removed unused ones

* Added try/catches where needed

Checking joint consistency in the description not required. Already done
by the parser

* Added transmission demo to README

* Use clang-format instead

* rename transmissions ros2_control macro

* Remove unnecessary actuator parameters

* Typo

* Removed gazebo, mock hardware and fake sensors from the transmission
demo

* Missing 'explicit' from constructor

* Further cleanup of gazebo, mock hardware and fake sensors from the transmission demo

* branding fix

* Remove pointer definitions

Co-authored-by: Bence Magyar <[email protected]>

* Fixed comment about reserve

Co-authored-by: Bence Magyar <[email protected]>

* Renamed member variable and added description

---------

Co-authored-by: Noel Jimenez <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
(cherry picked from commit 1007b1d)

* Fix preformatted block

* fix header guard

---------

Co-authored-by: Jordan Palacios <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to apply transmission gear ratio (mechanical_reduction) in hardware_interface
4 participants