From 744d58758a757e4606a478db60b73c94b03ae3ca Mon Sep 17 00:00:00 2001 From: jonnew Date: Wed, 31 Jul 2024 13:14:55 -0400 Subject: [PATCH] Minor documentation edits - Verbage clarification - Update some code comments in NeuropixelsV1 - I have some changes for HubDeviceFactory.cs documentation but the class name is changing to refect its true use as abstract device group and i want docs to reflect that. --- OpenEphys.Onix/ConfigureHarpSyncInput.cs | 14 ++++++++------ OpenEphys.Onix/ContextTask.cs | 16 ++++++++-------- OpenEphys.Onix/DataFrame.cs | 2 +- OpenEphys.Onix/DeviceFactory.cs | 7 ++++--- OpenEphys.Onix/NeuropixelsV1eRegisterContext.cs | 17 ++++++----------- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/OpenEphys.Onix/ConfigureHarpSyncInput.cs b/OpenEphys.Onix/ConfigureHarpSyncInput.cs index f9ab6083..b88185ec 100644 --- a/OpenEphys.Onix/ConfigureHarpSyncInput.cs +++ b/OpenEphys.Onix/ConfigureHarpSyncInput.cs @@ -13,7 +13,7 @@ namespace OpenEphys.Onix /// their clocks to a precision of tens of microseconds. This means that all experimental /// events are timestamped on the same clock and no post-hoc alignment of timing is necessary. /// - /// The Harp clock signal is transmitted over a serial line with high precision every second. + /// The Harp clock signal is transmitted over a serial line every second. /// Every time the Harp sync input device in the ONIX breakout board detects a full Harp /// synchronization packet, a new data frame is emitted pairing the current value of the /// Harp clock with the local ONIX acquisition clock. @@ -105,16 +105,18 @@ public NameConverter() public enum HarpSyncSource { /// - /// In standard ONIX breakout boards, the Harp mini-jack connector on the side of the - /// breakout is configured to receive Harp clock synchronization signals. + /// Specifies the Harp 3.5-mm audio jack connector on the side of the ONIX breakout board. /// Breakout = 0, /// - /// In early access versions of the ONIX breakout board, the Harp mini-jack connector is - /// configured for output only, so a special adapter is needed to transmit the - /// Harp clock synchronization signal to the breakout clock input zero. + /// Specifies SMA clock input 0 on the ONIX breakout board. /// + /// + /// In early access versions of the ONIX breakout board, Harp 3.5-mm audio jack connector was + /// configured for output only, so a special adapter was needed to transmit the Harp clock + /// synchronization signal to the breakout clock input zero. + /// ClockAdapter = 1 } } diff --git a/OpenEphys.Onix/ContextTask.cs b/OpenEphys.Onix/ContextTask.cs index f27f09d7..dfa23c5f 100644 --- a/OpenEphys.Onix/ContextTask.cs +++ b/OpenEphys.Onix/ContextTask.cs @@ -111,7 +111,7 @@ private void Reset() /// Gets the system clock rate in Hz. /// /// - /// This describes the frequency of the clock governing the controller hardware. + /// This describes the frequency of the clock governing the ONI controller. /// public uint SystemClockHz { get; private set; } @@ -119,8 +119,8 @@ private void Reset() /// Gets the acquisition clock rate in Hz. /// /// - /// This describes the frequency of the clock used to drive the acquisition counter which is used to provide the clock - /// counter values in and its derivative types (e.g. , + /// This describes the frequency of the clock used to drive the ONI controller's acquisition clock which is used + /// to generate the clock counter values in and its derivative types (e.g. , /// , etc.) /// public uint AcquisitionClockHz { get; private set; } @@ -144,7 +144,7 @@ private void Reset() public uint MaxWriteFrameSize { get; private set; } /// - /// Gets the device table containing the full device hierarchy governed by the internal . + /// Gets the device table containing the device hierarchy governed by the internal . /// /// /// This dictionary maps a fully-qualified to an instance. @@ -260,10 +260,10 @@ internal Task StartAsync(int blockReadSize, int blockWriteSize, CancellationToke ctx.BlockReadSize = blockReadSize; ctx.BlockWriteSize = blockWriteSize; - // NB: Stuff related to sync mode is 100% ONIX, not ONI, so long term another place - // to do this separation might be needed - int addr = ctx.HardwareAddress; - int mode = (addr & 0x00FF0000) >> 16; + // TODO: Stuff related to sync mode is 100% ONIX, not ONI. Therefore, in the long term, + // another place to do this separation might be needed + int address = ctx.HardwareAddress; + int mode = (address & 0x00FF0000) >> 16; if (mode == 0) // Standalone mode { ctx.Start(true); diff --git a/OpenEphys.Onix/DataFrame.cs b/OpenEphys.Onix/DataFrame.cs index e074b07b..5139e013 100644 --- a/OpenEphys.Onix/DataFrame.cs +++ b/OpenEphys.Onix/DataFrame.cs @@ -8,7 +8,7 @@ public abstract class DataFrame /// /// Initializes a new instance of the class. /// - /// System clock count. Generally provided by the underlying value. + /// Acquisition clock count. Generally provided by the underlying value. internal DataFrame(ulong clock) { Clock = clock; diff --git a/OpenEphys.Onix/DeviceFactory.cs b/OpenEphys.Onix/DeviceFactory.cs index 5ba151ab..77aa946d 100644 --- a/OpenEphys.Onix/DeviceFactory.cs +++ b/OpenEphys.Onix/DeviceFactory.cs @@ -48,7 +48,8 @@ internal SingleDeviceFactory(Type deviceType) /// /// The device name provides a unique, human-readable identifier that is used to link software /// elements for configuration, control, and data streaming to hardware. This is often a one-to-one - /// representation of an ONI device, but can also represent abstract ONI device aggregates or virtual devices. + /// representation of a single , but can also represent abstract ONI device + /// aggregates or virtual devices. /// [Description(DeviceNameDescription)] public string DeviceName { get; set; } @@ -58,7 +59,7 @@ internal SingleDeviceFactory(Type deviceType) /// /// /// This address provides a fully-qualified location of a device within the device table. This is often a one-to-one - /// representation of a ONI address, but can also represent abstract device addresses. + /// representation of a , but can also represent abstract device addresses. /// [Description(DeviceAddressDescription)] public uint DeviceAddress { get; set; } @@ -68,7 +69,7 @@ internal SingleDeviceFactory(Type deviceType) /// /// /// This type provides a device identity to each device within the device table. This is often a one-to-one - /// representation of a ONI device ID, but can also represent abstract device identities. + /// representation of a a , but can also represent abstract device identities. /// [Browsable(false)] public Type DeviceType { get; } diff --git a/OpenEphys.Onix/NeuropixelsV1eRegisterContext.cs b/OpenEphys.Onix/NeuropixelsV1eRegisterContext.cs index 10e44a21..8fc13de3 100644 --- a/OpenEphys.Onix/NeuropixelsV1eRegisterContext.cs +++ b/OpenEphys.Onix/NeuropixelsV1eRegisterContext.cs @@ -238,14 +238,10 @@ public void InitializeProbe() WriteByte(NeuropixelsV1e.OP_MODE, (uint)NeuropixelsV1OperationRegisterValues.RECORD); } - // TODO: There is an issue getting these SR write sequences to complete correctly. - // We have a suspicion it is due to the nature of the MCLK signal and that this - // headstage needs either a different oscillator with even more drive strength or - // a clock buffer (second might be easiest). public void WriteConfiguration() { - // shank - // NB: no read check because of ASIC bug + // shank configuration + // NB: no read check because of ASIC bug that is documented in IMEC-API comments var shankBytes = BitArrayToBytes(ShankConfig); WriteByte(NeuropixelsV1e.SR_LENGTH1, (uint)shankBytes.Length % 0x100); @@ -256,15 +252,14 @@ public void WriteConfiguration() WriteByte(NeuropixelsV1e.SR_CHAIN1, b); } - // base + // base configuration for (int i = 0; i < BaseConfigs.Length; i++) { var srAddress = i == 0 ? NeuropixelsV1e.SR_CHAIN2 : NeuropixelsV1e.SR_CHAIN3; for (int j = 0; j < 2; j++) { - // TODO: HACK HACK HACK - // If we do not do this, the ShiftRegisterSuccess check below will always fail + // WONTFIX: Without this reset, the ShiftRegisterSuccess check below will always fail // on whatever the second shift register write sequence regardless of order or // contents. Could be increased current draw during internal process causes MCLK // to droop and mess up internal state. Or that MCLK is just not good enough to @@ -293,8 +288,8 @@ public void WriteConfiguration() public void StartAcquisition() { - // TODO: Hack inside settings.WriteShiftRegisters() above puts probe in reset set that needs to be - // undone here + // WONTFIX: Soft reset inside settings.WriteShiftRegisters() above puts probe in reset set that + // needs to be undone here WriteByte(NeuropixelsV1e.OP_MODE, (uint)NeuropixelsV1OperationRegisterValues.RECORD); WriteByte(NeuropixelsV1e.REC_MOD, (uint)NeuropixelsV1RecordRegisterValues.ACTIVE); }