-
Notifications
You must be signed in to change notification settings - Fork 0
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
Karsten/serialized ipm touchups #2
base: dnae_adas/serialized_ipm
Are you sure you want to change the base?
Karsten/serialized ipm touchups #2
Conversation
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
@DensoADAS please let me know if you guys can work with this. I would appreciate to have the Serializer and SerialiedMessage being part of Foxy as such. |
Signed-off-by: Karsten Knese <[email protected]>
I had to disable the The problem with this test currently is that I some messages gets destroyed on the stack:
when enabling |
Signed-off-by: Karsten Knese <[email protected]>
any_callback_(callback), serializer_(serializer) | ||
{ | ||
if (!std::is_same<MessageT, CallbackMessageT>::value && | ||
!std::is_same<MessageT, rclcpp::SerializedMessage>::value && | ||
!std::is_same<CallbackMessageT, rcl_serialized_message_t>::value) | ||
!std::is_base_of<rcl_serialized_message_t, MessageT>::value) |
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 wasn't exactly sure why this check is happening. Also, I wouldn't compile if we check for the CallbackMessageT
, which I believe is the right thing to check for though.
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
Signed-off-by: Karsten Knese <[email protected]>
So I am at a state where the serialize to serialize communication work, however all others not. So the main critique I have for this PR is that it's adding always to id's to the intraprocess communication manager. And I think this should be possible to limit to one, given that it's the same publisher. |
This is still WIP, but essentially this is how I would envision to deal with the
Serializer
andSerializedMessage
class.The tests currently don't run, because I still have to get my head around the actual intraprocess communication. There's a lot of memory corruption and BAD_ACCESS exit codes around the
context
in the tests.