interaction-lab / interaction-lab/HARMONI

Preemption refactoring

Open
#37 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
10
Forks
8
PR merge metrics
No merged PRs in 30d

Description

Preemption issue originally Fixed in 59ad148ccaa4eca5271f1d2511cef82334d5b748 . 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. `Service`or `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 https://github.com/interaction-lab/HARMONI/issues/36#issuecomment-815154006_

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.