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

Robot: Report mesh loading issues #1629

Merged
merged 4 commits into from Jun 11, 2021

Conversation

rhaschke
Copy link
Contributor

This PR implements #1597 (comment) and fixes #1589.
@simonschmeisser, please have a look. I decided to summarize all mesh loading issues as part of the URDF field.
As you noticed in #1597, TF updates to the link status will overwrite previous errors and introducing a new status field, I didn't like so much.
rviz

@simonschmeisser
Copy link
Contributor

I haven't found the time to test it yet unfortunately. Generally I don't mind that much about how we present the errors. My version might be a bit simpler to parse for further use in our URDF editor but this one can also be regexed.

I wonder if that variadic function could be avoided by switching to QString right from the beginning, can open a PR on Monday evening

@rhaschke
Copy link
Contributor Author

Which variadic function you are referring to?

errors.erase(this);
}

void RobotLink::addError(const char* format, ...)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was referring to this variadic function that could be simplified by using QString or QStringList

loadMeshFromResource(model_name);
entity = scene_manager_->createEntity(ss.str(), model_name);
if (loadMeshFromResource(model_name).isNull())
addError("Could not load mesh resource '%s'", model_name.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
addError("Could not load mesh resource '%s'", model_name.c_str());
addError(QString("Could not load mesh resource '%1'").arg(model_name.c_str()));

or so, just a suggestion to keep things simple

@rhaschke rhaschke merged commit 0a26323 into ros-visualization:melodic-devel Jun 11, 2021
@simonschmeisser
Copy link
Contributor

Thanks for merging this! I was too busy with other stuff before my holidays unfortunately to open yet another PR

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