robotology / assistive-rehab

Assistive and Rehabilitative Robotics
https://robotology.github.io/assistive-rehab/doc/mkdocs/site
BSD 3-Clause "New" or "Revised" License
20 stars 11 forks source link

Improve handling of questions by `managerTUG` #351

Closed mfussi66 closed 1 year ago

mfussi66 commented 1 year ago

In the current state, the managerTUG can continuously receive questions and suspend execution of the demo at any time. This is suboptimal because it corrupts the outcome of the TUG results, and it does not interact robustly with the rest of the state machine.

It would be better if the module had a dedicated state in which the user can ask questions, after the explanation of the experiment and before its start.

mfussi66 commented 1 year ago

Last friday I kicked off the activity.

A bit of background: the managerTUG is a state machine (FSM) that is implemented inside an RFModule. The module ticks, and so does the FSM. To handle the speech trigger, managerTUG instantiates a thread that reads from the trigger port. The manTUG, at each tick, checks if the speech thread requested a freeze of the FSM sequence. The request is ALWAYS fulfilled.

(Side note: the speech thread is not periodic but there is a while loop in its run method that spins at clock speed)

To satisfy the issue in this request, I started a bit of logic refactoring: 1 - the speech thread is created, but suspended immediately 2 - I created a specific State::questions that occurs after R1 has explained the experiment to the subject 3 - as soon as the FSM enters in State::questions, the speech thread is resumed, and it should be ready to process the trigger 4 - the thread is suspended after a timeout

Maybe the timeout should be reset after each question, to avoid sudden dialogue interruptions.

pattacini commented 1 year ago

Maybe the timeout should be reset after each question, to avoid sudden dialogue interruptions.

I think so 👍🏻

pattacini commented 1 year ago

Linked the branch feat/mantug to this issue[^1].

Just a side note for branch naming convention. It turns out that it is convenient to call the branch like [feat|fix]/issue-number, where issue-number is the number of the issue describing the work. Then, we can connect the issue and the branch[^2], as I did here.

[^1]: I suppose this is the branch; feel free to change it, otherwise. [^2]: This will create automatically the connection between the issue and any PR opened on that branch.

mfussi66 commented 1 year ago

Today we were able to successfully test the changes directly on the demo, the logic proposed above works.

The only "breaking" change is the removal of the "frozen" state, in which navigation is stopped if a question is received. Now that there is a dedicated state for question time, that state is no longer needed since R1 will be still.

I'll update the issue with the related pr.