Skip to content

feat: add destructor to socket_can_receiver_node (#75) - #100

Merged
xmfcx merged 2 commits into
test/jazzy-09-node-destruction-testfrom
feat/jazzy-10-receiver-node-destructor
Sep 7, 2026
Merged

feat: add destructor to socket_can_receiver_node (#75)#100
xmfcx merged 2 commits into
test/jazzy-09-node-destruction-testfrom
feat/jazzy-10-receiver-node-destructor

Conversation

@xmfcx

@xmfcx xmfcx commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Description

Backport the receiver destructor from main to jazzy. This is layer 10 of 11, based on test/jazzy-09-node-destruction-test.

The destructor stops and joins the receiver thread. The stop flag also allows lifecycle cleanup while the ROS context remains up.

This branch includes the Humble formatting and GoogleTest fixes, plus the virtual CAN trigger fix, from the preceding layers.

The CI follow-up awaits a separate review.

AI usage

AI usage: Cherry-picked from main with Claude Code. Codex prepared the CI follow-up, propagated it through normal merge commits, and ran the local checks below.

Self-review: Pure backport, I checked the order and verified that it is correct.

Verification: Local package builds and tests passed on Humble and Jazzy. All five destruction cases passed with virtual CAN on each distribution.

CI on the published head

Humble and Jazzy package builds and tests

Tested bfbea2d48a31b7306dae3a874c3e79e1ebe89292 in Ubuntu 22.04 / Humble and Ubuntu 24.04 / Jazzy containers. All colcon commands ran from /home/mfc/projects/autoware, with isolated build and install paths, Release mode, and --coverage.

  • Both package builds, both package test commands, and both result commands exited with 0.
  • Humble: Summary: 220 tests, 0 errors, 0 failures, 51 skipped
  • Jazzy: Summary: 226 tests, 0 errors, 0 failures, 53 skipped
  • Each distribution ran all five destruction cases with ROS2_SOCKETCAN_REQUIRE_VCAN0=1: 5 tests, 0 errors, 0 failures, 0 skipped.
  • Not run on this head: GitHub CI, an Agnocast-enabled build, or nodes under the Agnocast executor.

@xmfcx xmfcx self-assigned this Sep 4, 2026
@xmfcx
xmfcx marked this pull request as ready for review September 4, 2026 09:11
@xmfcx
xmfcx requested a review from mitsudome-r September 4, 2026 09:13
@xmfcx
xmfcx force-pushed the feat/jazzy-10-receiver-node-destructor branch from 92c33fa to 8c5bea0 Compare September 4, 2026 09:48
@xmfcx
xmfcx force-pushed the feat/jazzy-10-receiver-node-destructor branch from 8c5bea0 to 1c2a249 Compare September 4, 2026 10:07
* feat: add destructor to socket_can_receiver_node

Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>

* fix: stop the receiver thread before joining it

The receive loop exits only when the global context shuts down, so a bare join blocks forever on lifecycle cleanup and on component unload. A stop flag in the loop condition makes every join return within one loop iteration.

The destructor, on_cleanup, and on_shutdown share one stop_receiver_thread() helper. on_shutdown needs it because the shutdown transition does not pass through on_cleanup. on_cleanup now joins before it resets the publishers, which closes a race between the thread's active-state check and its publish call. The helper resets the thread handle, so a later configure starts clean.

Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>

---------

Signed-off-by: mitsudome-r <ryohsuke.mitsudome@tier4.jp>
Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
Co-authored-by: Mete Fatih Cırıt <mfc@autoware.org>
(cherry picked from commit 544b8b0)
Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
@xmfcx
xmfcx force-pushed the feat/jazzy-10-receiver-node-destructor branch from 1c2a249 to 55416b6 Compare September 4, 2026 10:48
Signed-off-by: Mete Fatih Cırıt <mfc@autoware.org>
@xmfcx
xmfcx merged commit f7f57dd into jazzy Sep 7, 2026
10 checks passed
@xmfcx
xmfcx deleted the feat/jazzy-10-receiver-node-destructor branch September 7, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants