Conversation
Member
marknolan
commented
Apr 7, 2025
- Updated ExG channels for S3R ExG support
- removed S3R proto3 mini as there won't be any
- refactoring to remove "NewIMU" from names as that doesn't apply to S3R
Contributor
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (4)
ShimmerDriver/src/main/java/com/shimmerresearch/driver/Configuration.java:1349
- [nitpick] Ensure that renaming to remove 'NewImu' from S3R variables is applied consistently across all S3R definitions; for instance, verify if 'svoShimmer3RGsrNewImuLogAndStream' should also be renamed accordingly.
private static final ShimmerVerObject svoShimmer3RExgUnifiedLogAndStream = new ShimmerVerObject(HW_ID.SHIMMER_3R,FW_ID.LOGANDSTREAM,0,1,0,HW_ID_SR_CODES.EXP_BRD_EXG_UNIFIED, NEW_IMU_EXP_REV.EXG_UNIFIED);
ShimmerDriver/src/main/java/com/shimmerresearch/driver/Configuration.java:1372
- [nitpick] Confirm that the removal of 'NewImu' is intended here and consider whether the NEW_IMU_EXP_REV parameter should be updated to reflect the naming change.
private static final ShimmerVerObject svoShimmer3RBrAmpUnifiedLogAndStream = new ShimmerVerObject(HW_ID.SHIMMER_3R,FW_ID.LOGANDSTREAM,0,1,0,HW_ID_SR_CODES.EXP_BRD_BR_AMP_UNIFIED, NEW_IMU_EXP_REV.BRIDGE_AMP);
ShimmerDriver/src/main/java/com/shimmerresearch/driver/Configuration.java:1385
- [nitpick] Verify the updated variable name adheres to the overall naming conventions now that 'NewImu' has been removed, ensuring alignment with related S3R definitions.
private static final ShimmerVerObject svoShimmer3RProto3DeluxeLogAndStream =new ShimmerVerObject(HW_ID.SHIMMER_3R,FW_ID.LOGANDSTREAM,0,1,0,HW_ID_SR_CODES.EXP_BRD_PROTO3_DELUXE, NEW_IMU_EXP_REV.PROTO3_DELUXE);
ShimmerDriver/src/main/java/com/shimmerresearch/driver/Configuration.java:1410
- [nitpick] Confirm that the inclusion of 'svoShimmer3RExgUnifiedLogAndStream' in the ExG compatibility lists is consistent with the refactoring intent, and that all S3R-specific entries have been uniformly updated.
svoShimmerECGmd, svoShimmer4Stock, svoStrokare, svoShimmer3RExgUnifiedLogAndStream
JongChern
approved these changes
Apr 8, 2025
Collaborator
|
looks good |
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
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.