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

feat(replay): add user guidance #472

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

Mustaballer
Copy link
Collaborator

@Mustaballer Mustaballer commented Aug 18, 2023

What kind of change does this PR introduce?

This PR introduces a feature enhancement to address OpenAdapt Issue #104, providing functionality for handling user interactions during replay strategies.

Summary

This PR addresses the need to prompt users during replay strategies when the current Screenshot/WindowState deviates from the expected state in the recording. It implements the following steps:

  1. Compare the current state to the expected state in the current recording.
  2. If not found, compare the current state to states in other recordings with the same description.
  3. If not found, pause the replay.
  4. Notify the user that they need to demonstrate how to continue.
  5. Start a new recording (with the same description as the one being recorded) as soon as the user starts generating InputEvents.
  6. Wait for the user to finish the demonstration (e.g., by clicking on the system tray icon).
  7. Restart the replay from where we left off.

Checklist

  • My code follows the style guidelines of OpenAdapt.
  • I have performed a self-review of my code.
  • If applicable, I have added tests to prove my fix is functional/effective.
  • I have linted my code locally prior to submission.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt).
  • New and existing unit tests pass locally with my changes.

How can your code be run and tested?

Run python -m openadapt.replay_control

Other information

@@ -24,7 +24,7 @@

from openadapt import common, config
from openadapt.db import BaseModel
from openadapt.logging import filter_log_messages
from openadapt.logging_config import filter_log_messages
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was facing this error

PSYCHOPY Error: AttributeError: module 'logging' has no attribute 'getLogger'

Used this solution from stackoverflow post:

If the current file you are working has name logging.py change it else it will give circular import error.

@Mustaballer
Copy link
Collaborator Author

@abrichr Currently I created the base code for running pyside6 application which prompts the user to to record(essentially just the UI), but have not implemented the functionality for actually recording or inserting into the db yet. I have listed below some screenshots of the pyside application. Let me know how I can improve this!
image
image

@abrichr
Copy link
Contributor

abrichr commented Aug 21, 2023

What do you think about removing the "Recording in progress" screen?

As a next step, we need an API somewhere that can be triggered when running openadapt.replay (explicitly or by the model) that will:

  1. pause replay,
  2. start a recording,
  3. once the recording is in progress** (waiting until it's fully started is important), show the notification to the user that their assistance is required,
  4. stop the recording as usual (i.e. via system tray),
  5. restart replay*
  • There is some subtlety here that we need to explore.

** We need to have some additional process in record.py (or the main process) wait until all of the readers and writers have started (all of the readers and writers need to receive a started_event that they trigger once they have started), after which point it triggers its own global_started_event that gets picked up downstream.

@abrichr
Copy link
Contributor

abrichr commented Sep 4, 2023

From our 1-on-1 conversation notes:

API should implement a singleton pattern
https://python-patterns.guide/gang-of-four/singleton/

Replay.py: (rename to control.py, or control/control.py or similar)

    _pause_event = multiprocessing.Event()

    Def replay():
        # this has been implemented
        …
        strategy.run(pause_event=_pause_event)
        …

    Def pause():
        # set the pause_even

    Def unpause()
        # clear the pause event

replay/ → control/
    __init__.py
       # import contents of replay.py (control.py)
       __all__ = [replay, pause, unpause, …]

Have configurable pause key in config.py to pause replay
Rename to control submodule

Refactor stop_sequence_detected to stop_sequence_event (optional)

We need to pass something into each of the processes in record.py that the process sets once it has started. E.g. a started_event. If we use multiprocessing.Event, we need a separate event per process. If we use multiprocessing.Vlaue, we can have a single event that each process increments. (The latter sounds like a leaky abstraction).

# whenever a process is created in record.py, we create a multiprocessing.Event and append it to a list
# before `while not stop_sequence_detected:`, we loop over all events in the list until they are set, at which point we set a new event, e.g. started_event

Now during replay, in a function called on_assistance_required (when it is called is TBD), we start the recording (passing in the started_event to record.py), and wait until the started_event is set, after which we can display a notification to the user

We need to confirm that the memory usage of record.py does not increase when the user is idle during a recording.

https://python-patterns.guide/gang-of-four/

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

Successfully merging this pull request may close these issues.

None yet

2 participants