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

As a ros2_control user I would like to define transmissions for my robots #32

Closed
bmagyar opened this issue Jan 17, 2020 · 9 comments
Closed

Comments

@bmagyar
Copy link
Member

bmagyar commented Jan 17, 2020

Design

Port over transmission_interface

@ros-controls ros-controls deleted a comment from suab321321 Jan 26, 2020
@ros-controls ros-controls deleted a comment from suab321321 Jan 26, 2020
@bmagyar bmagyar self-assigned this Feb 3, 2020
@ddengster
Copy link
Contributor

ddengster commented Feb 19, 2020

hi, what's the status of this port? Karsten's PR has a number of missing things like parsing in actuators, and it's not a 1:1 port with minimal changes from ros1.

@Karsten1987
Copy link
Contributor

@ddengster I should have marked this PR as draft. I am agreeing with @bmagyar review and most likely that transmission interface should have a new design in the first place. I just need something going for get started with the gazebo_ros_control part of it.

I think this issue is the right place to voice some concerns or design proposals for a ROS2 implementation.

@ddengster
Copy link
Contributor

No problem, @Karsten1987. I also happen to be working on a minimal gazebo_ros_control plugin for my own priorities.

With regards to the PR, I was expecting something closer to a 1-to-1 for the parser - doing a minimal working port first from ros_control, then applying breaking design changes later.

Also I'm not sure if we need a TransmissionParser object instantiation? I just need a function call to populate my TransmissionInfo data structures. But then again I'm not of the mindset that we need to 'objectify' things to get our code to do what it needs to do.

@bmagyar
Copy link
Member Author

bmagyar commented Feb 19, 2020

Over the years there were quite a few attempts at improving ros_control to improve extendability but the one that kept failing was joint and actuator interfaces. They are locked into place by the intertwined structure of transmission_interface and it solidified bad patterns in the entire framework. If you have specific requirements from transmissions please do say so :)

Also, I agree with removing classes when not needed. If a class contains no state but only a bunch of (hopefully static) functions, we have a better name for that: namespace.

@ddengster
Copy link
Contributor

I'm assuming you're referring to JointInfo and ActuatorInfo? It's members are mostly strings, which are imo the most extendable types you can have without bringing in a whole bunch of other abstractions to resolve parsed xml strings into another type. (And I'm not really a guy who likes bringing in more abstractions without a few existing use cases)

Perhaps I'll pick an example to narrow it down?

default_robot_hw_sim.cpp

There's plenty of transmissions[j].joints_[0].name_, is this the bad thing you're referring to?

Anyway, let me know and mabye we can trash it out.

@bmagyar bmagyar added this to To do in Transmission handling May 28, 2020
@ahcorde
Copy link
Contributor

ahcorde commented Jun 4, 2020

I saw that @bmagyar has merged transmission_interface in ros2_control/master. Is there any plan for the following data structures.

struct JointInfo
{
  std::string name_;
  std::vector<std::string> hardware_interfaces_;
  std::string role_;
  std::string xml_element_;
};

/**
 * \brief Contains semantic info about a given actuator loaded from XML (URDF)
 */
struct ActuatorInfo
{
  std::string name_;
  std::vector<std::string> hardware_interfaces_;
  std::string xml_element_;
};

@destogl
Copy link
Member

destogl commented Jun 9, 2020

Where do you need those informations?

@bmagyar
Copy link
Member Author

bmagyar commented Feb 15, 2021

We are missing transmission-loader and an xml format to close this.

@bmagyar
Copy link
Member Author

bmagyar commented Apr 22, 2022

Done in the recent ros2_control release, 2.6

@bmagyar bmagyar closed this as completed Apr 22, 2022
destogl pushed a commit to StoglRobotics-forks/ros2_control that referenced this issue Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants