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

Enabled to export mesh as 3dxml format #154

Merged
merged 6 commits into from
Oct 17, 2024
Merged

Enabled to export mesh as 3dxml format #154

merged 6 commits into from
Oct 17, 2024

Conversation

KenMat765
Copy link
Contributor

This PR allows the URDF exporter to output mesh files not only in STL format but also in 3DXML format. This enables the export of models with color information.
This does not destroy the original functions.

This solves this issue.
#114

image

Exported in STL (no color)

grayscale

Exported in 3dxml (with color)

color

How to convert to .dae format for ros

pip3 install scikit-robot -U
convert-urdf-mesh <URDF_PATH> --output <OUTPUT_URDF_PATH>

Copy link
Collaborator

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Thank you so much for your PR. I think it would make a great addition to this tool, so my comments are mostly around future maintainability.

Can you please add:

  • A unit test for this functionality so that when I create a release I know it works appropriately.
  • An additional section in the README to explain converting meshes to DAE

Exporter.ExportRobot(exportSTL);

bool isSTL = radioButtonStl.Checked;
Exporter.ExportRobot(exportSTL, isSTL);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make the mesh format an enum instead of a bool? This will help make it easier to add support for more file formats in the future

string windowsMeshFileName = package.WindowsMeshesDirectory + linkName + ".STL";
string meshFilename = package.MeshesDirectory + linkName;
string windowsMeshFileName = package.WindowsMeshesDirectory + linkName;
if (isSTL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of an if/else, can you use a switch with the enum?

// Export STL
if (exportSTL)
{
SaveSTL(link, windowsMeshFileName);
if (isSTL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use switch

@KenMat765
Copy link
Contributor Author

I'm sorry for the late reply.

I've changed the bool to an enum of the export format.
I've also added a section in the README to explain converting meshes to DAE

Although, I had a problem with the unit test.

Problem

When I exported ORIGINAL_3_DOF_ARM, the links of the arm were shown in the wrong position in Rviz.
This might be related to these issues: #130, #116
I tried with the released code, but it still didn't work.

Temporary solution

When I created the same coordinates and axis of each link at the top-assembly, and selected those in the export menu instead of those in the sub-assemblies, the links were shown in the correct position.
This worked on both STL and 3DXML formats.

As discussed in #116, I believe this issue can occur with SolidWorks later than 2018, but is this supposed to function correctly originally?

wrong position in STL correct position in STL correct position in 3DXML
ORIGINAL_3_DOF_ARM_STL ORIGINAL_3_DOF_ARM_STL_TOPASSEM ORIGINAL_3_DOF_ARM_3DXML_TOPASSEM

Environment

Solidworks version: 2022
urdf_exporter: 1.6.1

@iory
Copy link

iory commented Oct 15, 2024

@brawner
I believe this PR could be highly beneficial for all SolidWorks and ROS users. I would appreciate it if you could kindly review it.

@brawner
Copy link
Collaborator

brawner commented Oct 15, 2024

@iory I believe this PR is still waiting for a unit test

@KenMat765
Copy link
Contributor Author

I have modified all the points you mentioned:

  • Converted bool to enum / if to switch
  • Added instructions for converting to .dae in the README
  • Added unit tests

I would greatly appreciate it if you could kindly review the changes.

Copy link
Collaborator

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Excellent, thank you for the contribution!

@brawner brawner merged commit 22cb778 into ros:master Oct 17, 2024
1 check passed
@vatanaksoytezer vatanaksoytezer mentioned this pull request Oct 22, 2024
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.

3 participants