Streamline settings management via ezmsg-baseproc delegation#26
Merged
Conversation
and a few docstring changes
…tion to ezmsg-baseproc
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.
Summary
Adopts the new
update_settings/NONRESET_SETTINGS_FIELDSmachinery from ezmsg-baseproc#8 so this repo's units delegate live-update logic to their underlying processors instead of carrying it in unit-levelon_settingsoverrides.Net effect:
CerePlexImpedanceloses ~40 lines of bespoke override; live tweaks tofreq_lo/freq_hi/test_current_nAno longer churn through a full processor recreate; the offsets-only fast path that preserves the accumulated impedance array is unchanged behaviorally but now expressed as a 10-lineupdate_settingsoverride on the processor.Per-unit decisions
NONRESET_SETTINGS_FIELDSChannelMapUnitfrozenset()(default;channel_mapchange should reset)CerePlexImpedanceon_settingsoverride; add processorupdate_settingsfor the offsets-only fast path{"freq_lo", "freq_hi", "test_current_nA", "headstage_channel_offsets"}CereLinkSourceBaseProducer.update_settings{"cbtime", "microvolts"}(documentary;cont_buffer_durdeliberately excluded — consumed only insideopen())CerePlexImpedanceProcessorfreq_lo,freq_hi,test_current_nAare read live inextract_impedance(...)and never cached in state → safe to update without resetting.headstage_channel_offsetsrequires re-laying out trackers but the accumulatedstate.impedancearray remains valid;update_settingspatches trackers in place via_build_trackers(n_ch).collect_duration_sandfft_duration_sare baked intostateduring_reset_state(buffer sizes, FFT window length); they correctly fall through the default reset path.on_settingsoverride (and thedataclassesimport) are removed; the inherited base-class default now routes throughprocessor.update_settings(self.SETTINGS).CereLinkProducerNONRESET_SETTINGS_FIELDSadded but documentary — the producer is non-stateful andCereLinkSource.on_settingsoverrides the base flow entirely (it manages two sessions across an asyncclose → open, emitsDeviceStatus, and rolls backSETTINGSon failure). The constant records which fields are safe to change live for future maintainers.What's not changing
CereLinkSource.on_settings— kept verbatim. The pycbsdk Session lifecycle needsawait asyncio.to_thread, must avoid two simultaneous Sessions, and needs to publishDeviceStatus— none of which fits inside a syncupdate_settings(new_settings) -> None.ChannelMapProcessor— no code change. The new default behavior fixes a latent bug for free: today, swappingchannel_mapon a stream whosen_chis unchanged would not re-parse the CMP because_hash_messageonly hashesn_ch. Post-PR,update_settingsarms_hash = -1so_reset_statere-runs on the next message.Dependency
Requires
ezmsg-baseprocwith the newBaseProcessor.update_settings/BaseProducer.update_settingsand the unit-level delegation inBaseProducerUnit.on_settings/BaseProcessorUnit.on_settings. The pinned range needs to be bumped to whatever version ships ezmsg-org/ezmsg-baseproc#8 once it merges (currently using a local editable checkout for development).Test plan
pytest tests/ --ignore=tests/test_impedance_integration.py --ignore=tests/test_integration.py— 44/44 passruff check src/ tests/— cleantest_offsets_only_update_preserves_impedance— verifies theaccumulated impedance array survives an offsets-only update and trackers
are re-laid out
test_non_safe_field_arms_reset— verifiescollect_duration_schange arms a full reset (
_hash == -1)test_live_safe_field_does_not_reset— verifiesfreq_lo/freq_hi/test_current_nAupdates don't reset statethat toggles only
microvoltsshould now apply without a Session restart(was previously silently ignored on idle, applied no-op on live session)