Skip to content
This repository has been archived by the owner on Dec 1, 2020. It is now read-only.

Feature: implement state machine shutdown #412

Merged
merged 23 commits into from
Nov 28, 2019

Conversation

Olavhaasie
Copy link
Contributor

Closes PM-61

Description

This PR fixes the preemption of the statemachine correctly when shutting down ROS. When ROS is shutdown, the state machine will preempt the current states and shutdown gracefully by going to the SHUTDOWN state. The concurrent SafetyState has now been implemented as a smach_ros.MonitorState, which will preempt the state machine when something is published on /march/error. Then the state machine will go to ERROR, which is currently not implemented and automatically goes to SHUTDOWN. In the future this can be used for any error handling to return to HEALTHY. The state machine is now also capable of preempting the WAIT FOR GAIT SERVER state. Furthermore, the IdleState is now implemented with callbacks from control_flow, which made it easier to preempt it.

tl;dr No more errors when pressing ctrl-c 🙆‍♂️

Changes

  • Implement SafetyState as MonitorState
  • Add preemption of concurrent states
  • Add preemption on IdleState
  • Add ShutdownState
  • Some cleaning up of code that was no longer necessary
  • Renaming files from CamelCase to underscore_case

@Olavhaasie Olavhaasie requested a review from a team as a code owner November 26, 2019 11:39
@Olavhaasie Olavhaasie self-assigned this Nov 26, 2019
@ghost ghost requested review from JorisWeeda and RutgerVanBeek November 26, 2019 11:39
RutgerVanBeek
RutgerVanBeek previously approved these changes Nov 26, 2019
Copy link
Contributor

@Roelemans Roelemans left a comment

Choose a reason for hiding this comment

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

Please implement an ugly solution to kill Gazebo

@Olavhaasie
Copy link
Contributor Author

Don't forget to install psutil

rosdep install -y --from-paths src --ignore-src --rosdistro kinetic

RutgerVanBeek
RutgerVanBeek previously approved these changes Nov 27, 2019
Copy link
Contributor

@RutgerVanBeek RutgerVanBeek left a comment

Choose a reason for hiding this comment

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

Nice for the Gazebo exit solution

Copy link
Contributor

@Roelemans Roelemans left a comment

Choose a reason for hiding this comment

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

Lekker gewerkt!

You should probably update the tutorial https://project-march.github.io/tutorials/doc/development/add_a_new_gait.html

You should also look through all the <gait_name>_sm.py files for 'preempted' -> 'preempted' transitions. They should go to failed.

JorisWeeda
JorisWeeda previously approved these changes Nov 27, 2019
Copy link
Contributor

@JorisWeeda JorisWeeda left a comment

Choose a reason for hiding this comment

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

except for the few comments Roel placed the request looks good to me.

from march_state_machine.ControlFlow import control_flow
from feedback_action_state import FeedbackActionState
from march_shared_resources.msg import GaitNameAction, GaitNameGoal
from march_state_machine.control_flow import control_flow


class StoppableState(FeedbackActionState):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this class should have a similar implementation of control_flow to the one in IdleState. This will also allow the removal of some unused methods in control_flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class is a bit different in implementation since it inherits from SimpleActionState, which implements preemption for itself, so it is not possible to use the same kind of implementation.

Copy link
Contributor

@Roelemans Roelemans left a comment

Choose a reason for hiding this comment

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

Beautiful work sir!

@Roelemans Roelemans merged commit 958b30b into develop Nov 28, 2019
@Roelemans Roelemans deleted the feature/PM-133-implement-shutdown branch November 28, 2019 10:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants