Skip to content

Refactor Trigger operator to more clearly reflect SpikeGLX API#9

Merged
glopesdev merged 5 commits intobonsai-rx:mainfrom
glopesdev:trigger-properties
Nov 21, 2025
Merged

Refactor Trigger operator to more clearly reflect SpikeGLX API#9
glopesdev merged 5 commits intobonsai-rx:mainfrom
glopesdev:trigger-properties

Conversation

@glopesdev
Copy link
Member

@glopesdev glopesdev commented Nov 20, 2025

The current Trigger operator mirrors the TriggerGT function in the SpikeGLX API, which explicitly controls file writing behavior.

To avoid ambiguity and confusion with the general word "Trigger", and the complex documentation of this function, here we rename this operator to match the original function name.

Furthermore, the Gate and Trigger values are exposed directly as properties in the operator, with corresponding named values, both of which get surfaced to the corresponding documentation pages.

Finally, the command server properties were grouped to make it easier to navigate each of the operator-specific properties.

Using the same function name makes identification easier. The arguments
are also exposed as properties for improved readability.
@glopesdev glopesdev requested a review from J-M-White November 20, 2025 20:53
@glopesdev glopesdev added documentation Improvements or additions to documentation feature New planned feature proposal Request for a new feature labels Nov 20, 2025
Copy link
Collaborator

@J-M-White J-M-White left a comment

Choose a reason for hiding this comment

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

Thanks, @glopesdev, this is great! TriggerGT has a much more elegant interface than Trigger did.

I've just added two comments about the documentation/example workflow.

@glopesdev glopesdev requested a review from J-M-White November 21, 2025 14:45
@glopesdev
Copy link
Member Author

glopesdev commented Nov 21, 2025

@J-M-White I've added the empty overload and redid the examples, I will need review acceptance to merge this (we can change this in the future to require only conversation resolution which is slightly less onerous and still allows people to flag issues when needed).

Would be great also if you had a chance to try this out with SpikeGLX to see if all the examples work, etc, as I don't have access to a testing rig with actual hardware at the moment.

@J-M-White
Copy link
Collaborator

@glopesdev, great - thanks! I'll go and test the examples on one of our rigs today (shouldn't take too long).

@J-M-White
Copy link
Collaborator

@glopesdev, I'm glad I went and tested on the rig!

The Fetch and TriggerGT examples work perfectly; however, it looks like in addition to the newer versions of the SpikeGLX API replacing sglx_setDigitalOut with sglx_ni_DO_set and sglx_obx_AO_set (which I was aware of), recent versions of SpikeGLX itself no longer accept calls to sglx_setDigitalOut. This means that DigitalOutput doesn't work with the current SpikeGLX release.

I hadn't noticed this sooner because we never use DigitalOutput (SpikeGLX) and just use DigitalOutput (DAQmx) instead in all of our workflows.

I could see three ways of moving forward:

  1. Release the package as is, but highlight which versions of SpikeGLX work with DigitalOutput in the documentation.
  2. Remove DigitalOutput for now, release, and then plan to re-implement it with the new API functions in future.
  3. Update how DigitalOutput works prior to the first release.

What do you think?

@glopesdev
Copy link
Member Author

@J-M-White thanks for testing! I guess 3. would be nice if it's not too much work, otherwise I would go with 2.

In any case, I think we should do this in a separate PR as this is separate from the TriggerGT functionality. If you approve this PR we can merge and create a new one for that (I also noticed a few other details we might want to consider that I will raise in issues).

@glopesdev glopesdev merged commit eeb6631 into bonsai-rx:main Nov 21, 2025
10 checks passed
@glopesdev glopesdev deleted the trigger-properties branch November 21, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation feature New planned feature proposal Request for a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants