Skip to content

Revert "Use serialized message (#1081)"#1090

Closed
ivanpauno wants to merge 1 commit intomasterfrom
ivanpauno/revert-#1081
Closed

Revert "Use serialized message (#1081)"#1090
ivanpauno wants to merge 1 commit intomasterfrom
ivanpauno/revert-#1081

Conversation

@ivanpauno
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno commented Apr 23, 2020 •
edited
Loading

@ivanpauno ivanpauno added the in review Waiting for review (Kanban column) label Apr 23, 2020
@ivanpauno ivanpauno requested review from Karsten1987 and hidmic April 23, 2020 12:35
@ivanpauno ivanpauno self-assigned this Apr 23, 2020
@ivanpauno
Revert "Use serialized message (#1081)"
e590bdc 
This reverts commit 46cfe84.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno force-pushed the ivanpauno/revert-#1081 branch from 985a981 to e590bdc Compare April 23, 2020 12:39
@ivanpauno
Copy link
Copy Markdown
Member Author

ivanpauno commented Apr 23, 2020 •
edited
Loading

CI config:

  • build: --packages-up-to test_communication
  • test: --packages-select test_communication --ctest-args -R test_publisher_subscriber_serialized__rmw_connext_cpp (that one is the test that is hanging)

Jobs:

  • Linux Build Status

This was referenced Apr 23, 2020
Closed
Closed
@j-rivero
Copy link
Copy Markdown

j-rivero commented Apr 23, 2020

I've verified that test run at the end of the #1081 (comment) did not run test_communication package. That is why it was not triggered during the PR testing and appeared in the new builds.

@j-rivero
Copy link
Copy Markdown

j-rivero commented Apr 23, 2020

@Karsten1987 is working on a proper fix for the communication tests. Hopefully we don't need to revert.

@ivanpauno
Copy link
Copy Markdown
Member Author

ivanpauno commented Apr 23, 2020 •
edited
Loading

@Karsten1987 The job above confirms that this unblocks CI.
I will left the decision to you if going ahead with these reverts or merging a fix.

(edit) If you need some reviews you can ping me.

@ivanpauno ivanpauno assigned Karsten1987 and unassigned ivanpauno Apr 23, 2020
This was referenced Apr 23, 2020
Merged
Merged
@Karsten1987
Copy link
Copy Markdown
Contributor

Karsten1987 commented Apr 23, 2020 •
edited by ivanpauno
Loading

I just linked two PRs which should fix this:

build all, test only test_communication:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status

@Karsten1987

This comment has been minimized.

Sign in to view
@dirk-thomas

This comment has been minimized.

Sign in to view
@ivanpauno ivanpauno closed this Apr 23, 2020
@ivanpauno
Copy link
Copy Markdown
Member Author

ivanpauno commented Apr 23, 2020

Closed in favor of #1092 and ros2/system_tests#427.

@ivanpauno ivanpauno deleted the ivanpauno/revert-#1081 branch April 23, 2020 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Karsten1987 Karsten1987 Awaiting requested review from Karsten1987

@hidmic hidmic Awaiting requested review from hidmic hidmic is a code owner

Assignees

@Karsten1987 Karsten1987

Labels

in review Waiting for review (Kanban column)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ivanpauno @j-rivero @Karsten1987 @dirk-thomas