Skip to content

Conversation

@SteveMacenski
Copy link
Contributor

@SteveMacenski SteveMacenski commented Mar 14, 2025

Fixing lifecycle node autostart issue #445

The issue boils down to the emitting of signals for the first transition in the transition_ids. It appears that sometimes the Launch system doesn't emit these in order or for whatever reason the second signal is received by the server before the first. By putting them both in a single transition, only the configure signal is emitted and activate is dependent fully on the state transition.

Honestly, this is probably how it should have always been implemented, but I didn't think it mattered at the time since I thought ordering was guaranteed.

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@tonynajjar
Copy link

Tested, looks good! Thanks

@ahcorde
Copy link
Contributor

ahcorde commented Mar 17, 2025

Pulls: #449
Gist: https://gist.githubusercontent.com/ahcorde/3f7cb3b5b6df5ba19d6aa21f40b4d8e3/raw/aa479fb8dbc6f90543c778d5dcd674b2732db638/ros2.repos
BUILD args: --packages-above-and-dependencies launch_ros
TEST args: --packages-above launch_ros
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15403

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RHEL Job got disconnected, i just restarted that. windows failures look unrelated.

@SteveMacenski
Copy link
Contributor Author

Great, can we merge?

@christophebedard
Copy link
Member

The RHEL job passed, let's go!

@christophebedard christophebedard merged commit 98952b5 into ros2:rolling Mar 17, 2025
2 checks passed
@ahcorde
Copy link
Contributor

ahcorde commented Mar 18, 2025

@SteveMacenski should we backport this to jazzy and/or humble ?

@christophebedard
Copy link
Member

I think Jazzy only, since I believe #430 was only backported to Jazzy, but I just need to actually review #438 (Jazzy backport of #430).

@christophebedard
Copy link
Member

@Mergifyio backport jazzy

@mergify
Copy link

mergify bot commented Mar 25, 2025

backport jazzy

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Mar 25, 2025
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
(cherry picked from commit 98952b5)

# Conflicts:
#	launch_ros/launch_ros/actions/lifecycle_node.py
#	launch_ros/launch_ros/actions/load_composable_nodes.py
christophebedard pushed a commit that referenced this pull request Mar 25, 2025
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
(cherry picked from commit 98952b5)
christophebedard pushed a commit that referenced this pull request Mar 25, 2025
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
(cherry picked from commit 98952b5)
christophebedard pushed a commit that referenced this pull request Mar 25, 2025
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
(cherry picked from commit 98952b5)

Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>
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.

5 participants