Skip to content

[jsk_topic_tools] Add header file to check jsk_topic_tools' version#1753

Merged
k-okada merged 2 commits intojsk-ros-pkg:masterfrom
iory:version
Jul 15, 2022
Merged

[jsk_topic_tools] Add header file to check jsk_topic_tools' version#1753
k-okada merged 2 commits intojsk-ros-pkg:masterfrom
iory:version

Conversation

@iory
Copy link
Copy Markdown
Member

@iory iory commented Jul 14, 2022

What is this?

I add version.h for jsk_topic_tools to check version for build.
jsk_topic_tools has base class such as DiagnosticNodelet and ConnectionBasedNodelet.
For example, nodes in jsk_recognition are using those notelets.

This PR is useful to check jsk_topic_tools version.

Related to jsk-ros-pkg/jsk_recognition#2712

@iory iory requested review from k-okada and knorth55 July 14, 2022 10:53
@k-okada k-okada merged commit d1e9474 into jsk-ros-pkg:master Jul 15, 2022
${CMAKE_CURRENT_SOURCE_DIR}/include/jsk_topic_tools/nodelet_version.h.in
${CATKIN_DEVEL_PREFIX}/${CATKIN_GLOBAL_INCLUDE_DESTINATION}/jsk_topic_tools/nodelet_version.h)

catkin_package_xml()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sorry, I think it safe to put configure_file after catkin_package so that we do not need to use catkin_package_xml, because I suppose catkin_package_xml is not designed to used in User's program. how do you think? @iory

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see. That's good for users.
https://github.com/ros/catkin/blob/2612c4ce82a285cac13263576f8a85b8a6d6cd3d/cmake/catkin_package.cmake#L97-L100
catkin_package internally calls catkin_package_xml.

I created a PR #1755

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants