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

Karsten/serialized ipm touchups #2

Draft
wants to merge 9 commits into
base: dnae_adas/serialized_ipm
Choose a base branch
from

Conversation

Karsten1987
Copy link

This is still WIP, but essentially this is how I would envision to deal with the Serializer and SerializedMessage 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.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@Karsten1987 Karsten1987 marked this pull request as draft April 18, 2020 04:59
@Karsten1987
Copy link
Author

@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 <karsten@openrobotics.org>
@Karsten1987
Copy link
Author

I had to disable the INSTANTIATE_TEST_CASE_P on my OSX machine as this led to unexpected memory corruption when trying to cleanup the parameters. I worked around it by putting them in the fixture.

The problem with this test currently is that I some messages gets destroyed on the stack:

test_intra_process_communication(59575,0x1007c75c0) malloc: *** error for object 0x1058a6600: pointer being freed was not allocated
test_intra_process_communication(59575,0x1007c75c0) malloc: *** set a breakpoint in malloc_error_break to debug

when enabling publisher->publish(std::make_unique<rcl_serialized_message_t>(msg0));

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
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)
Copy link
Author

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 <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@Karsten1987
Copy link
Author

So I am at a state where the serialize to serialize communication work, however all others not.
I would appreciate if you guys could have a look at this.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant