Support RHS2116 streaming and stimulation #67
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonnew
reviewed
Apr 1, 2024
There was a problem hiding this comment.
Overall, I think this is move in the right direction. However, base virtual functions return void and are therefore ripe for redundancy and abuse. It makes me wonder if we should really just virtualize the entire headstage finding procedure and tell the headstage to figure it out and let us know if it finds anything or not.
- Support acquisition from RHS2116, writing filter settings, etc. - Does not have the ability to configure or trigger stimulus sequences and does not yet include a Rhs2116TriggerDevice in ConfigureHeadstageRhs2116. GUI support will be critical for this.
- Add stimulus trigger device to headstage - Add stimulus sequence definition to RHS2116 configuration - This will require GUI support for configuring the stimulus waveform. - Tested with hardcoded stimulus parameters and it works great.
- Stimulus trigger device accepts a delay in microsends to the applicatin of a stimulus - Mark CheckLinkStatus as virtual so headstage can be reset between power cycles (see #120) - Rebase main and take advantage of latest improvments with respect to port voltage override
- The voltage drop is what it is. Im getting reliable locking with this the algorithm that is implemented in ConfigureHeadstageRhs2116LinkController
glopesdev
commented
Jul 2, 2024
Comment on lines
86
to
89
| if (base.CheckLinkState(device)) | ||
| { | ||
| SetPortVoltage(device, voltage + VoltageOffset); | ||
| if (CheckLinkState(device)) | ||
| { | ||
| // TODO: The RHS2116 headstage needs an additional reset after power on | ||
| // to provide its device table. | ||
| Thread.Sleep(500); | ||
| device.Context.Reset(); | ||
| return true; | ||
| } | ||
| else break; | ||
| return CheckLinkState(device); |
There was a problem hiding this comment.
Just noticed this now, and was really confused. If we just need to reset before the last check, why not simply add device.Context.Reset() before the return call?
My suspicion is that perhaps this is related to the case where we want to just set a port voltage override and therefore the code does not go through this path?
In that case, maybe the best would be to mark ConfigurePortVoltageOverride itself as virtual and override that?
There was a problem hiding this comment.
I think this was addressed in #124 which does what you suggest.
Bonsai/Bonsai.config
Outdated
| <AssemblyLocation assemblyName="Microsoft.Web.WebView2.Core" processorArchitecture="MSIL" location="Packages\Microsoft.Web.WebView2.1.0.1823.32\lib\net45\Microsoft.Web.WebView2.Core.dll" /> | ||
| <AssemblyLocation assemblyName="Microsoft.Web.WebView2.WinForms" processorArchitecture="MSIL" location="Packages\Microsoft.Web.WebView2.1.0.1823.32\lib\net45\Microsoft.Web.WebView2.WinForms.dll" /> | ||
| <AssemblyLocation assemblyName="Microsoft.Web.WebView2.Wpf" processorArchitecture="MSIL" location="Packages\Microsoft.Web.WebView2.1.0.1823.32\lib\net45\Microsoft.Web.WebView2.Wpf.dll" /> | ||
| <AssemblyLocation assemblyName="OpenCV.Net" processorArchitecture="MSIL" location="Packages\OpenCV.Net.3.4.2\lib\net462\OpenCV.Net.dll" /> | ||
| <AssemblyLocation assemblyName="OpenEphys.PhaseDetector" processorArchitecture="MSIL" location="Packages\OpenEphys.PhaseDetector.0.6.0\lib\net472\OpenEphys.PhaseDetector.dll" /> |
There was a problem hiding this comment.
Is the phase detector package used in the implementation, or just for examples?
There was a problem hiding this comment.
Oops. Should not be there. Also don't know why I committed this file.
- First shot at using XML documentaiton to generate coherent and user-friendly documentation
- Improve ConfigureAnalogIO.Process documentation - Use standard enum documentation practices
Minor updates to documentation
- Neuropixels 2.0 has a completely different register map compared to 2.0-beta. This is reflected in the changes in this commit. - Common static helper functions for 1.0, 2.0, and 2.0-beta were moved to BitHelper.cs and NeuropixelV2.cs
- There was lingering discussion after merge into main occured, this attempts to address that
- It was left connected to the last probe configured which caused a huge amount of cross talk if i2c bus was used during acquisition
- Gain corrections were not used in NeuropixelsV1 - Gain corrections for NeuropixelsV2 were changed form fixed point multiplication to floating point since we are no longer doing this on an FPGA
Fix register mapping for Neuropixels 2.0 public probes
- Emphasize Device Manager is singleton - Remove information on oni Hubs from MutliDeviceFactory since this has nothing to do with ONI even though it kinda feels like it does
Resolve issues in discussion of #189
Publish packages to github on release
Update bootstrapper version
- Support acquisition from RHS2116, writing filter settings, etc. - Does not have the ability to configure or trigger stimulus sequences and does not yet include a Rhs2116TriggerDevice in ConfigureHeadstageRhs2116. GUI support will be critical for this.
- Add stimulus trigger device to headstage - Add stimulus sequence definition to RHS2116 configuration - This will require GUI support for configuring the stimulus waveform. - Tested with hardcoded stimulus parameters and it works great.
- Stimulus trigger device accepts a delay in microsends to the applicatin of a stimulus - Mark CheckLinkStatus as virtual so headstage can be reset between power cycles (see #120) - Rebase main and take advantage of latest improvments with respect to port voltage override
- The voltage drop is what it is. Im getting reliable locking with this the algorithm that is implemented in ConfigureHeadstageRhs2116LinkController
- Up to date with frame distribution, DeviceType, etc.
- Deleted bracket by accident - Incorrectly removed virtual keywork from CheckLinkStatus function declaration
bparks13
reviewed
Aug 8, 2024
There was a problem hiding this comment.
This file (along with a few others in the same OpenEphys.Onix/OpenEphys.Onix folder path) are leftovers from the old folder pattern. These should be removed.
bparks13
reviewed
Aug 8, 2024
|
|
||
| return new CompositeDisposable( | ||
| DeviceManager.RegisterDevice(deviceName, device, DeviceType), | ||
| analogLowCutoff.Subscribe(newValue => |
There was a problem hiding this comment.
All calls should be using the SubscribeSafe method instead of Subscribe.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds basic support for RHS2116 configuration, amplifier data streaming and stimulation functionality.
Fixes #26