Added Mk3 iCub hand and Mk2 iCub wrist parts. #203

ale-git · 2021-05-12T13:35:46Z

New parts not yet mounted on robots are now available.
A new iCub_components folder has been added.

The paths used inside the `generate_components_simmechanics` macro were pointing to the files, but the paths were not set with CMake and it therefore could not find the files. This has been fixed by adding the `set` and `configure_files` commands before the `add_custom_command`. Also, we included the copying of the meshes and configuration files for the left_hand_mk3. If/when the wrist is included, the copy commands for the wrist should be included as well.

Fixed an issue with the components macro paths

ale-git · 2021-05-12T13:41:09Z

@fiorisi

traversaro · 2021-05-12T17:10:32Z

simmechanics/data/hand_mk3/convertModel.sh

@@ -0,0 +1,2 @@
+simmechanics_to_urdf SIM_LEFT_HAND_Mk3.xml --yaml simmechanics2urdf_configfile.yaml --csv-joints simmechanics2urdf_joints_configfile.csv --output xml --outputfile left_hand_mk3.urdf


Is this use? The bot will use the CMake infrastructure to generate the models, so to avoid confusion it could make sense to remove this (see #201).

traversaro · 2021-05-12T17:12:03Z

simmechanics/data/wrist_mk2/simmechanics2urdf_joints_configfile.csv

+joint_name,lower_limit,upper_limit,damping
+l_wrist_yaw_joint,-90.0,90.0,1.0
+l_wrist_pitch_joint,-60.0,50.0,1.0
+l_wrist_roll_joint,-30.0,30.0,1.0


Can we document this limits somewhere? For the rest of iCub 3 we had this problem that we were not sure what was the reference for this limits, so it would be great if we could document them somewhere, to know where to check for getting the correct values.

traversaro · 2021-05-12T17:16:19Z

simmechanics/data/hand_mk3/simmechanics2urdf_configfile.yaml

+ SIM_L_HAND_PROX_MEDIUM_FINGER: l_hand_prox_medium_finger 
+ SIM_L_HAND_DISTAL_MEDIUM_FINGER: l_hand_distal_medium_finger 
+ SIM_L_HAND_PROX_LITTLE_FINGER: l_hand_prox_little_finger 
+ SIM_L_HAND_DISTAL_LITTLE_FINGER: l_hand_distal_little_finger 


Are these joint names documented somewhere? On iCub 2 we have exactly this problem that the joint names of the hand are not defined (see robotology/icub-models#30), it would be great if we could start documenting the joint names for the hand of iCub3, do avoid this problem (see for example https://icub-tech-iit.github.io/documentation/icub_kinematics/icub-joints/icub-joints/).

To given an example why this is important, it seems that the configuration files in https://github.com/robotology/robots-configuration/pull/253/files#diff-0fb41e3c08177cef454c0dc1e823aac1a46346e4d44c58a008dd2912f67131d0R12 use completely different joint names, or perhaps the relation between this two set of names is not clear.

cc @pattacini

traversaro · 2021-05-12T17:20:27Z

CMakeLists.txt

@@ -16,6 +16,7 @@ option(ICUB_MODEL_GENERATE_SIMMECHANICS "Generate models using the model generat
 option(BUILD_TESTING "Run tests for the generated models" ON)

 set(BUILD_PREFIX "iCub")
+set(COMPONENTS_BUILD_PREFIX "iCub_components")


There is any specific reason for not installing these in iCub as the rest of the models? If we install the models in a different directory, then we need to:

Update the docs of icub-models ( https://github.com/robotology/icub-models#by-installing-the-models )

Update the setup scripts of the robotology-superbuild (see https://github.com/robotology/robotology-superbuild/blob/545ec06c6915e0581dfaa2a09cc435f37ad5d614/conda/multisheller/icub-models_deactivate.msh, and all the files in https://github.com/robotology/robotology-superbuild/tree/master/cmake/template)

While this can be done, it complicates maintenance, so probably we need to understand if it is worth it.

traversaro · 2021-05-12T17:21:41Z

simmechanics/CMakeLists.txt

+                       COMMAND ${CMAKE_COMMAND} -E 
+                               copy "${CMAKE_CURRENT_SOURCE_DIR}/data/${GIVTWO_YAML_TEMPLATE}" "${GENERATED_YAML_LOCATION}")
+
+	add_custom_command(OUTPUT "${GENERATED_CSV_LOCATION}"


As a general comment, it would be great if we could avoid mixing tabs and spaces, as this induces visualization artifacts and misalignments.

traversaro · 2021-05-12T17:23:02Z

simmechanics/CMakeLists.txt

+                               copy "${GIVTWO_YARP_COMPONENT_NAME}.urdf" "${CMAKE_BINARY_DIR}/${COMPONENTS_BUILD_PREFIX}/parts/${GIVTWO_YARP_COMPONENT_NAME}/model.urdf")
+
+    list(APPEND model-simmechanics-generated-models "${CMAKE_BINARY_DIR}/${COMPONENTS_BUILD_PREFIX}/parts/${GIVTWO_YARP_COMPONENT_NAME}/model.urdf")
+endmacro()


Just to understand, what's the difference between this macro and generate_icub_simmechanics?

traversaro · 2021-05-19T09:12:31Z

Recap of open points for this PR (@pattacini), based on our discussion:

Make sure that the models are eventually installed by https://github.com/robotology/icub-models in the same <prefix>/share/iCub of other models, and that can be found as all other models. Based on our discussion that may be the case, but this needs to be verified. Comments: Added Mk3 iCub hand and Mk2 iCub wrist parts. #203 (comment)
Document the names of joints and "actuated axis" somewhere. Comments: Added Mk3 iCub hand and Mk2 iCub wrist parts. #203 (comment), Added Mk3 iCub hand and Mk2 iCub wrist parts. #203 (comment))

Minor points, nice to have but not blocking the PR:

Remove the convertModel https://github.com/robotology/icub-models-generator/pull/203/files/0e63c7958e825d81138b1088877f158667590d00#diff-22e4d21bb4087071d65ef59b3b040aeea76495353a1ca12bbdc4fad86b1713b3, as this script is not used by the generation procedure and may confused devs (see converModel.sh script for iCub3 generates a faulty urdf model #200, Delete convertModel.sh #201), comment: Added Mk3 iCub hand and Mk2 iCub wrist parts. #203 (comment)
Avoiding mixing spaces and tabs (comment: Added Mk3 iCub hand and Mk2 iCub wrist parts. #203 (comment))
Avoid duplicating macro (comment: Added Mk3 iCub hand and Mk2 iCub wrist parts. #203 (comment))

…lder

ale-git · 2021-06-30T13:19:04Z

I've fixed all the points except for the naming, since I had to talk with @fiorisi about that.

…rist_pitch, l_wrist_yaw

ale-git · 2021-06-30T13:36:03Z

About the wrist I've aligned the joint names with the standard iCub names: l_wrist_prosup, l_wrist_pitch, l_wrist_yaw

…where applicable).

ale-git · 2021-06-30T14:49:33Z

About the hand I have maintained the names already used with the 2.0 hand as much as possible, or modified them in a way that makes sense. The physical joint names are:

l_thumb_oppose_phys
l_thumb_rotate_phys
l_thumb_proximal_phys
l_thumb_distal_phys
l_hand_finger_phys
l_index_proximal_phys
l_index_distal_phys
l_middle_proximal_phys
l_middle_distal_phys
l_pinky_proximal_phys
l_pinky_distal_phys

The logical (coupled) joint names as seen from the outside are:

l_thumb_oppose
l_thumb_rotate // new, not present in 2.0
l_thumb // l_thumb_proximal + l_thumb_distal in 2.0
l_hand_finger // only index adduction/abduction in 3.0
l_index // l_index_proximal + l_index_distal in 2.0
l_middle // l_middle_proximal + l_index_distal in 2.0
l_pinky

…wiki/New-iCub-hand-study-notes#joint-naming-convention

ale-git · 2021-07-08T08:24:30Z

@fiorisi agreed with keeping the present iCub wrist names. Is there anything else to do before merging?

traversaro · 2021-07-08T11:38:13Z

@fiorisi agreed with keeping the present iCub wrist names. Is there anything else to do before merging?

Can we write these wrist names somewhere, even just in internal documentation? Thanks!

(Just to understand, of the non-blocking issues in #203 (comment) were address or not?)

pattacini · 2021-07-08T11:44:32Z

Can we write these wrist names somewhere, even just in internal documentation? Thanks!

@ale-git I'd suggest that you update our internal wiki with the new names (if it's not done already).
Later on, @fiorisi and his group could move it to our official iCub Tech doc website.

ale-git · 2021-07-08T11:45:00Z

(Just to understand, of the non-blocking issues in #203 (comment) were address or not?)

Yes, I can't check the boxes but I took care of all points.

traversaro · 2021-07-08T11:46:14Z

(Just to understand, of the non-blocking issues in #203 (comment) were address or not?)

Yes, I can't check the boxes but I took care of all points.

Perfect, thanks!

ale-git and others added 4 commits May 5, 2021 09:21

Added hand_mk3 and wrist_mk2 (work in progress)

85fdaa3

Merge pull request #1 from AlexAntn/patch-1

0b36965

Fixed an issue with the components macro paths

Added wrist model, fixed generation errors

0e63c79

ale-git marked this pull request as draft May 12, 2021 13:36

ale-git marked this pull request as ready for review May 12, 2021 13:36

traversaro reviewed May 12, 2021

View reviewed changes

traversaro mentioned this pull request May 19, 2021

Added hand Mk3 and wrist Mk2 iCub components. robotology/icub-models#88

Closed

ale-git added 4 commits June 10, 2021 17:57

Added left hand Mk3 model generation

ca7137c

added wrist generation files, components moved into new components fo…

c07bf47

…lder

fixed model.config generation problem

640bd7e

Generation bugfixes, now hand works with yarpmotorgui

25e4033

Wrist joint names aligned to iCub standard names: l_wrist_prosup, l_w…

bb882a1

…rist_pitch, l_wrist_yaw

Logical and physical joints renamed accordingly to iCub conventions (…

218c361

…where applicable).

Physical joint names aligned to https://github.com/icub-tech-iit/dev/…

b15afff

…wiki/New-iCub-hand-study-notes#joint-naming-convention

traversaro merged commit 872c9bd into robotology:master Jul 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Mk3 iCub hand and Mk2 iCub wrist parts. #203

Added Mk3 iCub hand and Mk2 iCub wrist parts. #203

ale-git commented May 12, 2021

ale-git commented May 12, 2021

traversaro May 12, 2021

traversaro May 12, 2021

traversaro May 12, 2021

traversaro May 12, 2021

traversaro May 13, 2021

traversaro May 12, 2021

traversaro May 12, 2021

traversaro May 12, 2021

traversaro commented May 19, 2021 •

edited

Loading

ale-git commented Jun 30, 2021

ale-git commented Jun 30, 2021

ale-git commented Jun 30, 2021

ale-git commented Jul 8, 2021

traversaro commented Jul 8, 2021

pattacini commented Jul 8, 2021 •

edited

Loading

ale-git commented Jul 8, 2021 •

edited

Loading

traversaro commented Jul 8, 2021

		@@ -0,0 +1,2 @@
		simmechanics_to_urdf SIM_LEFT_HAND_Mk3.xml --yaml simmechanics2urdf_configfile.yaml --csv-joints simmechanics2urdf_joints_configfile.csv --output xml --outputfile left_hand_mk3.urdf

Added Mk3 iCub hand and Mk2 iCub wrist parts. #203

Added Mk3 iCub hand and Mk2 iCub wrist parts. #203

Conversation

ale-git commented May 12, 2021

ale-git commented May 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

traversaro commented May 19, 2021 • edited Loading

ale-git commented Jun 30, 2021

ale-git commented Jun 30, 2021

ale-git commented Jun 30, 2021

ale-git commented Jul 8, 2021

traversaro commented Jul 8, 2021

pattacini commented Jul 8, 2021 • edited Loading

ale-git commented Jul 8, 2021 • edited Loading

traversaro commented Jul 8, 2021

traversaro commented May 19, 2021 •

edited

Loading

pattacini commented Jul 8, 2021 •

edited

Loading

ale-git commented Jul 8, 2021 •

edited

Loading