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

Primary state error transitions #1064

Conversation

thebyohazard
Copy link

This PR (built on top of required PRs ros2/rcl_interfaces#97 and ros2/rcl#618) provides convenience functions for lifecycle nodes to self-trigger the error transitions created in the previous PRs.

This also addresses issue #547

Copy link
Collaborator

@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.

This needs to rebase and add description for each method.

const State &
LifecycleNode::raise_error()
{
if(get_current_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think this needs to be checked here. it will check the current state and get the reference to rcl_lifecycle_transition_t in rcl state_machine.

{
if(get_current_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE)
return impl_->trigger_transition(
lifecycle_msgs::msg::Transition::TRANSITION_ACTIVE_ERROR);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
lifecycle_msgs::msg::Transition::TRANSITION_ACTIVE_ERROR);
rcl_lifecycle_error_label);

@norro
Copy link

norro commented Nov 23, 2020

What is the state of this PR?

@Myzhar
Copy link

Myzhar commented Sep 25, 2021

Will this PR be ever merged?

@fujitatomoya
Copy link
Collaborator

I think we still need to work on design at 1st, but i don't think i can be working on this in soon.

@clalancette clalancette changed the base branch from master to rolling June 28, 2022 14:28
@clalancette
Copy link
Contributor

Given that this PR is so old, and that we need a design fix first, I'm going to close it out. If you are planning on working on it, please push forward on the design update first, then we can open a new PR with changes that implement that design. Thanks for the contribution.

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.

6 participants