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

Support RHS2116 streaming and stimulation #67

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

Support RHS2116 streaming and stimulation #67

wants to merge 123 commits into from

Conversation

glopesdev
Copy link
Collaborator

This PR adds basic support for RHS2116 configuration, amplifier data streaming and stimulation functionality.

  • Support acquisition from RHS2116, writing filter settings, etc.
  • Add stimulus sequence definition to RHS2116 configuration
  • Will require GUI support for configuring the stimulus waveform.
  • Tested with hardcoded stimulus parameters and it works great.

Fixes #26

@glopesdev glopesdev added the feature New planned feature label Mar 31, 2024
@glopesdev glopesdev added this to the 0.1.0 milestone Mar 31, 2024
Copy link
Member

@jonnew jonnew left a comment

Choose a reason for hiding this comment

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

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.

jonnew and others added 6 commits June 28, 2024 12:09
- 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
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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

I think this was addressed in #124 which does what you suggest.

<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" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is the phase detector package used in the implementation, or just for examples?

Copy link
Member

Choose a reason for hiding this comment

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

Oops. Should not be there. Also don't know why I committed this file.

jonnew and others added 24 commits August 2, 2024 11:12
- Clarification and typo fixes
- 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
Publish packages to github on release
- 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
Copy link
Member

Choose a reason for hiding this comment

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

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.


return new CompositeDisposable(
DeviceManager.RegisterDevice(deviceName, device, DeviceType),
analogLowCutoff.Subscribe(newValue =>
Copy link
Member

Choose a reason for hiding this comment

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

All calls should be using the SubscribeSafe method instead of Subscribe.

@jonnew jonnew modified the milestones: 0.2.0, 0.3.0 Aug 13, 2024
@jonnew jonnew modified the milestones: 0.3.0, 0.4.0 Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New planned feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RHS2116 configuration and streaming
3 participants