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

Make message_events build for ros2 (on windows 10) #72

Closed
wants to merge 3 commits into from

Conversation

bfjelds
Copy link

@bfjelds bfjelds commented Jan 9, 2018

Should go to a ROS2 branch.

Working towards enabling laser_filters in ROS2. This provides message_events for message_filters.

@dirk-thomas
Copy link
Member

Thank you for starting this PR.

I am not really sure I understand the scope of the patch though. The packages in this repo are very specific to the generated C++ code of ROS 1 messages. Even with the proposed changes those wouldn't be used in ROS 2 messages anywhere. So I currently don't see how this would be useful.

Maybe you can provide a more detailed context, describe the goal as well as provide steps how to reproduce what you are trying to achieve with this (and related) PRs.

@bfjelds
Copy link
Author

bfjelds commented Mar 20, 2018

My end goal was really to get laser_filters working, so I generally just chipped away at what needed porting without too much thought towards refactoring or bigger picture. This was created because rostime, message_event and the traits were needed to enable laser_filters.

Note there are a couple other PRs included in the work to get laser_filters going:

ros/filters#19
ros2/geometry2#53

@@ -17,7 +17,7 @@
* * Neither the name of Willow Garage, Inc. nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
*90
Copy link
Member

Choose a reason for hiding this comment

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

typo?

Copy link
Author

Choose a reason for hiding this comment

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

definitely :)

@dirk-thomas
Copy link
Member

Please check if the upcoming changes in ros2/message_filters#2 are covering all the cases of this patch.

@dirk-thomas
Copy link
Member

Closing in favor of ros2/message_filters#5.

@dirk-thomas dirk-thomas closed this Aug 3, 2018
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