Skip to content

Conversation

bparks13
Copy link
Member

Instead of requiring the user to manually select the calibration files for each new probe, this PR adds a UI element to both Neuropixels 1.0 and 2.0 probe settings tabs that can automatically scan a folder for probe calibration files. The file has to be chosen by the user, but it can exist at an arbitrary location.

Any time one of the following conditions is true, the automatic scan will occur, and will attempt to find one and only one calibration file matching the given serial number:

  • A new folder is chosen, and the probe serial number exists
  • When a new probe serial number is found, and the folder already exists
  • When the checkbox to automatically search through the folder is checked, if the folder is selected and a serial number exists

Folder paths and toggle state are saved in the XML. All UI elements are disabled during acquisition.

This PR also adds a couple small commits related to optimizing the loading of the XML file (1307ab1) and only calling an alert window asynchronously to avoid conflicts with the message thread (615f6d8) that came up during testing.

bparks13 added 3 commits July 28, 2025 14:32
- This solves the issue of a Neuropixels 2.0 probe causing up to four saves of the data into the same file, causing redundant writes
@bparks13 bparks13 added this to the 0.1.1 milestone Jul 28, 2025
@bparks13 bparks13 requested a review from jonnew July 28, 2025 18:39
@bparks13 bparks13 self-assigned this Jul 28, 2025
@bparks13
Copy link
Member Author

Neuropixels 2.0:
image

Neuropixels 1.0:
image

View of the checkbox toggled off:
image

@bparks13 bparks13 requested a review from cjsha July 29, 2025 18:16
@cjsha
Copy link
Collaborator

cjsha commented Jul 29, 2025

Without looking much at the code that implements these changes, here are my comments on the UI:


image

The above error message indicates that I should be able to see all the found calibration files in the log files. However, when I selected a folder that contains a non-matching calibration file, it doesn't show up in the logs.

[2025-07-29T17:24:51.925][open-ephys][action] Button '...' clicked
[2025-07-29T17:25:00.641][onix-source][debug] Expected to find 1 file matching '19051024281_gainCalValues.csv', but found 0 instead. Check logs for all files discovered.
[2025-07-29T17:25:00.652][onix-source][debug] Expected to find 1 file matching '19051024281_ADCCalibration.csv', but found 0 instead. Check logs for all files discovered.
[2025-07-29T17:25:03.480][open-ephys][action] Button 'OK' clicked
[2025-07-29T17:25:07.160][open-ephys][action] Button 'OK' clicked

I propose that the calibration file is checked for validity when the acquisition button is pressed and the probe is enabled instead of whenever disable/enable is pressed.


When I set the Calibration folder to a folder which doesn't contain any calibration files, I can start acquisition without any issue. Is it because it falls back onto the manually set ADC and Gain calibration files? Or should there be some kind of error here? This behavior is kinda confusing to me.

I propose that the "ADC Calibration File" & "Gain Calibration File" fields grey out when the "Search for calibration files automatically" checkbox is checked. Also, it seems searchDirectoryForCalibrationFile checks all directories recursively. I'm guessing that's what the other plugins do, but I have mixed feelings about that. Anyway, that makes more sense of why the error message is "Wrong Number of Calibration Files". I was thinking there can only be 1 or 0 files that have matching names, but that isn't necessarily the case when you're searching through multiple folders.


When I press enable/disable, I get four error prompts when I would expect two:
calibration-dialogs
I think I did something to make the behavior happen because it seems like it doesn't happen all the time. With that said, once I started getting four error prompts, I thereafter got those four error prompts every time. Maybe that's what happens if I set the calibration files and then changed the directory name.


This tooltip:
image

My beef in particular the original wording is that it's not just for gain calibration files, it's also for adc calibration folders. But I also wonder if something like this is more clear:
"Open a file dialog to choose a folder that contains your calibration files. The calibration file(s) that matches your probe will automatically be selected if it exists in this folder."

cjsha
cjsha previously requested changes Jul 29, 2025
Copy link
Collaborator

@cjsha cjsha left a comment

Choose a reason for hiding this comment

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

I left my review in an extended comment in this PR

@bparks13
Copy link
Member Author

@cjsha

The above error message indicates that I should be able to see all the found calibration files in the log files. However, when I selected a folder that contains a non-matching calibration file, it doesn't show up in the logs.

Good point, I will modify the language so that it only says the extra files are in the logs if there are more than 1 found.

When I set the Calibration folder to a folder which doesn't contain any calibration files, I can start acquisition without any issue. Is it because it falls back onto the manually set ADC and Gain calibration files?

That is correct, whatever files are listed in the ADC/Gain calibration text boxes are the currently selected ones, and if no files are found automatically then the previously selected files are not removed. @jonnew What should we do in this instance?

A side note, acquisition cannot start if the probe serial number found on the hardware does not match the serial number embedded in the calibration file, so there is a check already in place if the files are expected to change but do not, say for example if a different headstage was plugged in with a different probe.

When I press enable/disable, I get four error prompts when I would expect two:

This was an error on my part, the method got called twice when the device was toggled.

My beef in particular the original wording is that it's not just for gain calibration files, it's also for adc calibration folders.

I agree, this is a good change to make it more general.

I propose that the "ADC Calibration File" & "Gain Calibration File" fields grey out when the "Search for calibration files automatically" checkbox is checked.

I also agree with this, the check box grays out the folder if it is not checked, so the inverse should be true for manual selection.

Regarding the other part of this comment, where it searches recursively, this is the behavior of the other Neuropixels plugins. I believe it makes sense to have a single parent folder selected that contains all calibration files so that the user only has to hit Connect and it will automatically find the correct file even if multiple probes are connected throughout the day. Of course, the user has the ability to select whichever folder they want.

I propose that the calibration file is checked for validity when the acquisition button is pressed and the probe is enabled instead of whenever disable/enable is pressed.

I think it would be more appropriate to only search if the device is enabled; this would apply to all other triggers, so if a probe is connected but it is disabled then no search occurs, or if a new folder is selected then nothing would happen. Then, if the correct folder is selected, and the user toggles the device On, then it will immediately search for the correct calibration file(s).

@jonnew Do you agree with this implementation?

@bparks13 bparks13 modified the milestones: 0.2.1, 0.2.0 Aug 8, 2025
- Updated tooltips and error messages
- Only update the info string once per toggle
- If the automated search checkbox is checked, gray out the individual calibration file buttons
- Only search for calibration files when the device is enabled and any other trigger is hit
- Limit recursive search for calibration files to two folders from the root
- Change some log statements to console instead of debug so users can get real-time feedback
@bparks13 bparks13 dismissed cjsha’s stale review August 8, 2025 15:04

Addressed comments

@bparks13 bparks13 merged commit 8de7fef into main Aug 8, 2025
3 checks passed
@bparks13 bparks13 deleted the issue-134 branch August 8, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automated calibration file detection

3 participants