-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
DDS changes and fixes #13386
base: development
Are you sure you want to change the base?
DDS changes and fixes #13386
Conversation
@@ -31,6 +32,8 @@ namespace topics { | |||
|
|||
class image_msg | |||
{ | |||
sensor_msgs::msg::Image _raw; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This improves ROS compatibility? Were there image formats that were not supported before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not affect compatibility. Or maybe I misunderstand?
@@ -12,7 +12,7 @@ | |||
#define _FAST_DDS_GENERATED_STD_MSGS_MSG_TIME_H_ | |||
|
|||
|
|||
#include <fastrtps/utils/fixed_size_string.hpp> | |||
//#include <fastrtps/utils/fixed_size_string.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this line something we added or was it downloaded from the net like this?
This is an auto generated file so not recomended to change manually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done it elsewhere and this file is not expected to change.
If it does, I usually do a diff and make sure everything aligns.
Up to you, but I prefer it without the include
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please restore. I don't want to touch third-party files.
Also we don't clean all unneeded includes, just this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline:
We change the headers anyway: we change the license and we have to change the header names and locations.
I just removed the eProsima headers that are not needed; the standard headers, even if not in use, should be OK.
Miscellaneous updates for DDS:
image_msg
did not keep all header data, which made it hard to see what we actually get; now includes everythingfps.py --debug-frames
with better selection of profilespyrealdds.video_encoding.y12i