Skip to content

Conversation

bparks13
Copy link
Member

@bparks13 bparks13 commented May 28, 2025

This PR adds the Neuropixels 1.0e headstage as an available option to be selected in the drop-down menu for headstages.

Fixes #55
Fixes #65 (add labels to differentiate 1.0f and 1.0e headstages)

@bparks13 bparks13 added this to the Neuropixels 1.0e milestone May 28, 2025
@bparks13 bparks13 self-assigned this May 28, 2025
@bparks13 bparks13 marked this pull request as ready for review June 2, 2025 21:21
@bparks13 bparks13 marked this pull request as draft June 2, 2025 21:22
@bparks13 bparks13 marked this pull request as ready for review June 5, 2025 18:44
Copy link
Collaborator

@aacuevas aacuevas left a comment

Choose a reason for hiding this comment

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

Please review the units.
In different places they might appear as
µV, uV or �V (the latter being a non-ascii code)

Also, since this PR is changing to std::string objects, which are encoding-agnostic, it would be interesting to know what the GUI drawing methods use, because the "micro" sign is not standard ASCII, and thus it has a different encoding in UTF-8 or ASCII, so using a different encoding from what the GUI uses to print might result on display issues.

Juce::String hides this, since it uses utf-8 internally by default and has methods to convert between encodings, so this might be a little tricky as care needs to be taken manually. This is probably one of the reasons Juce::String is still part of the library, as it does offer extended functionality not present on std::string (unlike ScopedPointer, which became obsolete with C++11's std::unique_ptr and other smart pointers)

@bparks13 bparks13 requested a review from aacuevas June 17, 2025 14:19
Copy link
Collaborator

@aacuevas aacuevas left a comment

Choose a reason for hiding this comment

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

Seems good

@bparks13 bparks13 merged commit 5907dee into main Jun 17, 2025
@bparks13 bparks13 deleted the issue-55 branch June 17, 2025 17:26
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.

Distiguish headstages in dropdown and headstage not found confusion Add Neuropixels 1.0e Headstage

2 participants