Add FFI Functions for Clamp Control#2559
Conversation
|
Let's do your usual appraoch here where Luke/Tim can test the FFI functions before we write the tests. |
2f8e923 to
d5bf06c
Compare
|
If the current clamp mode for the headstage in MIES is different from the specific calmp mode of an amplifier setting (e.g. auto bridge balance that applies only to IC) then it is ignored by MIES. |
|
I gave this a try today:
|
That makes sense for any of the TriggerAuto* functions, but all other parameters I would expect to be settable even when the mode is not active (for example, I should be able to set the auto bias parameters before switching to IC) |
|
I think this is on @MichaelHuth plate again. |
|
@MichaelHuth I'm also taking over this one. |
d5bf06c to
93afc3b
Compare
There was a problem hiding this comment.
Pull request overview
Adds new Foreign Function Interface (FFI) entry points intended for external (ACQ4/ZeroMQ) control/inspection of amplifier clamp settings, addressing issue #2547 by avoiding direct GUI element access.
Changes:
- Added FFI functions to query clamp state and to set/trigger several clamp-related controls (mode, holding potential, bias current, auto bias, headstage active, and auto clamp actions).
- Introduced
DAP_SetClampModeas a shared helper and wired the clamp-mode checkbox handler to use it. - Exposed
AI_AmpStorageControlToRowLabelfor cross-module use and added a range assertion for the AutoBias Vcom parameter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Packages/MIES/MIES_ForeignFunctionInterface.ipf | Adds clamp-control-related FFI functions, including FFI_GetClampState and several setters/triggers. |
| Packages/MIES/MIES_DAEphys.ipf | Refactors clamp-mode setting into DAP_SetClampMode and uses it from the GUI checkbox handler. |
| Packages/MIES/MIES_AmplifierInteraction.ipf | Adds AutoBias Vcom range assertion and makes AI_AmpStorageControlToRowLabel non-static for use by FFI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
93afc3b to
f2c45fb
Compare
f2c45fb to
c4e9cc9
Compare
c4e9cc9 to
ddddce4
Compare
ddddce4 to
3b13ea8
Compare
3b13ea8 to
984406a
Compare
|
@campagnola I changed the following:
|
|
@campagnola Friendly ping. |
this was previously done directly in the GUI event handler
In MIES the range is limited through the corresponding GUi element. However, when changed externally through a ZeroMQ function call then the range was not checked.
Added the following functions: FFI_GetCurrentClampState returns a wave with the current clamp control values FFI_GetClampState returns a wave with the all clamp control values FFI_SetClampMode FFI_SetHoldingPotential - allowed only in VC mode FFI_SetBiasCurrent - allowed only in IC mode FFI_SetAutoBias - allowed only in IC mode FFI_SetHeadstageActive - allowed only when no DAQ is running FFI_GetHeadstageActive - returns 1 is the given headstage is active, 0 otherwise FFI_TriggerAutoClampControl - wraps Pipette, Capacitance and Bridge Balance auto setting in a single call
984406a to
17f5faf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
Packages/MIES/MIES_ForeignFunctionInterface.ipf:786
FFI_GetClampStatesays it returns a null wave for an "invalid" headstage, but invalid indices currently ASSERT inFFI_CheckValidDeviceAndHeadstage. Please align the API contract (either return null for invalid indices, or adjust docs to state that invalid input raises an error).
/// @param[in] device Device title, e.g. "Dev1"
/// @param[in] headstage headstage number
/// @returns wave with full clamp state, null wave for invalid/inactive headstage
Function/WAVE FFI_GetClampState(string device, variable headstage)
Packages/MIES/MIES_ForeignFunctionInterface.ipf:932
FFI_SetAutoBiascurrently forwardspotentialto the amplifier without validating it is finite and within the documented range (-99..99 mV). Adding explicit checks here provides a clearer FFI-level error (and avoids relying on downstream assertions).
clampMode = DAG_GetHeadstageMode(device, headstage)
ASSERT(clampMode == I_CLAMP_MODE, "Attempt to set auto bias but current clamp mode is not IC !")
Packages/MIES/MIES_ForeignFunctionInterface.ipf:618
- The VC documentation table for
FFI_GetCurrentClampStatelistsWholeCellCaptwice (once in pF and again in a.u.), but the implementation filters out button-only controls viaAI_AmpStorageControlToRowLabeland does not return an a.u. entry here. This duplicate label/unit is misleading for FFI consumers relying on dimension labels.
/// | PipetteOffsetVC | | mV |
/// +------------------------+-----------------------+---------+
/// | WholeCellCap | | a.u. |
/// +------------------------+-----------------------+---------+
| /// @param[in] device Device title, e.g. "Dev1" | ||
| /// @param[in] headstage headstage number | ||
| /// @returns wave with clamp state, null wave for invalid/inactive headstage | ||
| Function/WAVE FFI_GetCurrentClampState(string device, variable headstage) |
| /// @param[in] device Device title, e.g. "Dev1" | ||
| /// @param[in] headstage headstage number | ||
| /// @param[in] potential holding potential in mV when current clamp mode of headstage is VC, bias current in pA when current clamp mode of headstage is IC | ||
| /// @param[in] enable when set to 1, enables Holding Potential after setting its value, when set to 0, disables Holding Potential before setting its values | ||
| Function FFI_SetHoldingPotential(string device, variable headstage, variable potential, variable enable) |
|
|
||
| static Function FFI_CheckValidDeviceAndHeadstage(string device, variable headstage) | ||
|
|
||
| ASSERT(!DAP_DeviceIsUnlocked(device), "Target device is not locked:" + device) |
| Function DAP_SetClampMode(string device, variable headstage, variable mode) | ||
|
|
||
| NVAR dataAcqRunMode = $GetDataAcqRunMode(device) | ||
| if(dataAcqRunMode == DAQ_NOT_RUNNING) | ||
| DAP_ChangeHeadStageMode(device, mode, headstage, DO_MCC_MIES_SYNCING) |
|
@MichaelHuth After chatting with Tim we agreed to add some tests and then merge. |
close #2547