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

Add iCub 2.5+ models #83

Merged
merged 14 commits into from
Jan 5, 2018
Merged
26 changes: 14 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,17 @@ Both generation pipelines are still in `a work in progress` state, and several i

## Generated models

| `YARP_ROBOT_NAME` | Pipeline | Notes |
|:-----------------:|:------------:|:---------------------:|
| `iCubDarmstadt01` | simmechanics | v2.5 without backpack |
| `iCubGazeboV2_5` | simmechanics | v2.5 with backpack, and inertias of some links modified to run smoothly in Gazebo |
| `iCubGenova01` | simmechanics | v2.5 without backpack |
| `iCubGenova02` | simmechanics | v2.5 with backpack |
| `iCubGenova03` | dh | v2 with legs v1 and feet v2.5 |
| `iCubGenova04` | simmechanics | v2.5 with backpack |
| `iCubLisboa01` | dh | v1 with head v2 |
| `iCubNancy01` | dh | v2.5 with arms v1 and head v2|
| `iCubParis01` | dh | v1 with feet v2.5 |
| `iCubParis02` | dh | v2 with feet v2.5 |
| `YARP_ROBOT_NAME` | Pipeline | Notes |
|:--------------------:|:------------:|:-------------------------------:|
| `iCubDarmstadt01` | simmechanics | v2.5 without backpack |
| `iCubGazeboV2_5` | simmechanics | v2.5 with backpack, and inertias of some links modified to run smoothly in Gazebo |
| `iCubGazeboV2_5_plus`| simmechanics | v2.5+ with backpack, and inertias of some links modified to run smoothly in Gazebo |
| `iCubGenova01` | simmechanics | v2.5 without backpack |
| `iCubGenova02` | simmechanics | v2.5 with backpack |
| `iCubGenova02_plus` | simmechanics | v2.5+ with backpack |
Copy link
Member

Choose a reason for hiding this comment

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

Given that "plus" variant has been implemented only on iCubGenova04, would it make sense to have iCubGenova04_plus instead of iCubGenova02_plus?

Copy link
Member Author

@fiorisi fiorisi Jan 5, 2018

Choose a reason for hiding this comment

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

Corrected. Thanks!

| `iCubGenova03` | dh | v2 with legs v1 and feet v2.5 |
| `iCubGenova04` | simmechanics | v2.5 with backpack |
| `iCubLisboa01` | dh | v1 with head v2 |
| `iCubNancy01` | dh | v2.5 with arms v1 and head v2 |
| `iCubParis01` | dh | v1 with feet v2.5 |
| `iCubParis02` | dh | v2 with feet v2.5 |
111 changes: 93 additions & 18 deletions simmechanics/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ find_package(PythonInterp REQUIRED)
# Generate URDF models for
# v2.5 robots using the simmechanics-to-urdf script
macro(generate_icub_simmechanics)
set(options NO_BACKPACK BOGUS INCREASE_INERTIA_FOR_GAZEBO)
set(oneValueArgs YARP_ROBOT_NAME SIMMECHANICS_XML YAML_TEMPLATE CSV)
set(options NO_BACKPACK BOGUS INCREASE_INERTIA_FOR_GAZEBO ICUB_PLUS)
set(oneValueArgs YARP_ROBOT_NAME SIMMECHANICS_XML YAML_TEMPLATE CSV_TEMPLATE)
set(multiValueArgs)
cmake_parse_arguments(GIVTWO "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})

Expand Down Expand Up @@ -93,28 +93,96 @@ macro(generate_icub_simmechanics)
else()
set(GAZEBO_ASSIGNED_INERTIAS "")
endif()

# If we are generating the icub2.5+ model we need to add the xsens IMU, change the mesh folder
# and change the rotation axis list that need to be reversed.
if(GIVTWO_ICUB_PLUS)
set(MESH_FOLDER "filenameformatchangeext: \"package://iCub/meshes/simmechanics/2-5_plus/%s-binary.stl\"")
set(ANKLE_PITCH_ROM "-50,20")
set(CUSTOM_EPSILON "epsilon: 2e-7")
set(XSENS_IMU_FRAME
" - frameName: SCSYS_ROOT_LINK_XSENS_IMU
frameReferenceLink: root_link
exportedFrameName: root_link_imu_frame
")
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we now have 2 IMUs, could we rename the previous "imu_frame" in the head to "head_imu_frame"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we can do it. I am not sure if then we need to update also some other configuration files (e.g. wbd). If you think that is easy and quick we can change the name of the head imu as you suggest.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there a small impact in iDynTree, quite easy to implement, I let @traversaro confirm...

Copy link
Member

@traversaro traversaro Jan 5, 2018

Choose a reason for hiding this comment

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

We would need to either have different wbd configuration files for iCub*** and iCub***_plus, or update the frame name for all the models. To be honest, I am a bit afraid about this small changes that may have unexpected consequences, so I would open a different PR for changing the imu_frame frame.

Copy link
Member

@diegoferigo diegoferigo Jan 5, 2018

Choose a reason for hiding this comment

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

This would also affect all our controllers that should be updated accordingly @gabrielenava
@S-Dafarra

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @diegoferigo, as suggested by @traversaro we don't change the imu name in this PR. We can open a new one.

set(XSENS_IMU_SENSOR
" - frameName: SCSYS_ROOT_LINK_XSENS_IMU
linkName: root_link
exportFrameInURDF: No
sensorName: root_link_xsens_imu
sensorType: \"imu\"
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, while the additional frame (@XSENS_IMU_FRAME@) exported for the IMU is required for the dynamic computations in iDynTree, the IMU itself is not considered as a URDF-like "sensor" as per the description in https://github.com/robotology/idyntree/blob/master/doc/model_loading.md and doesn't require an entry in the "sensor" tag section. The following entry...

  - frameName: SCSYS_HEAD_MTX_IMU
    linkName: head
    sensorName: head_imu_acc_1x1
    sensorType: "accelerometer"
    ...

was already there and was intended to provide information on the accelerometers embedded in the head IMU. those information were used for the sensor measurement prediction computation in iDynTree.

This said, we should have for the root_link IMU:

    sensorName: root_link_imu_acc
    sensorType: "accelerometer"

Copy link
Member

Choose a reason for hiding this comment

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

I agree with nuno on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

updateRate: \"400\"
sensorBlobs:
- |
<plugin name=\"root_link_xsens_imu_plugin\" filename=\"libgazebo_yarp_imu.so\">
<yarpConfigurationFile>model://iCub/conf/gazebo_icub_inertial.ini</yarpConfigurationFile>
Copy link
Member Author

Choose a reason for hiding this comment

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

@nunoguedelha do we need also an additional configuration file for the new IMU?
E.g. gazebo_icub_xsens_inertial.ini

Copy link
Member

Choose a reason for hiding this comment

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

Yes, good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, considering that the parameters will be different:

  • period: 10ms -> 2.5ms (because of the rate change to 400Hz)
  • different yarp port name

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, updated!

</plugin>
")
set(REVERSE_ROTATION_AXIS
"reverseRotationAxis:
l_shoulder_roll
l_elbow
l_hip_yaw
l_knee
l_ankle_pitch
l_hip_pitch
r_ankle_roll
r_shoulder_pitch
r_shoulder_yaw
r_elbow
r_wrist_prosup
torso_pitch
neck_roll
")
else()
set(MESH_FOLDER "filenameformatchangeext: \"package://iCub/meshes/simmechanics/%s-binary.stl\"")
set(ANKLE_PITCH_ROM "-35,35")
set(CUSTOM_EPSILON "")
set(XSENS_IMU_FRAME "")
set(XSENS_IMU_SENSOR "")
set(REVERSE_ROTATION_AXIS
"reverseRotationAxis:
l_shoulder_roll
l_elbow
l_hip_yaw
l_knee
l_ankle_pitch
r_hip_pitch
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is r_hip_pitch inverted instead of l_hip_pitch?

Copy link
Member

Choose a reason for hiding this comment

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

The axis that are inverted are magically changing whenever we change the Creo model. 😕

Copy link
Member Author

Choose a reason for hiding this comment

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

This comes from the CAD model. In theory, it is possible to control the orientation of the axis but is not practical. It is easier to perform the change in the YAML file. Probably, since the upper body is the same for the iCub2.5 and iCub2.5+ we can change only the leg axis.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok.

r_ankle_roll
r_shoulder_pitch
r_shoulder_yaw
r_elbow
r_wrist_prosup
torso_pitch
neck_roll
")
endif()

set(GENERATED_YAML_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/${GIVTWO_YARP_ROBOT_NAME}.yaml)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_YAML_TEMPLATE}
${GENERATED_YAML_LOCATION}
@ONLY)
set(GENERATED_CSV_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/${GIVTWO_YARP_ROBOT_NAME}.csv)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_CSV_TEMPLATE}
${GENERATED_CSV_LOCATION}
@ONLY)

add_custom_command(OUTPUT ${GIVTWO_YARP_ROBOT_NAME}.urdf
COMMAND simmechanics_to_urdf
ARGS ${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_SIMMECHANICS_XML}
--output xml
--yaml ${GENERATED_YAML_LOCATION}
--csv-joints ${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_CSV}
--csv-joints ${GENERATED_CSV_LOCATION}
--outputfile ${GIVTWO_YARP_ROBOT_NAME}.urdf
MAIN_DEPENDENCY "${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_SIMMECHANICS_XML}"
DEPENDS "${GENERATED_YAML_LOCATION}"
"${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_CSV}")
"${GENERATED_CSV_LOCATION}")



# If we are generating a model without backpack, we need to tweack the COM of the chest link,
# and we have a custon python script for this
# If we are generating a model without backpack, we need to tweak the COM of the chest link,
# and we have a custom python script for this
if(${GIVTWO_NO_BACKPACK})
# instead of just copyng, we also modify the com on the fly
# instead of just copying, we also modify the com on the fly
add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/${BUILD_PREFIX}/robots/${GIVTWO_YARP_ROBOT_NAME}/model.urdf"
MAIN_DEPENDENCY "${GIVTWO_YARP_ROBOT_NAME}.urdf"
COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/data/icub2_5/adjust_com_for_model_without_backpack.py
Expand All @@ -134,29 +202,41 @@ set(model-simmechanics-generated-models "")
generate_icub_simmechanics(YARP_ROBOT_NAME iCubGazeboV2_5
SIMMECHANICS_XML "icub2_5/ICUB_2-5_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV "icub2_5/ICUB_2-5_BB_joint_parameters.csv"
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in"
INCREASE_INERTIA_FOR_GAZEBO)

generate_icub_simmechanics(YARP_ROBOT_NAME iCubGazeboV2_5_plus
SIMMECHANICS_XML "icub2_5/ICUB_2-5_plus_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in"
INCREASE_INERTIA_FOR_GAZEBO ICUB_PLUS)

generate_icub_simmechanics(YARP_ROBOT_NAME iCubGenova02_plus
SIMMECHANICS_XML "icub2_5/ICUB_2-5_plus_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in"
ICUB_PLUS)

generate_icub_simmechanics(YARP_ROBOT_NAME iCubGenova02
SIMMECHANICS_XML "icub2_5/ICUB_2-5_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV "icub2_5/ICUB_2-5_BB_joint_parameters.csv")
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in")

generate_icub_simmechanics(YARP_ROBOT_NAME iCubGenova04
SIMMECHANICS_XML "icub2_5/ICUB_2-5_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV "icub2_5/ICUB_2-5_BB_joint_parameters.csv")
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in")

generate_icub_simmechanics(YARP_ROBOT_NAME iCubDarmstadt01
SIMMECHANICS_XML "icub2_5/ICUB_2-5_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV "icub2_5/ICUB_2-5_BB_joint_parameters.csv"
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in"
NO_BACKPACK)

generate_icub_simmechanics(YARP_ROBOT_NAME iCubGenova01
SIMMECHANICS_XML "icub2_5/ICUB_2-5_BB_SIM_MODEL.xml"
YAML_TEMPLATE "icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in"
CSV "icub2_5/ICUB_2-5_BB_joint_parameters.csv"
CSV_TEMPLATE "icub2_5/ICUB_2-5_BB_joint_parameters.csv.in"
NO_BACKPACK)

add_custom_target(generate-models-simmechanics
Expand All @@ -169,8 +249,3 @@ add_custom_command(TARGET generate-models-simmechanics
COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_CURRENT_SOURCE_DIR}/data/icub2_5/meshes" "${CMAKE_BINARY_DIR}/${BUILD_PREFIX}/meshes"
COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_CURRENT_SOURCE_DIR}/data/icub2_5/conf" "${CMAKE_BINARY_DIR}/${BUILD_PREFIX}/conf"
COMMENT "Copying Simmechanics meshes")





Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ r_hip_pitch,-45,134,0.223,0,5.1,55.5
r_hip_roll,-20,120,0.223,0,7.64,37
r_hip_yaw,-80,80,0.223,0,7.64,37
r_knee,-124,4,0.223,0,7.64,37
r_ankle_pitch,-35,35,0.223,0,7.64,37
r_ankle_pitch,@ANKLE_PITCH_ROM@,0.223,0,7.64,37
r_ankle_roll,-25,25,0.223,0,7.64,37
l_hip_pitch,-45,134,0.223,0,5.1,55.5
l_hip_roll,-20,120,0.223,0,7.64,37
l_hip_yaw,-80,80,0.223,0,7.64,37
l_knee,-124,4,0.223,0,7.64,37
l_ankle_pitch,-35,35,0.223,0,7.64,37
l_ankle_pitch,@ANKLE_PITCH_ROM@,0.223,0,7.64,37
l_ankle_roll,-25,25,0.223,0,7.64,37
r_shoulder_pitch,-95.5,10,0.06,0,,
r_shoulder_roll,0,160.8,0.06,0,,
Expand Down
23 changes: 7 additions & 16 deletions simmechanics/data/icub2_5/ICUB_2-5_BB_simmechanics_options.yaml.in
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ originRPY: [0.0,0.0,3.14]
# Meshes options
scale: "0.001 0.001 0.001"
forcelowercase: Yes
filenameformatchangeext: "package://iCub/meshes/simmechanics/%s-binary.stl"
@MESH_FOLDER@
Copy link
Contributor

Choose a reason for hiding this comment

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

I would actually call this @MESH_FILE_FORMAT@ instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, better!


# Custom epsilon for testing whether a number is close to zero
@CUSTOM_EPSILON@

# Rename options (map Creo names to URDF names)
rename:
Expand Down Expand Up @@ -393,7 +396,7 @@ exportedFrames:
- frameName: SCSYS_R_LOWER_LEG_SKIN_21
frameReferenceLink: r_lower_leg
exportedFrameName: r_lower_leg_skin_21

@XSENS_IMU_FRAME@


linkFrames:
Expand Down Expand Up @@ -768,23 +771,11 @@ sensors:
linkName: l_foot
sensorName: l_foot_mtb_acc_10b13
sensorType: "accelerometer"
@XSENS_IMU_SENSOR@


# Workaround options
reverseRotationAxis:
l_shoulder_roll
l_elbow
l_hip_yaw
l_knee
l_ankle_pitch
r_hip_pitch
r_ankle_roll
r_shoulder_pitch
r_shoulder_yaw
r_elbow
r_wrist_prosup
torso_pitch
neck_roll
@REVERSE_ROTATION_AXIS@


assignedMasses:
Expand Down
Loading