-
Notifications
You must be signed in to change notification settings - Fork 1
Finalize Milestone 1.0f and 2.0e #62
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
Merged
Conversation
This file contains hidden or 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
- Remove unnecessary include statements
- Add Npx2e errors during initialization - Fix ++ vs. += 1 - Add calibration stream for Bno055
- Added field to probe settings for the probe contour, separately from the shank outline used for visualization in the GUI - Tested saving/loading ProbeInterface JSON files between the GUI and Bonsai, works well in both directions
- Due to the addition of the breakout board devices, the indices could pull the wrong number of headstages, leading to all connected headstages being removed even if one of them is valid
jonnew
requested changes
May 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My review is an integration test that is resulting in a bunch of new issues. I've reached a stopping point that needs to be resolved before continuing: #66
- All devices are now fully instantiated from the canvas, and when connecting to hardware this pointer is directly used. This fixes the issue of syncing states, since all references are to the same object - Modified how devices are configured; now these methods throw to make reporting failures easier. All calls are wrapped in a try-catch to avoid propagating these exceptions up to the level of the GUI - Various other refactors, such as swapping std::string for juce::String, removing unused methods, renaming headstages to hubs where appropriate, fixing whitespace
- Add information to error messages where needed - Show warning message boxes asynchronously using the message thread to avoid Juce assertions
jonnew
approved these changes
May 20, 2025
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 cleans up all of the remaining issues for the 1.0f and 2.0e Milestone. At this point, the plugin is in a mostly stable place for those two headstages and can be distributed for alpha testing.
Fixes #3
Fixes #34
Fixes #56
Fixes #61
Split out the calibration status into four different channels (mag / acc / gyr / sys) that go from 0 to 3 for simplicity. This ends up being less cumbersome to the end user, and potentially more useful for troubleshooting and debugging.
Fixes #63
When starting acquisition, specifically inside of the
isReady()
method, there was an issue that would block acquisition if a NPX 1.0f only had one probe enabled. This is now fixed by only checking for enabled devices in this method.Fixes #66
Refactored to look for Hub IDs instead of guessing which hubs are connected based on the discovered devices. Also fixed the passthrough indices issue, where the wrong index was being used for a device.
Fixes #70
Additionally, now the settings interfaces point to the same device object as what is created when connecting to the hardware, so there are no more synchronization issues with settings.
Fixes #37
Fixed and updated error messages so they are less confusing
Fixes #68
Fixes #66