Skip to content

Merge changes to the device abstraction ("EEG") class from my repo #43

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

Open
3 tasks
ErikBjare opened this issue Dec 3, 2020 · 7 comments · May be fixed by #93
Open
3 tasks

Merge changes to the device abstraction ("EEG") class from my repo #43

ErikBjare opened this issue Dec 3, 2020 · 7 comments · May be fixed by #93
Assignees
Labels
bhg2020 Brainhack global 2020 sub-project enhancement New feature or request

Comments

@ErikBjare
Copy link
Collaborator

ErikBjare commented Dec 3, 2020

My stuff is in: https://github.com/ErikBjare/thesis/blob/master/src/eegwatch/devices/

Includes a bunch of minor changes, including:

  • Added type annotations
  • Support for recording Muse PPG/ACC/GYRO streams

TODO

  • start/stop is different for Brainflow vs Muse devices, this should maybe be refactored, but at least be documented.
  • I've ran the black formatter on all my code, so merging would be easiest after similar formatting has been applied on the eeg-notebooks project (like in style: formatted with black eegnb/ #30)
  • Needs a way to enable PPG/ACC/GYRO streams (without hardcoding)
@ErikBjare ErikBjare self-assigned this Dec 3, 2020
@ErikBjare ErikBjare added the bhg2020 Brainhack global 2020 sub-project label Dec 3, 2020
@JadinTredup
Copy link
Collaborator

@ErikBjare it appears to me that you have completed this or is there still more work to be done?

@ErikBjare
Copy link
Collaborator Author

@JadinTredup Nope, never got around to making a PR.

I've made some pretty major changes, and working on a few more, almost to the point where I'm not sure if it makes sense to upstream it. But if it's of interest I'll consider it: https://github.com/ErikBjare/thesis/tree/master/src/eegwatch/devices

@JohnGriffiths
Copy link
Collaborator

@ErikBjare - what's your thoughts. Do you think these changes should be added or is divergence too great?

@JohnGriffiths JohnGriffiths added the enhancement New feature or request label Mar 19, 2021
@ErikBjare
Copy link
Collaborator Author

ErikBjare commented Apr 11, 2021

@JohnGriffiths I'm not sure, I'd love it if you had a look at it and told me what you think.

Code is here: https://github.com/ErikBjare/thesis/tree/master/src/eegwatch/devices

Basically I created an abstract class EEGDevice that is implemented by the classes MuseDevice and BrainflowDevice. It simplifies a lot of the code imo, no more if statements to check if using one type of device or another, for example.

I also implemented a check method (only for MuseDevice so far) which does a very basic signal check (old method, not the newer one I linked you recently) and returns the list of bad channels, but that probably doesn't belong in the class (at least not in its current state).

Edit: Oh, and it also uses the bleak backend in muse-lsl by default (not yet merged upstream in muse-lsl afaik).

@JadinTredup
Copy link
Collaborator

@ErikBjare I am going to look this over today. Thank you so much for doing this. One of my biggest take aways from my internship development-wise has been implementing classes like this and so I was intending to do this at some point.

@JadinTredup
Copy link
Collaborator

@JohnGriffiths I like this a lot and think it improves our flexibility/scalability in the future. If you're okay with it I say we set up another branch and work on merging this.

@ErikBjare
Copy link
Collaborator Author

@JadinTredup Branch set up in #92, comments welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bhg2020 Brainhack global 2020 sub-project enhancement New feature or request
Projects
3 participants