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

Alternative for: Added variant model HC10DT-B10 #434

Merged

Conversation

gavanderhoorn
Copy link
Member

@gavanderhoorn gavanderhoorn commented Dec 3, 2021

This is an alternative PR for #415.

It keeps changes to existing variants separate from the addition of the B10 variant, and does not delete any existing files.

It also changes the collision geometry to use STL instead of Collada (for the same reasons as stated in #433.

Commit provenance has been retained where it made sense.

@gavanderhoorn
Copy link
Member Author

@EricMarcil: if you could check this and see whether it works for you, I suggest we continue the review here.

@gavanderhoorn
Copy link
Member Author

Some additional points for discussion:

  • this variant is called HC10DT IP67 (here), with two sub types B10 and B12. Should we follow that naming here?
  • the Collada meshes seem to have some strange colouring (but could be just how this is supposed to look)
  • this seems to have the same 'problem' with the Yaskawa logo/lettering as in GP4 Support Package #433
  • is the robot in the correct zero position?
    • compare HC10DT:

      hc10dt_zero

      and HC10DT B10:

      hc10dt_b10_zero

    • is the joint transform for joint_4_r correct? It seems to be the negative version of the HC 10 DT macro

    • joint_4_r has a positive transform here, but for both the base HC 10 and the DT variant it's negative

  • how similar are the three DT variants? Could they potentially share meshes?
  • is the joint_6_t-tool0 orientation correct? The other variants here have rpy="${pi} 0 0"

Additionally some comments:

  • please do not delete files which were there already. We gain little by removing them, and they are part of the standard "structure". We 'suffer' a little duplication, but gain a lot (as it's immediately clear which file is responsible for what, and for which variant). I've reverted most of the changes to existing files, and added new files for the B10 which were missing.
  • we still need the blue material on the visual elements, otherwise the collision meshes are red (which is the default colour). This does not interfere with the vertex colours in the Collada files. I've already done this.

@EricMarcil
Copy link
Contributor

EricMarcil commented Dec 7, 2021

this variant is called HC10DT IP67 (here), with two sub types B10 and B12. Should we follow that naming here?

Normally, the last digit is not significant from a parameter or mechanical point of view. In the controller configuration, the last digits is usually replaced by the "" character (HC10DT-B1) but that wouldn't work for the file naming. So I think we can just make reference to the -B10 and if someone ask for a variation B12 variation, we can point them to this one.

the Collada meshes seem to have some strange colouring (but could be just how this is supposed to look)

Those are the colors that came in with the model. There are not quite as bright. I'll check with the GP4 colors to see if they are different.

this seems to have the same 'problem' with the Yaskawa logo/lettering as in GP4 Support Package #433

Yes, I've figured out how to fix this but forgot that it was also on this PR. I'll update it shortly.

is the robot in the correct zero position?

  • is the joint transform for joint_4_r correct? It seems to be the negative version of the HC 10 DT macro
  • joint_4_r has a positive transform here, but for both the base HC 10 and the DT variant it's negative

Yes, the home position has changed between the HC10-DT and HC10-DT-B10. The changes on the joint_4_r is a result of this change. The transform is going from Z=-0.500 (arm pointing down) to X=0.500 (arm being forward)

how similar are the three DT variants? Could they potentially share meshes?

The HC10 and HC10DT already share the same meshes except for the link_6_t. The HC10DT-B1* is a complete redesign, so all the components are different. Between the HC10DT-B1* variant, there are physically all the same.

is the joint_6_t-tool0 orientation correct? The other variants here have rpy="${pi} 0 0"

Yes, that is correct. It is again a result of the arm pointing forward instead of down.

Comments noted.

@EricMarcil
Copy link
Contributor

@gavanderhoorn I suggest that we add the flange link to the HC10DT-B10 since it hasn't been merged to the main repo yet. What do you think?

@gavanderhoorn
Copy link
Member Author

@gavanderhoorn I suggest that we add the flange link to the HC10DT-B10 since it hasn't been merged to the main repo yet. What do you think?

yes, that would make sense to me.

Added 'flange' link to model
@EricMarcil
Copy link
Contributor

@gavanderhoorn I've pushed an update adding the flange link to the model. I think with that we should be good to merge.

EricMarcil
EricMarcil previously approved these changes Jan 14, 2022
Copy link
Contributor

@EricMarcil EricMarcil left a comment

Choose a reason for hiding this comment

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

I think everything is good to merge.

@EricMarcil
Copy link
Contributor

@gavanderhoorn Are you OK with merging this?

@gavanderhoorn
Copy link
Member Author

gavanderhoorn commented Feb 2, 2022

I was actually just looking at this.

For some reason, the model renders like this for me right now:

rviz_screenshot_2022_02_02-14_34_33

as only link_2 and link_4 render in this strange way, I'm trying to figure out whether dae5eef did something to the normals of the meshes, or changed it in some other way.


Edit: as soon as I update RViz (from what's in osrf/ros:melodic-desktop), rendering of those meshes changes. Same things with the GP4 model.

I'm wondering whether the export/conversion method you're using results in meshes which RViz doesn't like somehow.

@gavanderhoorn
Copy link
Member Author

gavanderhoorn commented Feb 2, 2022

Looks like the rendering issue is known. See ros-visualization/rviz#1702. Apparently this has already been fixed (ros-visualization/rviz#1703).


Edit: but not released yet.

I'm going to ignore the rendering issue.

flange == attachment point for EEF (sub)models, and as such should follow REP-103 (ie: X+ out of flange, in "forward" direction (link-local)).

tool0 == an "all-zeros toolframe", corresponding to the OEMs definition. For Yaskawa, this has Z+ forward.
@gavanderhoorn
Copy link
Member Author

If you could review again @EricMarcil ?

We'll wait for green CI and then merge.

@EricMarcil
Copy link
Contributor

@gavanderhoorn Looks good. Thanks for the help on this one.

@gavanderhoorn
Copy link
Member Author

CI is very slow.

@gavanderhoorn gavanderhoorn merged commit 972f9ec into ros-industrial:kinetic-devel Feb 2, 2022
@gavanderhoorn gavanderhoorn deleted the hc10dt_b10_wip branch February 2, 2022 16:11
@gavanderhoorn
Copy link
Member Author

Thanks @EricMarcil and apologies for the delay merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants