-
Notifications
You must be signed in to change notification settings - Fork 249
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
OpenEphysBinaryRawIO: Fixing ttl multichan #1603
base: master
Are you sure you want to change the base?
Conversation
Thanks @vigji! Looking great! |
Great, let me know if there's anything needed before merging! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, although I would ask for one additional comment in the code to explain what version this works for (all or v0.6+ whatever) along with what the structure of the data is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for me, but Alessio knows this way better so I'll let him decide when to merge/do final review.
Addressing #1437.
Currently, all the logic in the events parser for OpenEphys data assumes that there is a single stream of events (simply positive or negative
states
), and it breaks down when acquiring multiple digital signals at once (states
having different absolute values for different channels).This is a simple fix that looks for rising and falling edges separately for each of the digital channels.
Add simple testing relying on existing test data, which fails before the fix and passes afterwards.