Rework amplifier interfaces #2363
Open
Conversation
This file contains 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
Collaborator
t-b
commented
Mar 3, 2025
•
t-b
commented
- Have a look at AMPLIFIER_CONTROLS_VC/AMPLIFIER_CONTROLS_BUTTONS_VC, also for IC again
- Check CI
- DAEphys: Sync MIES to MCC on checkbox click #2360
We are currently only syncing MIES to MCC settings when changing the headstage or clamp mode. But we forgot to sync initially which is required if we don't change headstage or clamp mode. Bug present since forever.
This is needed on the initial MIES->MCC sync as we don't know if our internal state is correct or not.
* bugfix/2360-mies-mcc-sync-checkbox: AI_SyncAmpStorageToGUI: Allow forcing to write all values DAEphys: Sync MIES to MCC on checkbox click
In that way we can reuse that string a later commit.
This is for a future commit where we also need the clamp mode.
t-b
force-pushed
the
feature/2363-rework-amplifier-interfaces
branch
2 times, most recently
from
773e92d to
be48333
Compare
The default values for whole cell resistance and capacitance are not valid. Change the defaults to 1% instead and also add limits for resistance.
t-b
force-pushed
the
feature/2363-rework-amplifier-interfaces
branch
from
be48333 to
b928b73
Compare
8 tasks
The current approach uses AI_SendToAmp with separate getter and setter constants. For setting people should
also prefer AI_UpdateAmpModel but as both functions are non-static it can happen that the wrong function is
called.
We have now reworked the interfaces with the following outcome:
- One public function to read/write amplifier settings (AI_WriteToAmplifier, AI_ReadFromAmplifier). These functions
also do the right thing out of the box.
- Deduplicated getter/setter constants to only denote the type of setting. The access type, read or write,
is now passed in separately.
- Public accessible functions now only accept mcc function constants
- Introduce low level AI_ReadFromMCC/AI_WriteToMCC functions which make AI_SendToAmp remarkable shorter
- Introduce various helper functions which translate between control names, function constants and human
readable names.
t-b
force-pushed
the
feature/2363-rework-amplifier-interfaces
branch
from
b928b73 to
ac6a2f7
Compare
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.