Config setter ergonomics: auto-sync, n_configured, clear_channel_map, async runlevel#182
Merged
Config setter ergonomics: auto-sync, n_configured, clear_channel_map, async runlevel#182
Conversation
Per-channel C-API setters that emit CHANSET-family packets are read-modify-write: they seed the outgoing CHANINFO from the local chaninfo cache. When a prior config command sent by this process is still in flight, the seed can be stale and the setter clobbers it back to the old value. Add a trailing `int auto_sync` parameter to the affected setters; when non-zero, run an internal `sync()` first so the cache reflects all prior in-flight CHANREP packets. set_channel_spkthrlevel stays unchanged because CHANSETSPKTHR is "narrow" — the firmware reads only spkthrlevel and the setter overwrites it fully. Also fix two long-standing packet-type bugs flagged while auditing the firmware-side handlers: - set_channel_spkopts emitted CHANSETSPKTHR; the firmware ignores spkopts on that type and only reads spkthrlevel, so the change was silently dropped. Use CHANSETSPK (firmware reads spkopts+spkfilter). - set_channel_spike_sorting had the same bug. Same fix. The existing call sites in tests/integration/test_capi_configuration.cpp are updated to pass auto_sync=0 explicitly and replace the post-call sleep with an explicit sync() (faster + race-free). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructure setSampleGroup, setACInputCoupling, setSpikeSorting, and setSpikeExtraction so callers no longer have to manually sync() before reading state back, and so they get the post-config count of channels matching the requested configuration. For each of the four bulk by-type setters in SdkSession: - Run sync() at entry so the local cache reflects any prior in-flight config from this process before we seed the outgoing CHANSET* packets (the #177 contract for broadly-scoped setters). - Always send a CHANSET* packet for every in-scope channel, even if the cached value already looks correct. A dropped CHANREP packet from another client would otherwise leave the local cache stuck against a concurrent change with no escape path other than re-sending. - Run sync() at exit so the device has applied our changes before we count. - Re-scan chaninfo to count channels of the requested type whose post-config state matches the request, and return that count. - Change the C++ return type from Result<void> to Result<uint32_t>; change nChans from size_t to uint32_t with UINT32_MAX as the "all matching" sentinel. Mirror through: - C-API: trailing `uint32_t* out_n_configured` out-param on the four cbsdk_session_set_* entrypoints; nChans becomes uint32_t. - pycbsdk: rename `n_chans` to `chans`, accept `int | None` (None → UINT32_MAX), return the count. Drop the "Call sync() before reading back state" docstring blocks since the methods are now self-syncing. Also fixes the STANDALONE-mode CLIENT-of-this-bug companion to commit 2708ca6: DeviceSession::setChannelsSpikeSortingByType emitted CHANSETSPKTHR while modifying spkopts; the firmware ignored those changes. Switch to CHANSET (firmware applies all chaninfo fields). The matching setChannelsSpikeSortingSync waits for CHANREP instead of CHANREPSPKTHR. Tests: - New TestNConfigured class in pycbsdk/tests/test_configuration.py covers chans=None ↔ all-matching and the n_configured return on set_sample_group, set_ac_input_coupling, and set_spike_extraction. - C-API test sites in tests/integration/test_capi_configuration.cpp pass &n_configured and assert the count; the now-redundant sleeps are dropped (sync is internal). - tests/unit/test_cbsdk_c_api.cpp signature update. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After load_channel_map() began accumulating overlays (one CMP per
headstage, multiple headstages per device), there was no way to clear
them. Loading "" was the only undo path and was non-obvious.
Add clear_channel_map() at every layer:
- SdkSession::clearChannelMap(): snapshot the chan_ids in
cmp_entries, drop the overlay map, zero any local position state in
shmem, and push CHANSETLABEL packets with default "chan{N}" labels
to the device for every previously-mapped channel so the device-side
label state matches. Fire-and-forget.
- C-API: cbsdk_session_clear_channel_map.
- pycbsdk: Session.clear_channel_map.
Tests in pycbsdk/tests/test_configuration.py (TestCMP) cover label
revert, position revert, and the no-op-with-no-map case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Async-aware applications (e.g. ezmsg-blackrock) need to wait for the device to reach cbRUNLEVEL_RUNNING before issuing config calls, but the existing primitive — polling session.running — is the SDK lifecycle flag, not the device runlevel. Add a proper runlevel-transition callback at the C++ level and a thread-bridged asyncio awaitable on top of it in pycbsdk. C++ (SdkSession): - New RunlevelCallback type and registerRunlevelChangeCallback(). - New runlevel_callbacks vector in Impl, plus updateRunlevel() helper that atomically swaps device_runlevel and fires registered callbacks on transition. Both SYSREP-handling sites (STANDALONE receive thread and CLIENT shmem-receive thread) now route through it. - unregisterCallback() extends to the new vector. C-API: - New cbsdk_runlevel_callback_fn typedef and cbsdk_session_register_runlevel_callback() entrypoint. pycbsdk: - _register_runlevel_callback() helper mirroring _register_config_callback (FFI callback + handle/ref tracking). - async wait_for_runlevel(level, timeout) — returns immediately if already at/past `level`, otherwise registers a one-shot listener and resolves a thread-bridged future via loop.call_soon_threadsafe. - async wait_until_running(timeout=10.0) — thin wrapper over wait_for_runlevel(RUNLEVEL_RUNNING). - Module-level RUNLEVEL_* constants mirroring cbRUNLEVEL_* in cbproto/types.h. TestWaitUntilRunning in pycbsdk/tests/test_configuration.py covers the immediate-return path; the registration path requires fault injection that isn't worth the test complexity here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extend the four bulk by-type setters (set_sample_group, set_ac_input_coupling, set_spike_sorting, set_spike_extraction) so callers can pass an explicit list of 1-based channel IDs in addition to the existing "first N matching" / "all matching" modes. This makes disjoint-set configurations possible — e.g. configuring chans 1-32 to one rate and chans 33-64 to another in two calls — without each call having to manually iterate. C++ (SdkSession): trailing `const uint32_t* chans = nullptr` parameter on each of the four setters. When non-null, the setter configures exactly the listed chan IDs (no chan_type filter on the listed entries; caller is trusted). When null, behavior is unchanged. `chan_type` still defines the "others" set for disable_others and the post-config count. Implementation also restructures the four setters around a shared sendBulkPackets path (now a public SdkSession method that picks the most efficient transport: device_session->sendPackets in STANDALONE for direct UDP with built-in pacing, per-packet shmem enqueue in CLIENT). This fixes a latent ordering issue introduced by an earlier draft of #181 where STANDALONE-mode bulk sends went through shmem while sync()'s SYSSETRUNLEV went via direct UDP, so the SYSREP barrier could fire ahead of queued CHANSET packets. C-API: trailing `const uint32_t* chans` argument on each of the four cbsdk_session_set_* entrypoints (positioned right after `n_chans`). pycbsdk: `chans` accepts `int | list[int] | Iterable[int] | None`. A new private `_normalize_chans` helper converts the Python value to (n_chans, ffi-uint32-array) for the C-API. bool is rejected explicitly. Tests: new TestBulkSettersChanList covers disjoint ranges, disjoint non-contiguous sets, disable_others combined with a list, explicit-list paths through the other three setters, the empty-list edge case, and that tuple/range/etc. are accepted alongside list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CApiACCouplingTest and CApiSpikeSortingTest tests asserted that n_configured == 4 after configuring the first 4 FE channels. Per issue #181 the returned count is "FE chans whose post-config state matches the request", not "chans we just touched" — and the device defaults all FE chans to AC-coupled with no sorting, so the 4-chan no-op produced n_configured == n_fe (== 256 on CI), failing the assertion. Fix by establishing the inverse state across all FE chans first (DC for the AC test, AC for the DC test, HOOPSORT for the spike-sort test), so the 4-chan partial config genuinely produces exactly 4 chans matching the request. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the call sequence reported as hanging indefinitely in ezmsg-blackrock integration tests: session.set_sample_group(2, FRONTEND, SR_30kHz, disable_others=True) Passes locally against vanilla nplay+pycbsdk (3 s), so a hang here is not reproducible from the SDK alone — likely environmental (callback interactions, threading, or an upstream call sequence). Keeping the test in tree so any future regression in the bulk setter's sync barriers is caught. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the post-sync + count-rescan that #181 added to the four bulk by-type setters. The post-sync was structurally tied to the count return — re-scanning local chaninfo for "post-config matches" only makes sense if all CHANREPs have arrived first. Without the count, the post-sync has no purpose, and a second sync per call is exactly the kind of response-bound wait we want to minimize: a dropped SYSREPRUNLEV would stall the call for 5 seconds, and back-to-back config calls double-pay the round-trip. What stays: - Top sync (#177 contract) — ensures local cache is fresh before we seed CHANSET* packets from it; required for correctness. - chans=None | int | list[int] (the #181 follow-up) — orthogonal to the count, retains the disjoint-set-of-channels API. - "Always send to every in-scope channel" idempotency — also orthogonal. C++: bulk setters return Result<void> instead of Result<uint32_t>. applyBulkSetter helper drops its predicate parameter. countChannelsMatching is removed (it was only used here). C-API: drop the `uint32_t* out_n_configured` out-param from the four cbsdk_session_set_* entrypoints. pycbsdk: methods return None; drop the int-return type and the out_n ffi.new buffer. Tests: TestNConfigured class removed entirely. TestBulkSettersChanList tests rewritten to verify state via get_group_channels (with explicit sync()) instead of asserting on the dropped count. CApi tests simplified back to smoke tests after the count assertion was removed. The ezmsg-blackrock repro test (test_disable_others_30k_no_preamble) calls sync() explicitly before reading state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 29, 2026
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
1 participant
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
Closes #177, #178, #181 (with side fix to a CHANSETSPKTHR bug); resolves #179. (#180 was already covered by
register_config_callbackand is closed.)Four commits, in order:
auto_syncopt-in. The 9 broadly-scoped setters now take a trailingint auto_sync(C-API) /auto_sync: bool = False(pycbsdk) flag; when true they runsync()first so the local cache reflects in-flight CHANREPs before the read-modify-write.set_channel_spkthrlevelstays narrow. Bundles two firmware-mismatch bug fixes:set_channel_spkoptsandset_channel_spike_sortingwere emittingCHANSETSPKTHR, which the firmware ignoresspkoptson — switched toCHANSETSPK.n_configuredpost-sync;chans=None(UINT32_MAXin C) means "all matching". Always sends to every in-scope channel — no skip-if-already-correct, since a dropped CHANREP could leave the local cache stuck against a concurrent change. Includes the matching CHANSETSPKTHR→CHANSET fix in DeviceSession (STANDALONE path).clear_channel_map()to undoload_channel_map(). Drops the local position+label overlay and pushes defaultchan{N}labels to the device.registerRunlevelChangeCallbackin C++/C-API +async session.wait_until_running()/await session.wait_for_runlevel(level)in pycbsdk. Resolves once the device-reported runlevel reaches the target; thread-bridged vialoop.call_soon_threadsafe. Returns immediately if already past target.Test plan
ctest -E '^(device\.|integration\.)')TestNConfigured,TestWaitUntilRunning, and 3 newclear_channel_mapcases inTestCMP)device.*,integration.*) — not run locally; needs an NSP/HUB or nplay-on-CIawait session.wait_until_running()andset_sample_group(chans=None)shapeBreaking API changes
C-API and pycbsdk both:
auto_syncparameter (callers must pass0/Falseto keep prior behavior).set_sample_group,set_ac_input_coupling,set_spike_sorting,set_spike_extraction): signature change —n_chansbecomeschansacceptingint | None, return type changes from result-only toint(n_configured).🤖 Generated with Claude Code