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

Preemption refactoring #37

Open
RMichaelSwan opened this issue Apr 7, 2021 · 1 comment
Open

Preemption refactoring #37

RMichaelSwan opened this issue Apr 7, 2021 · 1 comment
Labels
enhancement New feature or request
Projects

Comments

@RMichaelSwan
Copy link
Collaborator

RMichaelSwan commented Apr 7, 2021

Preemption issue originally Fixed in 59ad148 . Issue was with blocking processes and no threads to unblock them.
May want to do a full refactor to avoid use of preempt_cb. Discussion so far:

  1. The HarmoniActionServer has a main thread (thread 1) which creates two threads--the first we explicitly create for the executeCallback (thread 2) and another new thread that seems to be created for regular callbacks (thread 3--this thread creation must occur in the actionlib code somewhere).
  2. The service_server and its associated service live inside of the executeCallback thread (thread 2).
  3. Any while loops in your services which are called in response to start, do, request, etc. will block the service_server from responding to state transitions until the while loop breaks. As long as there is a sleep in your while loop, data (e.g. state) can be changed by a separate thread which can be used as a loop break signal.
  4. I implemented a workaround for the issue caused by previous point without rewriting existing loops. I used a preempt callback in the service_server that the ActionServer (thread 3) can call at any time. This callback updates the State to PAUSE, which the while loop checks for and breaks out of when seen.
  5. We should refactor code which contains long-running while loops. The preempt workaround I have provided requires extra knowledge of classes a service creator shouldn't care about too much. Possible solutions include putting the service_server in a separate thread, making an obvious requirement for service creators to thread their loops, or using a timer thread to poll for goal updates
  6. The current start_sending_feedback() function found in the service_server is another a blocking function; in the microphone_service case it is called on the main thread (thread 1) after the HarmoniActionServer initializes. This deserves a refactor--feedback should either be reported when changes occur (e.g. state changes) or it should have its own thread, assuming we want it to be a longrunning monitor process.
  7. I think this is an old discussion, but I still feel the title of the HarmoniServiceManager service template is confusing. It's being used as a service template usually, so why not just call it that? e.g. Serviceor AbstractService. I just checked, and there is no name overlap with built-ins.

Chris: Awesome write up! I completely agree that preempting is not a good long term solution, unless it is implemented under the hood as part of the action_server.py - so that no one needs to know what it is doing. Taking a step back, I think we may want to discuss if we want services to be state machines - with a state first approach and their own threads, or functions - which the action server is calling upon. I am excited to discuss on Wednesday. I'll try and wrap up the rostests before than so maybe we can close this branch before we get too far into the rewrite.

Originally posted by @RMichaelSwan in #36 (comment)

@RMichaelSwan RMichaelSwan added the enhancement New feature or request label May 12, 2021
@RMichaelSwan
Copy link
Collaborator Author

Update: Preemption has been refactored to use threading, but the preemption callback workaround is still in place...may want to remove it?

@chrismbirmingham chrismbirmingham added this to Backlog in Development Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

1 participant