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

Update VHACD to latest (v4.1, tag 454913f) #896

Merged
merged 7 commits into from
Jun 8, 2023

Conversation

rjoomen
Copy link
Contributor

@rjoomen rjoomen commented May 30, 2023

This updates the outdated VHACD library to the latest version. It also removes OMP and OpenCL dependencies, as VHACD now works without these.

@rjoomen
Copy link
Contributor Author

rjoomen commented May 31, 2023

@Levi-Armstrong I'm not sure how to satisfy clang-tidy. Using NOLINTBEGIN/NOLINTEND is only supported from clang-format 14 onwards, and TESSERACT_COMMON_IGNORE_WARNINGS_PUSH/TESSERACT_COMMON_IGNORE_WARNINGS_POP doesn't work for analyzer checks. Any ideas?

@Levi-Armstrong
Copy link
Contributor

Clang tidy only provides a way to NOLINT individual lines, so there is three options. I recommend option 2, but if it is taking to long then I would be okay with option 3 and create an issue to fix it later since this is a utility.

  1. The first is add the // NOLINT to each issue.
  2. The second is fix the issue, which is usually the same amount of work as adding the // NOLINT to each line.
  3. Comment out the target_clang_tidy in the CMakeLists.txt for this target.

@rjoomen rjoomen force-pushed the master branch 2 times, most recently from c053455 to 0542bca Compare June 1, 2023 09:47
@rjoomen
Copy link
Contributor Author

rjoomen commented Jun 1, 2023

Thanks for the suggestions, I tried option:

  1. A line filter to exclude VHACD.h

But unfortunately, it doesn't seem to work, even though I accidentally found an issue by @Levi-Armstrong mentioning exactly this. Do you know why this is?

Otherwise, I'll go for option 3, as with 654 warnings options 1&2 are a lot of work.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jun 7, 2023

@Levi-Armstrong friendly ping.

@Levi-Armstrong
Copy link
Contributor

I am not sure why it is not working, but I would just comment out the target_clang_tidy line in the CMakeLists.txt file.

@Levi-Armstrong
Copy link
Contributor

I will merge after the pipeline finishes.

@Levi-Armstrong
Copy link
Contributor

Thanks for making the update.

@Levi-Armstrong Levi-Armstrong merged commit 6afa6f9 into tesseract-robotics:master Jun 8, 2023
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.

2 participants