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

Harmoni_microphone stop not working #36

Closed
micolspitale93 opened this issue Jan 21, 2021 · 1 comment
Closed

Harmoni_microphone stop not working #36

micolspitale93 opened this issue Jan 21, 2021 · 1 comment

Comments

@micolspitale93
Copy link
Collaborator

The stop() (OFF actionType) is not working in the harmoni_microphone.

@RMichaelSwan
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants