Skip to content

stimulator: Add combined calibration function#237

Merged
BrianJKoopman merged 29 commits intomainfrom
yseino/stimulator-new_func
Oct 15, 2025
Merged

stimulator: Add combined calibration function#237
BrianJKoopman merged 29 commits intomainfrom
yseino/stimulator-new_func

Conversation

@YudaiSeino
Copy link
Copy Markdown
Contributor

I made a new function for stimulator calibration. The new function takes data for gain and time-constant with one stream.

@BrianJKoopman BrianJKoopman changed the base branch from main to yseino/stimulator-update September 26, 2025 18:53
Base automatically changed from yseino/stimulator-update to main September 26, 2025 23:42
Copy link
Copy Markdown
Member

@kmharrington kmharrington left a comment

Choose a reason for hiding this comment

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

This looks good to me

@YudaiSeino
Copy link
Copy Markdown
Contributor Author

I fixed one wrong parameter name. Fixed code worked using Nextline.

Copy link
Copy Markdown
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

This looks good to me, though I think do_setup needs to get dropped from the args list.

We also need a test for this, which is currently what's failing in the checks list. Please see the contributing guide for details on how to install and then the main readme for how to run the tests locally (the -e on install is needed for local coverage reporting to work). Then add a test in test_stimulator.py. The other existing tests should provide a good example to get started.

Comment thread src/sorunlib/stimulator.py Outdated
@YudaiSeino
Copy link
Copy Markdown
Contributor Author

Sorry I forgot to remove it. I just removed do_setup. I'll work on test_stimulator.py next.

Copy link
Copy Markdown
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

I added a test for the new function, feel free to PR any improvements separately.

@BrianJKoopman BrianJKoopman merged commit 939345b into main Oct 15, 2025
6 checks passed
@BrianJKoopman BrianJKoopman deleted the yseino/stimulator-new_func branch October 15, 2025 18:21
@BrianJKoopman BrianJKoopman changed the title Yseino/stimulator new func stimulator: Add combined calibration function Oct 15, 2025
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.

3 participants