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

<transmission> tags for gazebo_ros_control #46

Merged
merged 5 commits into from
Jun 26, 2013
Merged

Conversation

davetcoleman
Copy link
Collaborator

I changed a lot here and I'm open to further modification/undoing some of it if necessary...

  • Default namespace for ros_control stuff is simply /MYROBOT
  • robot_description namespace currently not name spaced because of the effort based controller issue
  • Added a default controlPeriod of 0.001 if not specified
  • Removed <joint> tags in favor of <transmission><joint/><actuator/></tranmission> tags (see below)
  • robot_hw_sim_->initSim() no longer accepts a vector of joint names, but rather a vector of 'JointData' structs that also includes the associated hardware interface. This is to be expanded or maybe changed in the future.

Per the way the PR2 did it, and Adolpho's comments, I propose ros_control use something like this:

<transmission name="trans1" type="transmission_interface/SimpleTransmission">
  <joint name="joint1"/>
  <actuator name="motor1" type="EffortJointInterface" />
  <mechanicalReduction>1</mechanicalReduction>
</transmission>

For now I don't think we should specify a hardware interface for joints, that is too complicated for version one. See the README for more info on how gazebo_ros_control uses the transmission tags.

@jbohren
Copy link
Collaborator

jbohren commented Jun 26, 2013

Default namespace for ros_control stuff is simply /MYROBOT

Can we use the model name (SDF) or robot name (URDF) for the default namespace, instead?

robot_description namespace currently not name spaced because of the effort based controller issue

Which issue?

Added a default controlPeriod of 0.001 if not specified

Maybe instead, if it's not specified, it defaults to the gazebo update rate? We should also note that the update rate cannot be faster than the rate at which gazebo calls Update(). This could confuse people if they try to make the control period smaller.

Removed tags in favor of tags (see below)

Yeah, I like this direction.

robot_hw_sim_->initSim() no longer accepts a vector of joint names, but rather a vector of 'JointData' structs that also includes the associated hardware interface. This is to be expanded or maybe changed in the future.

JointData def here

Cool, that does sound more extensible. I assume you'll strongly type the members of JointData so that it contains actual transmission structures?

@jbohren
Copy link
Collaborator

jbohren commented Jun 26, 2013

w.r.t. default namespace, is there a reason why you changed this when you forked ros_control_gazebo?
https://github.com/osrf/gazebo_ros_pkgs/blob/hydro-devel/gazebo_ros_control/src/ros_control_plugin.cpp#L117

@davetcoleman
Copy link
Collaborator Author

I made it blank at the time because I was having a namespace issue that I was experimenting with. I already changed it back in this pull request, except I was thinking maybe it didn't need the ros_control prefix:
https://github.com/osrf/gazebo_ros_pkgs/blob/9421ed789e300df187607a17e3a875f41f64f098/gazebo_ros_control/src/ros_control_plugin.cpp#L121

@jbohren
Copy link
Collaborator

jbohren commented Jun 26, 2013

@davetcoleman sounds good w.r.t. default namespace

@davetcoleman
Copy link
Collaborator Author

I just saw your first comment above, response:

Can we use the model name (SDF) or robot name (URDF) for the default namespace, instead?

Is this what you mean?

Which issue?

ros-controls/ros_controllers#19 (comment) though now I see there is an easy fix of initString()

Maybe instead, if it's not specified, it defaults to the gazebo update rate?

Good idea, fixed 1cb8e85

I assume you'll strongly type the members of JointData so that it contains actual transmission structures?

Not sure what you mean - something like enums for the tranmission types? Feel free to modify this yourself. Also, the current solution might be short-sighted for when we want to actually implement transmissions.

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.

None yet

2 participants