Skip to content

added pause/resume to Tracker.cs #152

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

Merged
merged 2 commits into from
Apr 20, 2023

Conversation

martinmmueller
Copy link
Contributor

@martinmmueller martinmmueller commented Feb 15, 2023

Motivation for the Pull request

I wanted the option to pause and resume trackers without dumping the data that was already collected (StartRecording creates a new DataTable), so I quickly built in a Pause and a Resume function with an enum-based state to keep track of what the tracker is currently doing.

Maybe this could be useful to others as well, so I thought I'd put the edit up here.

Some notes:

  • I realise the state enum I added makes the 'recording' field kind of redundant but I left that in place to not break any possible dependencies.
  • I added some debug warnings for telling the tracker to start/pause/stop etc. when it does not really make sense. Ideally the tracker would only react to commands when it is in the matching state in the first place, but I did not want to change too much about the base class, so I left it at warnings for now. If those don't match your policy for logging/ error handling they could be escalated to full Exceptions (like you do in RecordRow), or just removed so the Tracker just silently does its thing.

added the option to pause/resume tracker without dumping old data
@jackbrookes
Copy link
Member

Looks good other than minor tweaks

@martinmmueller
Copy link
Contributor Author

Hey, thanks for the fast reply!
I implemented the changes as you requested, but I changed the calls to the recording field in checks (e.g. in Update()) to calls to the Recording Property (which now wraps the the state check as you suggested) instead of making the explicit check every time. I think it should be more readable this way.

@jackbrookes jackbrookes merged commit fbeeb77 into immersivecognition:master Apr 20, 2023
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.

2 participants