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 vtk_viewer CMake #93

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

mpowelson
Copy link
Contributor

This is a start to addressing #71. It makes vtk_viewer ROS agnostic and updates it to use the cmake_common_scripts macros. These add clang-tidy, include what you use, and cppCheck.

@mpowelson mpowelson requested a review from Levi-Armstrong July 7, 2020 19:54
@mpowelson mpowelson force-pushed the feature/cmake_updates branch from b3b0b9b to b342ee4 Compare July 7, 2020 19:57
{
LOG4CXX_INFO(SEGMENTATION_LOGGER, "Segment " << i << " size: " << included_indices_.at(i)->GetNumberOfIds());
CONSOLE_BRIDGE_logInform(("Segment " + std::to_string(i) + " size: " + std::to_string(included_indices_.at(i)->GetNumberOfIds())).c_str());
Copy link
Member

Choose a reason for hiding this comment

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

Why console bridge over log4cxx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were getting log4cxx from rosconsole. Switching to console_bridge seemed like the thing to do to make this ROS agnostic.

Copy link
Member

@Levi-Armstrong Levi-Armstrong Jul 7, 2020

Choose a reason for hiding this comment

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

Also I believe when using the ROS agnostic package within the ros ecosystem you can pipe console bridge output to ros log files.

@mpowelson mpowelson force-pushed the feature/cmake_updates branch from 3e2ec55 to 19694b7 Compare July 7, 2020 22:24
@mpowelson mpowelson force-pushed the feature/cmake_updates branch 2 times, most recently from 1bce43c to afca4a3 Compare July 7, 2020 22:41
@mpowelson
Copy link
Contributor Author

@Levi-Armstrong I restarted CI a few hours ago, and it still passes.

@Levi-Armstrong
Copy link
Member

Looks good to me. @jrgnicho You good with the changes?

@mpowelson
Copy link
Contributor Author

ping

@Levi-Armstrong Levi-Armstrong merged commit 617d698 into ros-industrial:master Jul 22, 2020
DavidSpielman pushed a commit to DavidSpielman/noether that referenced this pull request Sep 8, 2023
* Update vtk_viewer using cmake_common_scripts

* Replace log4cxx with console_bridge

* Address review comments and update to latest cmake_common_scripts
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