From 2708ca623c1027cf5503ce77f48c3181c4cdc4d2 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 27 Apr 2026 17:41:32 -0400 Subject: [PATCH 1/8] Add auto_sync opt-in to per-channel CHANSET setters (#177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pycbsdk/src/pycbsdk/_cdef.py | 24 +++-- pycbsdk/src/pycbsdk/session.py | 102 +++++++++++++----- src/cbsdk/include/cbsdk/cbsdk.h | 51 +++++++-- src/cbsdk/src/cbsdk.cpp | 68 ++++++++---- tests/integration/test_capi_configuration.cpp | 12 +-- 5 files changed, 184 insertions(+), 73 deletions(-) diff --git a/pycbsdk/src/pycbsdk/_cdef.py b/pycbsdk/src/pycbsdk/_cdef.py index c13bcfd5..09330147 100644 --- a/pycbsdk/src/pycbsdk/_cdef.py +++ b/pycbsdk/src/pycbsdk/_cdef.py @@ -226,25 +226,29 @@ cbsdk_result_t cbsdk_session_get_channel_scaling( cbsdk_session_t session, uint32_t chan_id, cbsdk_channel_scaling_t* scaling); -// Per-channel setters +// Per-channel setters. +// `auto_sync` is non-zero to run an internal sync() before the read-modify-write +// (so any prior in-flight config from this process has landed in the cache); +// set_channel_spkthrlevel is "narrow" (CHANSETSPKTHR overwrites the only field +// the firmware reads) and so does not need an auto_sync flag. cbsdk_result_t cbsdk_session_set_channel_label(cbsdk_session_t session, - uint32_t chan_id, const char* label); + uint32_t chan_id, const char* label, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_smpgroup(cbsdk_session_t session, - uint32_t chan_id, cbproto_group_rate_t rate); + uint32_t chan_id, cbproto_group_rate_t rate, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_smpfilter(cbsdk_session_t session, - uint32_t chan_id, uint32_t filter_id); + uint32_t chan_id, uint32_t filter_id, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_spkfilter(cbsdk_session_t session, - uint32_t chan_id, uint32_t filter_id); + uint32_t chan_id, uint32_t filter_id, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_ainpopts(cbsdk_session_t session, - uint32_t chan_id, uint32_t ainpopts); + uint32_t chan_id, uint32_t ainpopts, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_lncrate(cbsdk_session_t session, - uint32_t chan_id, uint32_t lncrate); + uint32_t chan_id, uint32_t lncrate, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_spkopts(cbsdk_session_t session, - uint32_t chan_id, uint32_t spkopts); + uint32_t chan_id, uint32_t spkopts, int auto_sync); cbsdk_result_t cbsdk_session_set_channel_spkthrlevel(cbsdk_session_t session, uint32_t chan_id, int32_t level); cbsdk_result_t cbsdk_session_set_channel_autothreshold(cbsdk_session_t session, - uint32_t chan_id, _Bool enabled); + uint32_t chan_id, _Bool enabled, int auto_sync); // Channel info field selector typedef enum { @@ -334,7 +338,7 @@ cbsdk_session_t session, size_t n_chans, cbproto_channel_type_t chan_type, uint32_t sort_options); cbsdk_result_t cbsdk_session_set_channel_spike_sorting( - cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options); + cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options, int auto_sync); // Spike extraction (enable/disable cbAINPSPK_EXTRACT via CHANSETSPK) cbsdk_result_t cbsdk_session_set_spike_extraction( diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index 011bf16e..d5d8b6cf 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -973,22 +973,33 @@ def set_ac_input_coupling( "Failed to set AC input coupling", ) - def set_channel_label(self, chan_id: int, label: str): - """Set a channel's label.""" + def set_channel_label(self, chan_id: int, label: str, auto_sync: bool = False): + """Set a channel's label. + + Per-channel CHANSET setters seed the outgoing CHANINFO from the + local cache, so a prior in-flight setter from this process can + leave the seed stale. Pass ``auto_sync=True`` to run an internal + :meth:`sync` first (one round-trip latency cost). + """ _check( _get_lib().cbsdk_session_set_channel_label( - self._session, chan_id, label.encode() + self._session, chan_id, label.encode(), 1 if auto_sync else 0 ), "Failed to set channel label", ) - def set_channel_smpgroup(self, chan_id: int, rate: SampleRate | int): + def set_channel_smpgroup( + self, chan_id: int, rate: SampleRate | int, auto_sync: bool = False + ): """Set a single channel's sample group (fire-and-forget). Handles group-specific logic: RAWSTREAM flag for group 6 (raw), filter mapping for groups 1-4, clearing conflicting flags. Groups 5 (30 kHz filtered) and 6 (raw) are mutually exclusive. - Call :meth:`sync` before reading back state that depends on this. + + Pass ``auto_sync=True`` to internally sync before the read-modify- + write so the local cache reflects any in-flight config from this + process. .. note:: @@ -1002,61 +1013,76 @@ def set_channel_smpgroup(self, chan_id: int, rate: SampleRate | int): chan_id: 1-based channel ID. rate: Sample group (``SampleRate.NONE`` to disable, 1-5 for filtered groups, ``SampleRate.SR_RAW`` for raw). + auto_sync: If True, sync before the read-modify-write. """ _check( _get_lib().cbsdk_session_set_channel_smpgroup( self._session, chan_id, int(_coerce_enum(SampleRate, rate, _RATE_ALIASES)), + 1 if auto_sync else 0, ), "Failed to set channel smpgroup", ) - def set_channel_smpfilter(self, chan_id: int, filter_id: int): + def set_channel_smpfilter( + self, chan_id: int, filter_id: int, auto_sync: bool = False + ): """Set a channel's continuous-time pathway filter.""" _check( _get_lib().cbsdk_session_set_channel_smpfilter( - self._session, chan_id, filter_id + self._session, chan_id, filter_id, 1 if auto_sync else 0 ), "Failed to set channel smpfilter", ) - def set_channel_spkfilter(self, chan_id: int, filter_id: int): + def set_channel_spkfilter( + self, chan_id: int, filter_id: int, auto_sync: bool = False + ): """Set a channel's spike pathway filter.""" _check( _get_lib().cbsdk_session_set_channel_spkfilter( - self._session, chan_id, filter_id + self._session, chan_id, filter_id, 1 if auto_sync else 0 ), "Failed to set channel spkfilter", ) - def set_channel_ainpopts(self, chan_id: int, ainpopts: int): + def set_channel_ainpopts( + self, chan_id: int, ainpopts: int, auto_sync: bool = False + ): """Set a channel's analog input options (cbAINP_* flags).""" _check( _get_lib().cbsdk_session_set_channel_ainpopts( - self._session, chan_id, ainpopts + self._session, chan_id, ainpopts, 1 if auto_sync else 0 ), "Failed to set channel ainpopts", ) - def set_channel_lncrate(self, chan_id: int, rate: int): + def set_channel_lncrate(self, chan_id: int, rate: int, auto_sync: bool = False): """Set a channel's line noise cancellation adaptation rate.""" _check( - _get_lib().cbsdk_session_set_channel_lncrate(self._session, chan_id, rate), + _get_lib().cbsdk_session_set_channel_lncrate( + self._session, chan_id, rate, 1 if auto_sync else 0 + ), "Failed to set channel lncrate", ) - def set_channel_spkopts(self, chan_id: int, spkopts: int): + def set_channel_spkopts(self, chan_id: int, spkopts: int, auto_sync: bool = False): """Set a channel's spike processing options (cbAINPSPK_* flags).""" _check( _get_lib().cbsdk_session_set_channel_spkopts( - self._session, chan_id, spkopts + self._session, chan_id, spkopts, 1 if auto_sync else 0 ), "Failed to set channel spkopts", ) def set_channel_spkthrlevel(self, chan_id: int, level: int): - """Set a channel's spike threshold level.""" + """Set a channel's spike threshold level. + + Uses CHANSETSPKTHR which is "narrow" — the firmware only reads + ``spkthrlevel`` and the setter overwrites it fully, so no + ``auto_sync`` flag is needed. + """ _check( _get_lib().cbsdk_session_set_channel_spkthrlevel( self._session, chan_id, level @@ -1064,23 +1090,30 @@ def set_channel_spkthrlevel(self, chan_id: int, level: int): "Failed to set channel spkthrlevel", ) - def set_channel_autothreshold(self, chan_id: int, enabled: bool): + def set_channel_autothreshold( + self, chan_id: int, enabled: bool, auto_sync: bool = False + ): """Enable or disable auto-thresholding for a channel.""" _check( _get_lib().cbsdk_session_set_channel_autothreshold( - self._session, chan_id, enabled + self._session, chan_id, enabled, 1 if auto_sync else 0 ), "Failed to set channel autothreshold", ) - def configure_channel(self, chan_id: int, **kwargs): + def configure_channel(self, chan_id: int, *, auto_sync: bool = False, **kwargs): """Configure one or more attributes of a single channel. This is a convenience method that dispatches to the individual setters. Each keyword argument maps to a channel attribute. + When ``auto_sync=True``, the wrapper applies it to the **first** + dispatched setter only. Subsequent setters skip the sync because the + local cache is already fresh after the first one. + Args: chan_id: 1-based channel ID. + auto_sync: Sync once before the first dispatched setter. Keyword Args: label (str): Channel label (max 15 chars). @@ -1101,6 +1134,17 @@ def configure_channel(self, chan_id: int, **kwargs): autothreshold=True, ) """ + # All setters except set_channel_spkthrlevel accept an auto_sync kwarg. + _accepts_auto_sync = { + "label", + "smpgroup", + "smpfilter", + "spkfilter", + "ainpopts", + "lncrate", + "spkopts", + "autothreshold", + } _dispatch = { "label": self.set_channel_label, "smpfilter": self.set_channel_smpfilter, @@ -1111,13 +1155,18 @@ def configure_channel(self, chan_id: int, **kwargs): "spkthrlevel": self.set_channel_spkthrlevel, "autothreshold": self.set_channel_autothreshold, } + first = True for key, value in kwargs.items(): + sync_arg = auto_sync and first and key in _accepts_auto_sync if key == "smpgroup": - self.set_channel_smpgroup(chan_id, value) - elif key in _dispatch: + self.set_channel_smpgroup(chan_id, value, auto_sync=sync_arg) + elif key == "spkthrlevel": _dispatch[key](chan_id, value) + elif key in _dispatch: + _dispatch[key](chan_id, value, auto_sync=sync_arg) else: raise ValueError(f"Unknown channel attribute: {key!r}") + first = False # --- Channel Mapping (CMP) Files --- @@ -1377,7 +1426,9 @@ def set_spike_sorting( "Failed to set spike sorting", ) - def set_channel_spike_sorting(self, chan_id: int, sort_options: int): + def set_channel_spike_sorting( + self, chan_id: int, sort_options: int, auto_sync: bool = False + ): """Set spike sorting options for a single channel (fire-and-forget). Clears ``cbAINPSPK_ALLSORT`` bits then sets *sort_options*. @@ -1385,16 +1436,15 @@ def set_channel_spike_sorting(self, chan_id: int, sort_options: int): ``spkopts`` field), this preserves non-sorting bits like ``cbAINPSPK_EXTRACT``. - Call :meth:`sync` before reading back state that depends on this - configuration. - Args: chan_id: 1-based channel ID. sort_options: Spike sorting option flags (cbAINPSPK_*). + auto_sync: If True, sync before the read-modify-write so the + local cache reflects any in-flight config from this process. """ _check( _get_lib().cbsdk_session_set_channel_spike_sorting( - self._session, chan_id, sort_options + self._session, chan_id, sort_options, 1 if auto_sync else 0 ), "Failed to set channel spike sorting", ) diff --git a/src/cbsdk/include/cbsdk/cbsdk.h b/src/cbsdk/include/cbsdk/cbsdk.h index e3e24cc0..edc510e6 100644 --- a/src/cbsdk/include/cbsdk/cbsdk.h +++ b/src/cbsdk/include/cbsdk/cbsdk.h @@ -577,15 +577,24 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_config( cbsdk_session_t session, const cbPKT_CHANINFO* chaninfo); -/// Set a channel's label +/// Set a channel's label. +/// +/// Per-channel CHANSET setters are read-modify-write: they seed the outgoing +/// CHANINFO from the locally cached state. If a prior config command sent by +/// this process is still in flight, the seed may be stale. Pass +/// @p auto_sync != 0 to run an internal sync() barrier before reading the +/// cache, at the cost of one round-trip to the device. +/// /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param label New label string (max 15 chars, null-terminated) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_label( cbsdk_session_t session, uint32_t chan_id, - const char* label); + const char* label, + int auto_sync); /// Set a single channel's sample group (fire-and-forget). /// Handles group-specific logic: RAWSTREAM flag for group 6, filter mapping @@ -593,63 +602,79 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_label( /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param rate Sample group (0=NONE, 1-5=filtered, 6=RAW) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_smpgroup( cbsdk_session_t session, uint32_t chan_id, - cbproto_group_rate_t rate); + cbproto_group_rate_t rate, + int auto_sync); /// Set a channel's continuous-time pathway filter /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param filter_id Filter ID (0 to cbMAXFILTS-1) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_smpfilter( cbsdk_session_t session, uint32_t chan_id, - uint32_t filter_id); + uint32_t filter_id, + int auto_sync); /// Set a channel's spike pathway filter /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param filter_id Filter ID (0 to cbMAXFILTS-1) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spkfilter( cbsdk_session_t session, uint32_t chan_id, - uint32_t filter_id); + uint32_t filter_id, + int auto_sync); /// Set a channel's analog input options (LNC mode, reference electrode, etc.) /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param ainpopts Analog input option flags (cbAINP_* flags) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_ainpopts( cbsdk_session_t session, uint32_t chan_id, - uint32_t ainpopts); + uint32_t ainpopts, + int auto_sync); /// Set a channel's line noise cancellation adaptation rate /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param lncrate LNC rate +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_lncrate( cbsdk_session_t session, uint32_t chan_id, - uint32_t lncrate); + uint32_t lncrate, + int auto_sync); /// Set a channel's spike processing options /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param spkopts Spike option flags (cbAINPSPK_* flags) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spkopts( cbsdk_session_t session, uint32_t chan_id, - uint32_t spkopts); + uint32_t spkopts, + int auto_sync); -/// Set a channel's spike threshold level +/// Set a channel's spike threshold level. +/// +/// CHANSETSPKTHR is narrow (firmware only reads spkthrlevel and the setter +/// fully overwrites it), so no auto_sync flag is required. +/// /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param level Threshold level @@ -663,11 +688,13 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spkthrlevel( /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param enabled true to enable, false to disable +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_autothreshold( cbsdk_session_t session, uint32_t chan_id, - bool enabled); + bool enabled, + int auto_sync); /////////////////////////////////////////////////////////////////////////////////////////////////// // Bulk Channel Queries @@ -986,11 +1013,13 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_spike_sorting( /// @param session Session handle (must not be NULL) /// @param chan_id 1-based channel ID (1 to cbMAXCHANS) /// @param sort_options Spike sorting option flags (cbAINPSPK_*) +/// @param auto_sync Non-zero to sync() before the read-modify-write /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spike_sorting( cbsdk_session_t session, uint32_t chan_id, - uint32_t sort_options); + uint32_t sort_options, + int auto_sync); /// Enable or disable spike extraction for channels of a specific type. /// Controls the cbAINPSPK_EXTRACT bit via cbPKTTYPE_CHANSETSPK. diff --git a/src/cbsdk/src/cbsdk.cpp b/src/cbsdk/src/cbsdk.cpp index 3ef78926..ac4fa14e 100644 --- a/src/cbsdk/src/cbsdk.cpp +++ b/src/cbsdk/src/cbsdk.cpp @@ -922,13 +922,20 @@ cbsdk_result_t cbsdk_session_set_channel_config( /// Helper: read-modify-write a channel config field /// Gets current chaninfo from shared memory, calls `modify` on a copy, sends the packet. +/// If @p auto_sync is true, runs sync() first so the local cache reflects any +/// in-flight CHANREP packets before this read-modify-write seeds from it. static cbsdk_result_t modify_and_send_chaninfo( cbsdk_session_t session, uint32_t chan_id, uint16_t pkt_type, - std::function modify) { + std::function modify, + bool auto_sync = false) { if (!session || !session->cpp_session) return CBSDK_RESULT_INVALID_PARAMETER; try { + if (auto_sync) { + auto sync_result = session->cpp_session->sync(5000); + if (sync_result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; + } const cbPKT_CHANINFO* info = session->cpp_session->getChanInfo(chan_id); if (!info) return CBSDK_RESULT_INVALID_PARAMETER; cbPKT_CHANINFO ci = *info; @@ -945,22 +952,28 @@ static cbsdk_result_t modify_and_send_chaninfo( } cbsdk_result_t cbsdk_session_set_channel_label( - cbsdk_session_t session, uint32_t chan_id, const char* label) { + cbsdk_session_t session, uint32_t chan_id, const char* label, int auto_sync) { if (!label) return CBSDK_RESULT_INVALID_PARAMETER; return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETLABEL, [label](cbPKT_CHANINFO& ci) { std::strncpy(ci.label, label, sizeof(ci.label) - 1); ci.label[sizeof(ci.label) - 1] = '\0'; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_channel_smpgroup( - const cbsdk_session_t session, const uint32_t chan_id, const cbproto_group_rate_t rate) { + const cbsdk_session_t session, const uint32_t chan_id, const cbproto_group_rate_t rate, + int auto_sync) { if (!session || !session->cpp_session) return CBSDK_RESULT_INVALID_PARAMETER; // Mirror the per-group logic from DeviceSession::setChannelsGroupByType. // The packet type varies by group because the device firmware only reads // specific fields depending on the type. try { + if (auto_sync) { + auto sync_result = session->cpp_session->sync(5000); + if (sync_result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; + } const cbPKT_CHANINFO* info = session->cpp_session->getChanInfo(chan_id); if (!info) return CBSDK_RESULT_INVALID_PARAMETER; cbPKT_CHANINFO ci = *info; @@ -1002,47 +1015,57 @@ cbsdk_result_t cbsdk_session_set_channel_smpgroup( } cbsdk_result_t cbsdk_session_set_channel_smpfilter( - cbsdk_session_t session, uint32_t chan_id, uint32_t filter_id) { + cbsdk_session_t session, uint32_t chan_id, uint32_t filter_id, int auto_sync) { return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSMP, [filter_id](cbPKT_CHANINFO& ci) { ci.smpfilter = filter_id; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_channel_spkfilter( - cbsdk_session_t session, uint32_t chan_id, uint32_t filter_id) { + cbsdk_session_t session, uint32_t chan_id, uint32_t filter_id, int auto_sync) { return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSPK, [filter_id](cbPKT_CHANINFO& ci) { ci.spkfilter = filter_id; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_channel_ainpopts( - cbsdk_session_t session, uint32_t chan_id, uint32_t ainpopts) { + cbsdk_session_t session, uint32_t chan_id, uint32_t ainpopts, int auto_sync) { return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETAINP, [ainpopts](cbPKT_CHANINFO& ci) { ci.ainpopts = ainpopts; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_channel_lncrate( - cbsdk_session_t session, uint32_t chan_id, uint32_t lncrate) { + cbsdk_session_t session, uint32_t chan_id, uint32_t lncrate, int auto_sync) { return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETAINP, [lncrate](cbPKT_CHANINFO& ci) { ci.lncrate = lncrate; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_channel_spkopts( - cbsdk_session_t session, uint32_t chan_id, uint32_t spkopts) { - return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSPKTHR, + cbsdk_session_t session, uint32_t chan_id, uint32_t spkopts, int auto_sync) { + // Use CHANSETSPK (firmware reads spkopts+spkfilter for this type). + // CHANSETSPKTHR — used by older revisions of this code — only reads + // spkthrlevel and would silently ignore the spkopts update. + return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSPK, [spkopts](cbPKT_CHANINFO& ci) { ci.spkopts = spkopts; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_channel_spkthrlevel( cbsdk_session_t session, uint32_t chan_id, int32_t level) { + // CHANSETSPKTHR is "narrow": firmware reads only spkthrlevel and we + // overwrite it fully here, so no auto_sync flag is needed. return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSPKTHR, [level](cbPKT_CHANINFO& ci) { ci.spkthrlevel = level; @@ -1050,14 +1073,15 @@ cbsdk_result_t cbsdk_session_set_channel_spkthrlevel( } cbsdk_result_t cbsdk_session_set_channel_autothreshold( - cbsdk_session_t session, uint32_t chan_id, bool enabled) { + cbsdk_session_t session, uint32_t chan_id, bool enabled, int auto_sync) { return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETAUTOTHRESHOLD, [enabled](cbPKT_CHANINFO& ci) { if (enabled) ci.spkopts |= cbAINPSPK_THRAUTO; else ci.spkopts &= ~cbAINPSPK_THRAUTO; - }); + }, + auto_sync != 0); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -1527,12 +1551,16 @@ cbsdk_result_t cbsdk_session_set_spike_sorting( } cbsdk_result_t cbsdk_session_set_channel_spike_sorting( - cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options) { - return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSPKTHR, + cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options, int auto_sync) { + // Use CHANSETSPK (firmware reads spkopts+spkfilter). Earlier revisions + // used CHANSETSPKTHR, which only reads spkthrlevel and would silently + // ignore the spkopts update we want here. + return modify_and_send_chaninfo(session, chan_id, cbPKTTYPE_CHANSETSPK, [sort_options](cbPKT_CHANINFO& ci) { ci.spkopts &= ~cbAINPSPK_ALLSORT; ci.spkopts |= sort_options; - }); + }, + auto_sync != 0); } cbsdk_result_t cbsdk_session_set_spike_extraction( diff --git a/tests/integration/test_capi_configuration.cpp b/tests/integration/test_capi_configuration.cpp index 6201b406..4f151929 100644 --- a/tests/integration/test_capi_configuration.cpp +++ b/tests/integration/test_capi_configuration.cpp @@ -327,9 +327,9 @@ TEST_F(CApiPerChannelTest, SetChannelLabel) { SessionGuard sg; ASSERT_TRUE(sg.create()); - EXPECT_EQ(cbsdk_session_set_channel_label(sg.session, 1, "TestCh"), + EXPECT_EQ(cbsdk_session_set_channel_label(sg.session, 1, "TestCh", /*auto_sync=*/0), CBSDK_RESULT_SUCCESS); - std::this_thread::sleep_for(std::chrono::milliseconds(300)); + EXPECT_EQ(cbsdk_session_sync(sg.session, 5000), CBSDK_RESULT_SUCCESS); const char* label = cbsdk_session_get_channel_label(sg.session, 1); ASSERT_NE(label, nullptr); @@ -340,9 +340,9 @@ TEST_F(CApiPerChannelTest, SetChannelSmpfilter) { SessionGuard sg; ASSERT_TRUE(sg.create()); - EXPECT_EQ(cbsdk_session_set_channel_smpfilter(sg.session, 1, 2), + EXPECT_EQ(cbsdk_session_set_channel_smpfilter(sg.session, 1, 2, /*auto_sync=*/0), CBSDK_RESULT_SUCCESS); - std::this_thread::sleep_for(std::chrono::milliseconds(300)); + EXPECT_EQ(cbsdk_session_sync(sg.session, 5000), CBSDK_RESULT_SUCCESS); EXPECT_EQ(cbsdk_session_get_channel_smpfilter(sg.session, 1), 2u); } @@ -351,9 +351,9 @@ TEST_F(CApiPerChannelTest, SetChannelSpkfilter) { SessionGuard sg; ASSERT_TRUE(sg.create()); - EXPECT_EQ(cbsdk_session_set_channel_spkfilter(sg.session, 1, 3), + EXPECT_EQ(cbsdk_session_set_channel_spkfilter(sg.session, 1, 3, /*auto_sync=*/0), CBSDK_RESULT_SUCCESS); - std::this_thread::sleep_for(std::chrono::milliseconds(300)); + EXPECT_EQ(cbsdk_session_sync(sg.session, 5000), CBSDK_RESULT_SUCCESS); EXPECT_EQ(cbsdk_session_get_channel_spkfilter(sg.session, 1), 3u); } From d0072c04cb2477f57f65a4b5da56221f599f1973 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 27 Apr 2026 19:35:12 -0400 Subject: [PATCH 2/8] Bulk by-type setters: auto-sync, n_configured, chans=None (#177, #181) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 to Result; 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) --- pycbsdk/src/pycbsdk/_cdef.py | 16 +- pycbsdk/src/pycbsdk/session.py | 102 ++++-- pycbsdk/tests/test_configuration.py | 55 +++ src/cbdev/src/device_session.cpp | 11 +- src/cbsdk/include/cbsdk/cbsdk.h | 53 ++- src/cbsdk/include/cbsdk/sdk_session.h | 63 ++-- src/cbsdk/src/cbsdk.cpp | 39 ++- src/cbsdk/src/sdk_session.cpp | 330 ++++++++++-------- tests/integration/test_capi_configuration.cpp | 27 +- tests/unit/test_cbsdk_c_api.cpp | 5 +- 10 files changed, 440 insertions(+), 261 deletions(-) diff --git a/pycbsdk/src/pycbsdk/_cdef.py b/pycbsdk/src/pycbsdk/_cdef.py index 09330147..f9f738fd 100644 --- a/pycbsdk/src/pycbsdk/_cdef.py +++ b/pycbsdk/src/pycbsdk/_cdef.py @@ -206,11 +206,11 @@ // Channel configuration cbsdk_result_t cbsdk_session_set_sample_group( - cbsdk_session_t session, size_t n_chans, cbproto_channel_type_t chan_type, - cbproto_group_rate_t rate, _Bool disable_others); + cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, + cbproto_group_rate_t rate, _Bool disable_others, uint32_t* out_n_configured); cbsdk_result_t cbsdk_session_set_ac_input_coupling( - cbsdk_session_t session, size_t n_chans, cbproto_channel_type_t chan_type, - _Bool enabled); + cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, + _Bool enabled, uint32_t* out_n_configured); // Per-channel getters cbproto_channel_type_t cbsdk_session_get_channel_type(cbsdk_session_t session, uint32_t chan_id); @@ -335,15 +335,15 @@ // Spike sorting cbsdk_result_t cbsdk_session_set_spike_sorting( - cbsdk_session_t session, size_t n_chans, cbproto_channel_type_t chan_type, - uint32_t sort_options); + cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, + uint32_t sort_options, uint32_t* out_n_configured); cbsdk_result_t cbsdk_session_set_channel_spike_sorting( cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options, int auto_sync); // Spike extraction (enable/disable cbAINPSPK_EXTRACT via CHANSETSPK) cbsdk_result_t cbsdk_session_set_spike_extraction( - cbsdk_session_t session, size_t n_chans, cbproto_channel_type_t chan_type, - bool enabled); + cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, + bool enabled, uint32_t* out_n_configured); // Clock synchronization cbsdk_result_t cbsdk_session_get_clock_offset(cbsdk_session_t session, int64_t* offset_ns); diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index d5d8b6cf..6727b0bc 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -12,6 +12,9 @@ from ._lib import ffi, load_library +# "All matching" sentinel for bulk channel-config setters. +_ALL_CHANS = 0xFFFFFFFF + lib = None # Lazy-loaded @@ -903,37 +906,46 @@ def get_channels_positions( def set_sample_group( self, - n_chans: int, + chans: int | None, channel_type: ChannelType, rate: SampleRate, disable_others: bool = False, - ): - """Set sampling rate for channels of a specific type (fire-and-forget). + ) -> int: + """Set sampling rate for channels of a specific type. - Configures the first *n_chans* channels matching *channel_type*. - To configure a specific channel by ID, use :meth:`set_channel_smpgroup`. + Configures the first *chans* channels matching *channel_type* (or + every matching channel when *chans* is ``None``). To configure a + specific channel by ID, use :meth:`set_channel_smpgroup`. - The device will not have applied the new configuration when this - call returns. Call :meth:`sync` before reading back state (e.g., - :meth:`get_group_channels`) or registering callbacks that depend - on the new configuration. + Self-synchronizing: runs :meth:`sync` before reading the local cache + (so prior in-flight config has landed) and again after sending, so + the returned count reflects the post-config state. Always sends a + CHANSET* packet for every in-scope channel — never skips channels + that look already configured (the local cache may be stale due to + a dropped CHANREP). .. note:: The device does not update the ``smpgroup`` field for raw - channels. After setting ``SampleRate.SR_RAW``, - :meth:`get_channel_config` will show ``smpgroup=0``. - Use ``get_group_channels(int(SampleRate.SR_RAW))`` to check - raw group membership. + channels. When *rate* is ``SampleRate.SR_RAW`` the returned + count and :meth:`get_group_channels` use the + ``cbAINP_RAWSTREAM`` bit instead. Args: - n_chans: Number of channels to configure. + chans: Number of channels to configure, or ``None`` for all + matching. channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). rate: Sample rate (e.g., ``SampleRate.SR_30kHz``, ``SampleRate.NONE`` to disable). disable_others: Disable sampling on unselected channels. + + Returns: + Count of *channel_type* channels whose post-config state matches + *rate*. """ _lib = _get_lib() + n_chans = _ALL_CHANS if chans is None else int(chans) + out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_sample_group( self._session, @@ -941,37 +953,47 @@ def set_sample_group( int(_coerce_enum(ChannelType, channel_type)), int(_coerce_enum(SampleRate, rate, _RATE_ALIASES)), disable_others, + out_n, ), "Failed to set channel sample group", ) + return int(out_n[0]) def set_ac_input_coupling( self, - n_chans: int, + chans: int | None, channel_type: ChannelType, enabled: bool, - ): - """Set AC/DC input coupling for channels of a specific type (fire-and-forget). + ) -> int: + """Set AC/DC input coupling for channels of a specific type. - Call :meth:`sync` before reading back state that depends on this - configuration. + Self-synchronizing — see :meth:`set_sample_group` for the contract. Args: - n_chans: Number of channels to configure. + chans: Number of channels to configure, or ``None`` for all + matching. channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). enabled: ``True`` for AC coupling (offset correction on), ``False`` for DC coupling. + + Returns: + Count of *channel_type* channels whose post-config + ``cbAINP_OFFSET_CORRECT`` bit matches *enabled*. """ _lib = _get_lib() + n_chans = _ALL_CHANS if chans is None else int(chans) + out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_ac_input_coupling( self._session, n_chans, int(_coerce_enum(ChannelType, channel_type)), enabled, + out_n, ), "Failed to set AC input coupling", ) + return int(out_n[0]) def set_channel_label(self, chan_id: int, label: str, auto_sync: bool = False): """Set a channel's label. @@ -1401,30 +1423,38 @@ def close_central_file_dialog(self): def set_spike_sorting( self, - n_chans: int, + chans: int | None, channel_type: ChannelType, sort_options: int, - ): - """Set spike sorting options for channels of a specific type (fire-and-forget). + ) -> int: + """Set spike sorting options for channels of a specific type. - Call :meth:`sync` before reading back state that depends on this - configuration. + Self-synchronizing — see :meth:`set_sample_group` for the contract. Args: - n_chans: Number of channels to configure. + chans: Number of channels to configure, or ``None`` for all + matching. channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). sort_options: Spike sorting option flags (cbAINPSPK_*). + + Returns: + Count of *channel_type* channels whose post-config + ``spkopts & cbAINPSPK_ALLSORT`` matches *sort_options*. """ _lib = _get_lib() + n_chans = _ALL_CHANS if chans is None else int(chans) + out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_spike_sorting( self._session, n_chans, int(_coerce_enum(ChannelType, channel_type)), sort_options, + out_n, ), "Failed to set spike sorting", ) + return int(out_n[0]) def set_channel_spike_sorting( self, chan_id: int, sort_options: int, auto_sync: bool = False @@ -1451,33 +1481,41 @@ def set_channel_spike_sorting( def set_spike_extraction( self, - n_chans: int, + chans: int | None, channel_type: ChannelType, enabled: bool, - ): - """Enable or disable spike extraction for channels of a specific type (fire-and-forget). + ) -> int: + """Enable or disable spike extraction for channels of a specific type. Controls the ``cbAINPSPK_EXTRACT`` bit which determines whether the device emits spike event packets. Uses ``cbPKTTYPE_CHANSETSPK``. - Call :meth:`sync` before reading back state that depends on this - configuration. + Self-synchronizing — see :meth:`set_sample_group` for the contract. Args: - n_chans: Number of channels to configure. + chans: Number of channels to configure, or ``None`` for all + matching. channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). enabled: ``True`` to enable spike extraction, ``False`` to disable. + + Returns: + Count of *channel_type* channels whose post-config + ``cbAINPSPK_EXTRACT`` bit matches *enabled*. """ _lib = _get_lib() + n_chans = _ALL_CHANS if chans is None else int(chans) + out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_spike_extraction( self._session, n_chans, int(_coerce_enum(ChannelType, channel_type)), enabled, + out_n, ), "Failed to set spike extraction", ) + return int(out_n[0]) # --- Clock Synchronization --- diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index 279fd0fd..c6965125 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -417,6 +417,61 @@ def test_set_spike_sorting(self, nplay_session): nplay_session.sync() +# --------------------------------------------------------------------------- +# n_configured return + chans=None semantics +# --------------------------------------------------------------------------- + + +class TestNConfigured: + """Bulk setters return the count of channels in the post-config state.""" + + def test_set_sample_group_returns_count(self, nplay_session): + n_fe = nplay_session.num_fe_chans() + n = nplay_session.set_sample_group( + n_fe, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, + ) + assert n == n_fe + # And matches a fresh re-scan via get_group_channels. + assert n == len(nplay_session.get_group_channels(int(SampleRate.SR_30kHz))) + + def test_set_sample_group_chans_none_is_all(self, nplay_session): + n_fe = nplay_session.num_fe_chans() + n = nplay_session.set_sample_group( + None, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, + ) + assert n == n_fe + + def test_set_sample_group_partial(self, nplay_session): + # Configure only the first 2 channels at 1kHz, leave the rest untouched. + n = nplay_session.set_sample_group( + 2, ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=False, + ) + # Returned count is "channels at SR_1kHz post-config", i.e. exactly 2. + assert n == 2 + + def test_set_ac_input_coupling_returns_count(self, nplay_session): + n_fe = nplay_session.num_fe_chans() + n_on = nplay_session.set_ac_input_coupling( + None, ChannelType.FRONTEND, True, + ) + assert n_on == n_fe + n_off = nplay_session.set_ac_input_coupling( + None, ChannelType.FRONTEND, False, + ) + assert n_off == n_fe + + def test_set_spike_extraction_returns_count(self, nplay_session): + n_fe = nplay_session.num_fe_chans() + n_on = nplay_session.set_spike_extraction( + None, ChannelType.FRONTEND, True, + ) + assert n_on == n_fe + n_off = nplay_session.set_spike_extraction( + None, ChannelType.FRONTEND, False, + ) + assert n_off == n_fe + + # --------------------------------------------------------------------------- # Per-channel configuration (configure_channel) # --------------------------------------------------------------------------- diff --git a/src/cbdev/src/device_session.cpp b/src/cbdev/src/device_session.cpp index e5423e46..a28c0906 100644 --- a/src/cbdev/src/device_session.cpp +++ b/src/cbdev/src/device_session.cpp @@ -1220,9 +1220,13 @@ Result DeviceSession::setChannelsSpikeSortingByType(const size_t nChans, c continue; } - // Create channel config packet + // Create channel config packet. + // + // Use CHANSET so the firmware applies the spkopts modification. + // CHANSETSPKTHR (used by older revisions) only reads spkthrlevel + // server-side, so the spkopts changes were silently dropped. cbPKT_CHANINFO pkt = chaninfo; // Start with current config - pkt.cbpkt_header.type = cbPKTTYPE_CHANSETSPKTHR; // Use spike threshold set command + pkt.cbpkt_header.type = cbPKTTYPE_CHANSET; pkt.chan = chan; // Clear all spike sorting flags and set new ones @@ -1249,8 +1253,9 @@ Result DeviceSession::setChannelsSpikeSortingSync(const size_t nChans, con return setChannelsSpikeSortingByType(nChans, chanType, sortOptions); }, [](const cbPKT_HEADER* hdr) { + // CHANSET broadcasts a CHANREP echo (not CHANREPSPKTHR). return (hdr->chid & cbPKTCHAN_CONFIGURATION) == cbPKTCHAN_CONFIGURATION && - hdr->type == cbPKTTYPE_CHANREPSPKTHR; + hdr->type == cbPKTTYPE_CHANREP; }, timeout, total_matching diff --git a/src/cbsdk/include/cbsdk/cbsdk.h b/src/cbsdk/include/cbsdk/cbsdk.h index edc510e6..417ab1b5 100644 --- a/src/cbsdk/include/cbsdk/cbsdk.h +++ b/src/cbsdk/include/cbsdk/cbsdk.h @@ -543,31 +543,46 @@ CBSDK_API cbsdk_result_t cbsdk_session_get_group_list( // Channel Configuration /////////////////////////////////////////////////////////////////////////////////////////////////// -/// Set sampling rate for channels of a specific type +/// Set sampling rate for channels of a specific type. +/// +/// Internally syncs before reading the local cache and again after sending, +/// so @p out_n_configured reflects the channels' post-config state rather +/// than the count just changed by this call. Always sends a CHANSET* packet +/// for every in-scope channel — no skip-if-already-correct optimization, +/// since the local cache may be stale due to a dropped CHANREP. +/// /// @param session Session handle (must not be NULL) -/// @param n_chans Number of channels to configure (use cbMAXCHANS for all) +/// @param n_chans Channels of @p chan_type to configure, in ascending +/// channel-id order. Use UINT32_MAX for all matching. /// @param chan_type Channel type filter /// @param rate Sample rate (CBPROTO_GROUP_RATE_NONE to disable, _500Hz through _RAW) /// @param disable_others If true, disable sampling on unselected channels of this type +/// @param[out] out_n_configured Receives the count of @p chan_type channels +/// whose post-config state matches @p rate. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_sample_group( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, - bool disable_others); + bool disable_others, + uint32_t* out_n_configured); -/// Set AC input coupling (offset correction) for channels of a specific type +/// Set AC input coupling (offset correction) for channels of a specific type. +/// See cbsdk_session_set_sample_group() for the auto-sync contract. /// @param session Session handle (must not be NULL) -/// @param n_chans Number of channels to configure (use cbMAXCHANS for all) +/// @param n_chans Channels to configure (UINT32_MAX = all matching) /// @param chan_type Channel type filter /// @param enabled true = AC coupling, false = DC coupling +/// @param[out] out_n_configured Count of @p chan_type channels whose +/// post-config OFFSET_CORRECT bit matches @p enabled. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, - bool enabled); + bool enabled, + uint32_t* out_n_configured); /// Set full channel configuration by sending a CHANINFO packet /// @param session Session handle (must not be NULL) @@ -996,17 +1011,21 @@ CBSDK_API cbsdk_result_t cbsdk_session_close_central_file_dialog(cbsdk_session_t // Spike Sorting /////////////////////////////////////////////////////////////////////////////////////////////////// -/// Set spike sorting options for channels of a specific type +/// Set spike sorting options for channels of a specific type. +/// See cbsdk_session_set_sample_group() for the auto-sync contract. /// @param session Session handle (must not be NULL) -/// @param n_chans Number of channels to configure +/// @param n_chans Channels to configure (UINT32_MAX = all matching) /// @param chan_type Channel type filter /// @param sort_options Spike sorting option flags (cbAINPSPK_*) +/// @param[out] out_n_configured Count of @p chan_type channels whose +/// post-config spkopts ALLSORT bits match @p sort_options. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_spike_sorting( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, - uint32_t sort_options); + uint32_t sort_options, + uint32_t* out_n_configured); /// Set spike sorting options for a single channel (fire-and-forget). /// Clears cbAINPSPK_ALLSORT bits then sets sort_options. @@ -1024,16 +1043,20 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spike_sorting( /// Enable or disable spike extraction for channels of a specific type. /// Controls the cbAINPSPK_EXTRACT bit via cbPKTTYPE_CHANSETSPK. /// When enabled, the device emits spike event packets for matching channels. +/// See cbsdk_session_set_sample_group() for the auto-sync contract. /// @param session Session handle (must not be NULL) -/// @param n_chans Number of channels to configure +/// @param n_chans Channels to configure (UINT32_MAX = all matching) /// @param chan_type Channel type filter /// @param enabled true = enable spike extraction, false = disable +/// @param[out] out_n_configured Count of @p chan_type channels whose +/// post-config EXTRACT bit matches @p enabled. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_spike_extraction( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, - bool enabled); + bool enabled, + uint32_t* out_n_configured); /////////////////////////////////////////////////////////////////////////////////////////////////// // Clock Synchronization diff --git a/src/cbsdk/include/cbsdk/sdk_session.h b/src/cbsdk/include/cbsdk/sdk_session.h index 55d538a9..ab83e34f 100644 --- a/src/cbsdk/include/cbsdk/sdk_session.h +++ b/src/cbsdk/include/cbsdk/sdk_session.h @@ -509,46 +509,53 @@ class SdkSession { /// Channel Configuration ///-------------------------------------------------------------------------------------------- - /// Set sampling rate for channels of a specific type (fire-and-forget). - /// The device will not have applied the new configuration when this call - /// returns. Call sync() before reading back state (e.g., getGroupChannelList) - /// or registering callbacks that depend on the new configuration. - /// @param nChans Number of channels to configure (cbMAXCHANS for all) + /// Set sampling rate for channels of a specific type. + /// + /// Internally runs sync() before reading the local cache (so any prior + /// in-flight config from this process has landed) and again after sending + /// (so the returned count reflects post-config state). Always sends a + /// CHANSET* packet for every in-scope channel — never skips channels that + /// look already configured, since the local cache may be stale due to + /// dropped CHANREP packets from a concurrent client. + /// + /// @param nChans Number of channels of @p chanType to configure, in + /// ascending channel-id order. Use UINT32_MAX for all matching. /// @param chanType Channel type filter /// @param rate Desired sample rate (NONE to disable, SR_500 through SR_RAW) - /// @param disableOthers Disable sampling on channels not in the first nChans of type - /// @return Result indicating success or error - Result setSampleGroup(size_t nChans, ChannelType chanType, - SampleRate rate, bool disableOthers = false); - - /// Set spike sorting options for channels of a specific type (fire-and-forget). - /// Call sync() before reading back state that depends on this configuration. - /// @param nChans Number of channels to configure + /// @param disableOthers Disable sampling on channels not in the first + /// @p nChans of @p chanType + /// @return Number of channels of @p chanType whose post-config state + /// matches @p rate, or error. + Result setSampleGroup(uint32_t nChans, ChannelType chanType, + SampleRate rate, bool disableOthers = false); + + /// Set spike sorting options for channels of a specific type. + /// See setSampleGroup() for the auto-sync and idempotency contract. + /// @param nChans Channels to configure (UINT32_MAX = all matching) /// @param chanType Channel type filter /// @param sortOptions Spike sorting option flags (cbAINPSPK_*) - /// @return Result indicating success or error - Result setSpikeSorting(size_t nChans, ChannelType chanType, - uint32_t sortOptions); + /// @return Count of @p chanType channels whose spkopts ALLSORT bits + /// match @p sortOptions post-sync, or error. + Result setSpikeSorting(uint32_t nChans, ChannelType chanType, + uint32_t sortOptions); /// Enable or disable spike extraction (cbAINPSPK_EXTRACT) for channels - /// of a type (fire-and-forget). - /// This controls whether the device emits spike event packets for these channels. - /// Uses cbPKTTYPE_CHANSETSPK (not the threshold command). - /// Call sync() before reading back state that depends on this configuration. - /// @param nChans Number of channels to configure + /// of a type. See setSampleGroup() for the auto-sync contract. + /// @param nChans Channels to configure (UINT32_MAX = all matching) /// @param chanType Channel type filter /// @param enabled true = enable spike extraction, false = disable - /// @return Result indicating success or error - Result setSpikeExtraction(size_t nChans, ChannelType chanType, bool enabled); + /// @return Count of @p chanType channels whose EXTRACT bit matches + /// @p enabled post-sync, or error. + Result setSpikeExtraction(uint32_t nChans, ChannelType chanType, bool enabled); /// Set AC input coupling (offset correction) for channels of a specific - /// type (fire-and-forget). - /// Call sync() before reading back state that depends on this configuration. - /// @param nChans Number of channels to configure (cbMAXCHANS for all) + /// type. See setSampleGroup() for the auto-sync contract. + /// @param nChans Channels to configure (UINT32_MAX = all matching) /// @param chanType Channel type filter /// @param enabled true = AC coupling (offset correction on), false = DC coupling - /// @return Result indicating success or error - Result setACInputCoupling(size_t nChans, ChannelType chanType, bool enabled); + /// @return Count of @p chanType channels whose OFFSET_CORRECT bit + /// matches @p enabled post-sync, or error. + Result setACInputCoupling(uint32_t nChans, ChannelType chanType, bool enabled); /// Set full channel configuration by packet (fire-and-forget). /// Call sync() before reading back state that depends on this configuration. diff --git a/src/cbsdk/src/cbsdk.cpp b/src/cbsdk/src/cbsdk.cpp index ac4fa14e..25099392 100644 --- a/src/cbsdk/src/cbsdk.cpp +++ b/src/cbsdk/src/cbsdk.cpp @@ -890,17 +890,21 @@ cbsdk_result_t cbsdk_session_get_group_list( cbsdk_result_t cbsdk_session_set_sample_group( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, - bool disable_others) { + bool disable_others, + uint32_t* out_n_configured) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setSampleGroup( - n_chans, to_cpp_channel_type(chan_type), static_cast(rate), disable_others); - return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; + n_chans, to_cpp_channel_type(chan_type), + static_cast(rate), disable_others); + if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; + if (out_n_configured) *out_n_configured = result.value(); + return CBSDK_RESULT_SUCCESS; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } @@ -1514,16 +1518,19 @@ cbsdk_result_t cbsdk_session_close_central_file_dialog(cbsdk_session_t session) cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, - bool enabled) { + bool enabled, + uint32_t* out_n_configured) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setACInputCoupling( n_chans, to_cpp_channel_type(chan_type), enabled); - return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; + if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; + if (out_n_configured) *out_n_configured = result.value(); + return CBSDK_RESULT_SUCCESS; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } @@ -1535,16 +1542,19 @@ cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_result_t cbsdk_session_set_spike_sorting( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, - uint32_t sort_options) { + uint32_t sort_options, + uint32_t* out_n_configured) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setSpikeSorting( n_chans, to_cpp_channel_type(chan_type), sort_options); - return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; + if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; + if (out_n_configured) *out_n_configured = result.value(); + return CBSDK_RESULT_SUCCESS; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } @@ -1565,16 +1575,19 @@ cbsdk_result_t cbsdk_session_set_channel_spike_sorting( cbsdk_result_t cbsdk_session_set_spike_extraction( cbsdk_session_t session, - size_t n_chans, + uint32_t n_chans, cbproto_channel_type_t chan_type, - bool enabled) { + bool enabled, + uint32_t* out_n_configured) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setSpikeExtraction( n_chans, to_cpp_channel_type(chan_type), enabled); - return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; + if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; + if (out_n_configured) *out_n_configured = result.value(); + return CBSDK_RESULT_SUCCESS; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } diff --git a/src/cbsdk/src/sdk_session.cpp b/src/cbsdk/src/sdk_session.cpp index 5938ef78..53270a8e 100644 --- a/src/cbsdk/src/sdk_session.cpp +++ b/src/cbsdk/src/sdk_session.cpp @@ -1663,191 +1663,219 @@ static cbdev::ChannelType toDevChannelType(const ChannelType chanType) { } -Result SdkSession::setSampleGroup(const size_t nChans, const ChannelType chanType, - const SampleRate rate, const bool disableOthers) { +// Count channels of `chanType` for which `predicate(chaninfo)` is true. +// Caller must ensure the local chaninfo cache is fresh (typically by calling +// sync() first) so the count reflects post-config state rather than pre-config. +template +static uint32_t countChannelsMatching(const SdkSession& session, + ChannelType chanType, Predicate&& predicate) { + uint32_t count = 0; + for (uint32_t chan = 1; chan <= cbMAXCHANS; ++chan) { + const cbPKT_CHANINFO* ci = session.getChanInfo(chan); + if (!ci) continue; + if (classifyChannelByCaps(*ci) != chanType) continue; + if (predicate(*ci)) count++; + } + return count; +} + +Result SdkSession::setSampleGroup(const uint32_t nChans, const ChannelType chanType, + const SampleRate rate, const bool disableOthers) { + // Pre-sync: ensure local chaninfo cache is up to date before we seed + // outgoing CHANSET* packets from it. Stale cache would re-send obsolete + // values for fields we don't explicitly modify here. + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + const uint32_t group_id = static_cast(rate); - // STANDALONE mode: delegate to device session (has full config + direct send) + Result send_result = Result::error("No session available"); if (m_impl->device_session) { - const auto r = m_impl->device_session->setChannelsGroupByType( - nChans, toDevChannelType(chanType), static_cast(group_id), disableOthers); - if (r.isError()) - return Result::error(r.error()); - return Result::ok(); - } + send_result = m_impl->device_session->setChannelsGroupByType( + nChans, toDevChannelType(chanType), + static_cast(group_id), disableOthers); + } else if (m_impl->shmem_session) { + // CLIENT mode: build packets from shmem chaninfo and send through xmt queue. + // Always send for every in-scope channel — don't skip "looks already correct" + // since a dropped CHANREP could leave us cached against a concurrent change + // we'd have no way to escape from except by sending the desired value. + uint32_t sent_count = 0; + for (uint32_t chan = 1; chan <= cbMAXCHANS; ++chan) { + if (!disableOthers && sent_count >= nChans) + break; - // CLIENT mode: build packets from shmem chaninfo and send through shmem transmit queue - if (!m_impl->shmem_session) - return Result::error("No session available"); + auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); + if (ci_result.isError()) continue; + auto chaninfo = ci_result.value(); - size_t count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS; ++chan) { - if (!disableOthers && count >= nChans) - break; - - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) - continue; - auto chaninfo = ci_result.value(); - - if (classifyChannelByCaps(chaninfo) != chanType) - continue; - - const auto grp = count < nChans ? group_id : 0u; - chaninfo.chan = chan; - - if (grp > 0 && grp < 6) { - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSMP; - chaninfo.smpgroup = grp; - constexpr uint32_t filter_map[] = {0, 5, 6, 7, 10, 0, 0}; - chaninfo.smpfilter = filter_map[grp]; - if (grp == 5) { + if (classifyChannelByCaps(chaninfo) != chanType) continue; + + const auto grp = sent_count < nChans ? group_id : 0u; + chaninfo.chan = chan; + + if (grp > 0 && grp < 6) { + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSMP; + chaninfo.smpgroup = grp; + constexpr uint32_t filter_map[] = {0, 5, 6, 7, 10, 0, 0}; + chaninfo.smpfilter = filter_map[grp]; + if (grp == 5) { + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; + chaninfo.ainpopts &= ~cbAINP_RAWSTREAM; + } + } else if (grp == 6) { + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; + chaninfo.ainpopts |= cbAINP_RAWSTREAM; + } else { chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; + chaninfo.smpgroup = 0; chaninfo.ainpopts &= ~cbAINP_RAWSTREAM; } - } else if (grp == 6) { - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; - chaninfo.ainpopts |= cbAINP_RAWSTREAM; - } else { - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; - chaninfo.smpgroup = 0; - chaninfo.ainpopts &= ~cbAINP_RAWSTREAM; - } - auto r = sendPacket(reinterpret_cast(chaninfo)); - if (r.isError()) - return r; - count++; + auto sr = sendPacket(reinterpret_cast(chaninfo)); + if (sr.isError()) return Result::error(sr.error()); + sent_count++; + } + send_result = Result::ok(); } + if (send_result.isError()) return Result::error(send_result.error()); - if (count == 0) - return Result::error("No channels found matching type"); - return Result::ok(); -} + // Post-sync: ensure the device has applied our changes before we count. + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); -Result SdkSession::setSpikeSorting(const size_t nChans, const ChannelType chanType, - const uint32_t sortOptions) { - // STANDALONE mode: delegate to device session - if (m_impl->device_session) { - const auto r = m_impl->device_session->setChannelsSpikeSortingByType( - nChans, toDevChannelType(chanType), sortOptions); - if (r.isError()) - return Result::error(r.error()); - return Result::ok(); + // Count channels of chanType whose post-config state matches the request. + // SR_RAW is the special case: the device does not update smpgroup for raw + // channels, so check the RAWSTREAM bit instead. + if (rate == SampleRate::SR_RAW) { + return Result::ok(countChannelsMatching(*this, chanType, + [](const cbPKT_CHANINFO& ci) { + return (ci.ainpopts & cbAINP_RAWSTREAM) != 0; + })); } + return Result::ok(countChannelsMatching(*this, chanType, + [group_id](const cbPKT_CHANINFO& ci) { + return ci.smpgroup == group_id; + })); +} - // CLIENT mode: build packets from shmem chaninfo - if (!m_impl->shmem_session) - return Result::error("No session available"); +Result SdkSession::setSpikeSorting(const uint32_t nChans, const ChannelType chanType, + const uint32_t sortOptions) { + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - size_t count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS && count < nChans; ++chan) { - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) - continue; - auto chaninfo = ci_result.value(); + Result send_result = Result::error("No session available"); + if (m_impl->device_session) { + send_result = m_impl->device_session->setChannelsSpikeSortingByType( + nChans, toDevChannelType(chanType), sortOptions); + } else if (m_impl->shmem_session) { + uint32_t sent_count = 0; + for (uint32_t chan = 1; chan <= cbMAXCHANS && sent_count < nChans; ++chan) { + auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); + if (ci_result.isError()) continue; + auto chaninfo = ci_result.value(); - if (classifyChannelByCaps(chaninfo) != chanType) - continue; + if (classifyChannelByCaps(chaninfo) != chanType) continue; - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSPKTHR; - chaninfo.chan = chan; - chaninfo.spkopts &= ~cbAINPSPK_ALLSORT; - chaninfo.spkopts |= sortOptions; + // Use CHANSET so the device applies spkopts changes. CHANSETSPKTHR + // (used by older revisions) only reads spkthrlevel and would + // silently ignore the spkopts modification we want here. + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; + chaninfo.chan = chan; + chaninfo.spkopts &= ~cbAINPSPK_ALLSORT; + chaninfo.spkopts |= sortOptions; - auto r = sendPacket(reinterpret_cast(chaninfo)); - if (r.isError()) - return r; - count++; + auto sr = sendPacket(reinterpret_cast(chaninfo)); + if (sr.isError()) return Result::error(sr.error()); + sent_count++; + } + send_result = Result::ok(); } + if (send_result.isError()) return Result::error(send_result.error()); - if (count == 0) - return Result::error("No channels found matching type"); - return Result::ok(); + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + + const uint32_t want_sort = sortOptions & cbAINPSPK_ALLSORT; + return Result::ok(countChannelsMatching(*this, chanType, + [want_sort](const cbPKT_CHANINFO& ci) { + return (ci.spkopts & cbAINPSPK_ALLSORT) == want_sort; + })); } -Result SdkSession::setSpikeExtraction(const size_t nChans, const ChannelType chanType, - const bool enabled) { - // STANDALONE mode: delegate to device session +Result SdkSession::setSpikeExtraction(const uint32_t nChans, const ChannelType chanType, + const bool enabled) { + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + + Result send_result = Result::error("No session available"); if (m_impl->device_session) { - const auto r = m_impl->device_session->setSpikeExtraction( + send_result = m_impl->device_session->setSpikeExtraction( nChans, toDevChannelType(chanType), enabled); - if (r.isError()) - return Result::error(r.error()); - return Result::ok(); + } else if (m_impl->shmem_session) { + uint32_t sent_count = 0; + for (uint32_t chan = 1; chan <= cbMAXCHANS && sent_count < nChans; ++chan) { + auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); + if (ci_result.isError()) continue; + auto chaninfo = ci_result.value(); + + if (classifyChannelByCaps(chaninfo) != chanType) continue; + + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSPK; + chaninfo.chan = chan; + chaninfo.spkopts &= ~cbAINPSPK_EXTRACT; + if (enabled) + chaninfo.spkopts |= cbAINPSPK_EXTRACT; + + auto sr = sendPacket(reinterpret_cast(chaninfo)); + if (sr.isError()) return Result::error(sr.error()); + sent_count++; + } + send_result = Result::ok(); } + if (send_result.isError()) return Result::error(send_result.error()); - // CLIENT mode: build packets from shmem chaninfo - if (!m_impl->shmem_session) - return Result::error("No session available"); - - size_t count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS && count < nChans; ++chan) { - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) - continue; - auto chaninfo = ci_result.value(); - - if (classifyChannelByCaps(chaninfo) != chanType) - continue; - - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSPK; - chaninfo.chan = chan; - chaninfo.spkopts &= ~cbAINPSPK_EXTRACT; - if (enabled) - chaninfo.spkopts |= cbAINPSPK_EXTRACT; - - auto r = sendPacket(reinterpret_cast(chaninfo)); - if (r.isError()) - return r; - count++; - } + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - if (count == 0) - return Result::error("No channels found matching type"); - return Result::ok(); + return Result::ok(countChannelsMatching(*this, chanType, + [enabled](const cbPKT_CHANINFO& ci) { + const bool is_extracting = (ci.spkopts & cbAINPSPK_EXTRACT) != 0; + return is_extracting == enabled; + })); } -Result SdkSession::setACInputCoupling(const size_t nChans, const ChannelType chanType, - const bool enabled) { - // STANDALONE mode: delegate to device session +Result SdkSession::setACInputCoupling(const uint32_t nChans, const ChannelType chanType, + const bool enabled) { + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + + Result send_result = Result::error("No session available"); if (m_impl->device_session) { - const auto r = m_impl->device_session->setChannelsACInputCouplingByType( + send_result = m_impl->device_session->setChannelsACInputCouplingByType( nChans, toDevChannelType(chanType), enabled); - if (r.isError()) - return Result::error(r.error()); - return Result::ok(); - } - - // CLIENT mode: build packets from shmem chaninfo - if (!m_impl->shmem_session) - return Result::error("No session available"); + } else if (m_impl->shmem_session) { + uint32_t sent_count = 0; + for (uint32_t chan = 1; chan <= cbMAXCHANS && sent_count < nChans; ++chan) { + auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); + if (ci_result.isError()) continue; + auto chaninfo = ci_result.value(); - size_t count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS && count < nChans; ++chan) { - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) - continue; - auto chaninfo = ci_result.value(); - - if (classifyChannelByCaps(chaninfo) != chanType) - continue; - - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; - chaninfo.chan = chan; - if (enabled) - chaninfo.ainpopts |= cbAINP_OFFSET_CORRECT; - else - chaninfo.ainpopts &= ~cbAINP_OFFSET_CORRECT; + if (classifyChannelByCaps(chaninfo) != chanType) continue; - auto r = sendPacket(reinterpret_cast(chaninfo)); - if (r.isError()) - return r; - count++; + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; + chaninfo.chan = chan; + if (enabled) + chaninfo.ainpopts |= cbAINP_OFFSET_CORRECT; + else + chaninfo.ainpopts &= ~cbAINP_OFFSET_CORRECT; + + auto sr = sendPacket(reinterpret_cast(chaninfo)); + if (sr.isError()) return Result::error(sr.error()); + sent_count++; + } + send_result = Result::ok(); } + if (send_result.isError()) return Result::error(send_result.error()); - if (count == 0) - return Result::error("No channels found matching type"); - return Result::ok(); + if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + + return Result::ok(countChannelsMatching(*this, chanType, + [enabled](const cbPKT_CHANINFO& ci) { + const bool is_offset_correct = (ci.ainpopts & cbAINP_OFFSET_CORRECT) != 0; + return is_offset_correct == enabled; + })); } Result SdkSession::setChannelConfig(const cbPKT_CHANINFO& chaninfo) { diff --git a/tests/integration/test_capi_configuration.cpp b/tests/integration/test_capi_configuration.cpp index 4f151929..2f4779ca 100644 --- a/tests/integration/test_capi_configuration.cpp +++ b/tests/integration/test_capi_configuration.cpp @@ -131,11 +131,11 @@ TEST_F(CApiSampleGroupTest, SetFrontend30kHz) { SessionGuard sg; ASSERT_TRUE(sg.create()); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_sample_group( sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, - CBPROTO_GROUP_RATE_30000Hz, true), CBSDK_RESULT_SUCCESS); - - std::this_thread::sleep_for(std::chrono::milliseconds(500)); + CBPROTO_GROUP_RATE_30000Hz, true, &n_configured), CBSDK_RESULT_SUCCESS); + EXPECT_EQ(n_configured, 4u); // Verify via group list uint16_t list[256]; @@ -149,11 +149,11 @@ TEST_F(CApiSampleGroupTest, SetAndVerifyField) { SessionGuard sg; ASSERT_TRUE(sg.create()); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_sample_group( sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, - CBPROTO_GROUP_RATE_10000Hz, true), CBSDK_RESULT_SUCCESS); - - std::this_thread::sleep_for(std::chrono::milliseconds(500)); + CBPROTO_GROUP_RATE_10000Hz, true, &n_configured), CBSDK_RESULT_SUCCESS); + EXPECT_EQ(n_configured, 4u); // Query via bulk field getter int64_t values[512]; @@ -267,16 +267,22 @@ TEST_F(CApiACCouplingTest, SetACCoupling) { SessionGuard sg; ASSERT_TRUE(sg.create()); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, true), CBSDK_RESULT_SUCCESS); + sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, true, &n_configured), + CBSDK_RESULT_SUCCESS); + EXPECT_EQ(n_configured, 4u); } TEST_F(CApiACCouplingTest, SetDCCoupling) { SessionGuard sg; ASSERT_TRUE(sg.create()); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, false), CBSDK_RESULT_SUCCESS); + sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, false, &n_configured), + CBSDK_RESULT_SUCCESS); + EXPECT_EQ(n_configured, 4u); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -289,8 +295,11 @@ TEST_F(CApiSpikeSortingTest, SetSpikeSorting) { SessionGuard sg; ASSERT_TRUE(sg.create()); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_spike_sorting( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, 0), CBSDK_RESULT_SUCCESS); + sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, 0, &n_configured), + CBSDK_RESULT_SUCCESS); + EXPECT_EQ(n_configured, 4u); } /////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/tests/unit/test_cbsdk_c_api.cpp b/tests/unit/test_cbsdk_c_api.cpp index f1ebab21..497298f1 100644 --- a/tests/unit/test_cbsdk_c_api.cpp +++ b/tests/unit/test_cbsdk_c_api.cpp @@ -385,8 +385,9 @@ TEST_F(CbsdkCApiTest, ConfigAccess_WithSession) { /////////////////////////////////////////////////////////////////////////////////////////////////// TEST_F(CbsdkCApiTest, SetChannelSampleGroup_NullSession) { - EXPECT_EQ(cbsdk_session_set_sample_group(nullptr, 256, - CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_30000Hz, false), CBSDK_RESULT_INVALID_PARAMETER); + EXPECT_EQ(cbsdk_session_set_sample_group(nullptr, 256u, + CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_30000Hz, false, nullptr), + CBSDK_RESULT_INVALID_PARAMETER); } TEST_F(CbsdkCApiTest, SetChannelConfig_NullSession) { From 89d08e0ec9ff4f0d972992dbcb6eeed8d6c18d56 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 27 Apr 2026 19:53:12 -0400 Subject: [PATCH 3/8] Add clear_channel_map() (#178) 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) --- pycbsdk/src/pycbsdk/_cdef.py | 1 + pycbsdk/src/pycbsdk/session.py | 14 ++++++++ pycbsdk/tests/test_configuration.py | 48 ++++++++++++++++++++++++++ src/cbsdk/include/cbsdk/cbsdk.h | 10 ++++++ src/cbsdk/include/cbsdk/sdk_session.h | 11 ++++++ src/cbsdk/src/cbsdk.cpp | 12 +++++++ src/cbsdk/src/sdk_session.cpp | 49 +++++++++++++++++++++++++++ 7 files changed, 145 insertions(+) diff --git a/pycbsdk/src/pycbsdk/_cdef.py b/pycbsdk/src/pycbsdk/_cdef.py index f9f738fd..78150ff5 100644 --- a/pycbsdk/src/pycbsdk/_cdef.py +++ b/pycbsdk/src/pycbsdk/_cdef.py @@ -321,6 +321,7 @@ // CCF configuration files cbsdk_result_t cbsdk_session_load_channel_map(cbsdk_session_t session, const char* filepath, uint32_t start_chan, uint32_t hs_id); +cbsdk_result_t cbsdk_session_clear_channel_map(cbsdk_session_t session); cbsdk_result_t cbsdk_session_save_ccf(cbsdk_session_t session, const char* filename); cbsdk_result_t cbsdk_session_load_ccf(cbsdk_session_t session, const char* filename); cbsdk_result_t cbsdk_session_load_ccf_sync(cbsdk_session_t session, const char* filename, uint32_t timeout_ms); diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index 6727b0bc..24af589b 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -1219,6 +1219,20 @@ def load_channel_map(self, filepath: str, start_chan: int = 1, hs_id: int = 0): "Failed to load channel map", ) + def clear_channel_map(self): + """Remove all channel maps loaded via :meth:`load_channel_map`. + + Drops the local position+label overlay and pushes the device's + default labels (``"chan{N}"``) back to the device for every + previously-mapped channel so the device-side state matches. + Fire-and-forget; call :meth:`sync` if you need to read back state + before issuing further config calls. + """ + _check( + _get_lib().cbsdk_session_clear_channel_map(self._session), + "Failed to clear channel map", + ) + # --- CCF Configuration Files --- def save_ccf(self, filename: str): diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index c6965125..f4b26c5b 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -928,6 +928,54 @@ def test_overlay_survives_chanrep_refresh(self, nplay_session, cmp_path): assert pos == expected[chan_id].position assert label == expected[chan_id].label + def test_clear_channel_map_resets_labels(self, nplay_session, cmp_path): + """clear_channel_map() reverts labels to the device default ("chanN").""" + from pycbsdk.cmp import parse_cmp + + nplay_session.load_channel_map(str(cmp_path), hs_id=11) + nplay_session.sync() + + expected = parse_cmp(str(cmp_path), hs_id=11) + view_before = self._frontend_view(nplay_session) + loaded = sorted(set(expected) & set(view_before)) + assert loaded + for chan_id in loaded: + _, label = view_before[chan_id] + assert label.startswith("hs11-") + + nplay_session.clear_channel_map() + nplay_session.sync() + + view_after = self._frontend_view(nplay_session) + for chan_id in loaded: + _, label = view_after[chan_id] + assert label == f"chan{chan_id}", ( + f"chan {chan_id}: expected default label, got {label!r}" + ) + + def test_clear_channel_map_resets_positions(self, nplay_session, cmp_path): + """clear_channel_map() drops the position overlay; positions revert to zero.""" + nplay_session.load_channel_map(str(cmp_path)) + nplay_session.sync() + + view_before = self._frontend_view(nplay_session) + loaded = [c for c, (pos, _) in view_before.items() if any(p != 0 for p in pos)] + assert loaded, "expected at least one frontend chan with non-zero position" + + nplay_session.clear_channel_map() + nplay_session.sync() + + view_after = self._frontend_view(nplay_session) + for chan_id in loaded: + pos, _ = view_after[chan_id] + assert all(p == 0 for p in pos), ( + f"chan {chan_id}: expected zeroed position after clear, got {pos}" + ) + + def test_clear_channel_map_empty_is_noop(self, nplay_session): + """Calling clear_channel_map() with no map loaded is a no-op.""" + nplay_session.clear_channel_map() # must not raise + # --------------------------------------------------------------------------- # Spike length configuration diff --git a/src/cbsdk/include/cbsdk/cbsdk.h b/src/cbsdk/include/cbsdk/cbsdk.h index 417ab1b5..794c4693 100644 --- a/src/cbsdk/include/cbsdk/cbsdk.h +++ b/src/cbsdk/include/cbsdk/cbsdk.h @@ -888,6 +888,16 @@ CBSDK_API cbsdk_result_t cbsdk_session_load_channel_map( uint32_t start_chan, uint32_t hs_id); +/// Clear all channel maps loaded via cbsdk_session_load_channel_map(). +/// +/// Drops the local position+label overlay and pushes default labels +/// ("chan{N}") to the device for every previously-mapped channel so the +/// device-side label state reverts. Fire-and-forget. +/// +/// @param session Session handle (must not be NULL) +/// @return CBSDK_RESULT_SUCCESS on success, error code on failure +CBSDK_API cbsdk_result_t cbsdk_session_clear_channel_map(cbsdk_session_t session); + /////////////////////////////////////////////////////////////////////////////////////////////////// // CCF Configuration Files /////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/cbsdk/include/cbsdk/sdk_session.h b/src/cbsdk/include/cbsdk/sdk_session.h index ab83e34f..37e04f27 100644 --- a/src/cbsdk/include/cbsdk/sdk_session.h +++ b/src/cbsdk/include/cbsdk/sdk_session.h @@ -666,6 +666,17 @@ class SdkSession { uint32_t start_chan = 1, uint32_t hs_id = 0); + /// Clear all channel maps loaded via loadChannelMap(). + /// + /// Wipes the local position+label overlay (so positions revert) and pushes + /// default labels ("chan1", "chan2", ...) to the device for every channel + /// that was previously mapped, so the device-side label state matches. + /// Fire-and-forget: returns once labels are queued; the caller can call + /// sync() if it needs to read back state. + /// + /// @return Result indicating success or error + Result clearChannelMap(); + ///-------------------------------------------------------------------------------------------- /// CCF Configuration Files ///-------------------------------------------------------------------------------------------- diff --git a/src/cbsdk/src/cbsdk.cpp b/src/cbsdk/src/cbsdk.cpp index 25099392..bb0df2ea 100644 --- a/src/cbsdk/src/cbsdk.cpp +++ b/src/cbsdk/src/cbsdk.cpp @@ -1338,6 +1338,18 @@ cbsdk_result_t cbsdk_session_load_channel_map( } } +cbsdk_result_t cbsdk_session_clear_channel_map(cbsdk_session_t session) { + if (!session || !session->cpp_session) { + return CBSDK_RESULT_INVALID_PARAMETER; + } + try { + auto result = session->cpp_session->clearChannelMap(); + return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; + } catch (...) { + return CBSDK_RESULT_INTERNAL_ERROR; + } +} + /////////////////////////////////////////////////////////////////////////////////////////////////// // CCF Configuration Files /////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/cbsdk/src/sdk_session.cpp b/src/cbsdk/src/sdk_session.cpp index 53270a8e..f4a9f0a7 100644 --- a/src/cbsdk/src/sdk_session.cpp +++ b/src/cbsdk/src/sdk_session.cpp @@ -2139,6 +2139,55 @@ Result SdkSession::loadChannelMap( return Result::ok(); } +Result SdkSession::clearChannelMap() { + // Snapshot the previously-mapped channel ids and drop the overlay so + // future CHANREPs land in chaninfo as the device sends them. Any further + // applyCmpToAllChannels() call is now a no-op. + std::vector mapped_chans; + { + std::lock_guard lock(m_impl->cmp_mutex); + mapped_chans.reserve(m_impl->cmp_entries.size()); + for (const auto& [chan_id, _] : m_impl->cmp_entries) { + mapped_chans.push_back(chan_id); + } + m_impl->cmp_entries.clear(); + } + + // Reset shmem positions to zero for previously-mapped channels. The + // device doesn't persist positions, so there's no on-device action — just + // wipe the local overlay we applied in applyCmpToAllChannels(). + if (m_impl->shmem_session) { + for (uint32_t chan_id : mapped_chans) { + if (chan_id < 1 || chan_id > cbMAXCHANS) continue; + auto r = m_impl->shmem_session->getChanInfo(chan_id - 1); + if (r.isError()) continue; + cbPKT_CHANINFO ci = r.value(); + std::memset(ci.position, 0, sizeof(ci.position)); + m_impl->shmem_session->setChanInfo(chan_id - 1, ci); + } + } + + // Push default labels ("chan{N}") to the device so the device-side label + // state matches. Labels were modified by loadChannelMap; positions were + // local-only. + if (m_impl->device_session || m_impl->shmem_session) { + for (uint32_t chan_id : mapped_chans) { + const cbPKT_CHANINFO* info = getChanInfo(chan_id); + if (!info) continue; + cbPKT_CHANINFO ci = *info; + ci.chan = chan_id; + ci.cbpkt_header.type = cbPKTTYPE_CHANSETLABEL; + char default_label[16]; + std::snprintf(default_label, sizeof(default_label), "chan%u", chan_id); + std::strncpy(ci.label, default_label, sizeof(ci.label) - 1); + ci.label[sizeof(ci.label) - 1] = '\0'; + (void)setChannelConfig(ci); // best-effort + } + } + + return Result::ok(); +} + /////////////////////////////////////////////////////////////////////////////////////////////////// // CCF Configuration Files /////////////////////////////////////////////////////////////////////////////////////////////////// From 32dde5a77acce6197b8e8a8af2e673b495296266 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 27 Apr 2026 19:58:36 -0400 Subject: [PATCH 4/8] Add runlevel-change callback + async wait_until_running (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pycbsdk/src/pycbsdk/_cdef.py | 3 + pycbsdk/src/pycbsdk/session.py | 83 +++++++++++++++++++++++++++ pycbsdk/tests/test_configuration.py | 34 +++++++++++ src/cbsdk/include/cbsdk/cbsdk.h | 18 ++++++ src/cbsdk/include/cbsdk/sdk_session.h | 15 +++++ src/cbsdk/src/cbsdk.cpp | 18 ++++++ src/cbsdk/src/sdk_session.cpp | 30 +++++++++- 7 files changed, 199 insertions(+), 2 deletions(-) diff --git a/pycbsdk/src/pycbsdk/_cdef.py b/pycbsdk/src/pycbsdk/_cdef.py index 78150ff5..6841c24d 100644 --- a/pycbsdk/src/pycbsdk/_cdef.py +++ b/pycbsdk/src/pycbsdk/_cdef.py @@ -135,6 +135,7 @@ size_t n_channels, const uint64_t* timestamps, void* user_data); typedef void (*cbsdk_config_callback_fn)(const cbPKT_GENERIC* pkt, void* user_data); +typedef void (*cbsdk_runlevel_callback_fn)(uint32_t runlevel, void* user_data); typedef void (*cbsdk_error_callback_fn)(const char* error_message, void* user_data); /////////////////////////////////////////////////////////////////////////// @@ -178,6 +179,8 @@ cbsdk_callback_handle_t cbsdk_session_register_config_callback( cbsdk_session_t session, uint16_t packet_type, cbsdk_config_callback_fn callback, void* user_data); +cbsdk_callback_handle_t cbsdk_session_register_runlevel_callback( + cbsdk_session_t session, cbsdk_runlevel_callback_fn callback, void* user_data); void cbsdk_session_unregister_callback(cbsdk_session_t session, cbsdk_callback_handle_t handle); diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index 24af589b..b690e680 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -4,6 +4,7 @@ from __future__ import annotations +import asyncio import enum import time as _time import threading @@ -12,6 +13,17 @@ from ._lib import ffi, load_library +# Device runlevels (cbRUNLEVEL_*). Mirrors values in cbproto types.h. +RUNLEVEL_STARTUP = 10 +RUNLEVEL_HARDRESET = 20 +RUNLEVEL_STANDBY = 30 +RUNLEVEL_RESET = 40 +RUNLEVEL_RUNNING = 50 +RUNLEVEL_STRESSED = 60 +RUNLEVEL_ERROR = 70 +RUNLEVEL_UPDATE = 78 +RUNLEVEL_SHUTDOWN = 80 + # "All matching" sentinel for bulk channel-config setters. _ALL_CHANS = 0xFFFFFFFF @@ -503,6 +515,31 @@ def c_config_cb(pkt, user_data): self._handles.append(handle) self._callback_refs.append(c_config_cb) + def _register_runlevel_callback(self, fn: Callable[[int], None]) -> int: + """Register a runlevel-change callback. Returns the callback handle. + + The callback is invoked on the SDK's receive thread (or shmem-receive + thread in CLIENT mode), so ``fn`` must be fast and thread-safe. For + async use, see :meth:`wait_for_runlevel`. + """ + _lib = _get_lib() + + @ffi.callback("void(uint32_t, void*)") + def c_runlevel_cb(runlevel, user_data): + try: + fn(int(runlevel)) + except Exception: + pass # Never let exceptions propagate into C + + handle = _lib.cbsdk_session_register_runlevel_callback( + self._session, c_runlevel_cb, ffi.NULL + ) + if handle == 0: + raise RuntimeError("Failed to register runlevel callback") + self._handles.append(handle) + self._callback_refs.append(c_runlevel_cb) + return handle + def _register_packet_callback(self, fn): _lib = _get_lib() @@ -557,6 +594,52 @@ def runlevel(self) -> int: """Get the current device run level.""" return _get_lib().cbsdk_session_get_runlevel(self._session) + async def wait_for_runlevel(self, level: int, timeout: float = 10.0) -> None: + """Resolve when the device runlevel reaches ``level``. + + Returns immediately if ``self.runlevel >= level`` at call time. + Otherwise registers a one-shot listener for the next runlevel + transition matching the predicate and resolves on its arrival. + + Args: + level: Target runlevel (use one of the ``RUNLEVEL_*`` module + constants, e.g. :data:`RUNLEVEL_RUNNING`). + timeout: Seconds to wait before raising + :class:`asyncio.TimeoutError`. + """ + if self.runlevel >= level: + return + + loop = asyncio.get_running_loop() + fut: asyncio.Future = loop.create_future() + + def _on_change(rl: int) -> None: + if rl >= level and not fut.done(): + loop.call_soon_threadsafe(_resolve, rl) + + def _resolve(_rl: int) -> None: + if not fut.done(): + fut.set_result(None) + + handle = self._register_runlevel_callback(_on_change) + try: + await asyncio.wait_for(fut, timeout) + finally: + _get_lib().cbsdk_session_unregister_callback(self._session, handle) + # Drop our local handle reference so close() doesn't double-free. + try: + self._handles.remove(handle) + except ValueError: + pass + + async def wait_until_running(self, timeout: float = 10.0) -> None: + """Resolve when the device runlevel reaches ``RUNLEVEL_RUNNING``. + + Thin wrapper over :meth:`wait_for_runlevel` for the common case of + gating boot-time configuration on device readiness. + """ + await self.wait_for_runlevel(RUNLEVEL_RUNNING, timeout) + @property def is_standalone(self) -> bool: """Whether this session owns the device connection (STANDALONE mode). diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index f4b26c5b..2bddaf78 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -472,6 +472,40 @@ def test_set_spike_extraction_returns_count(self, nplay_session): assert n_off == n_fe +# --------------------------------------------------------------------------- +# Async wait_until_running +# --------------------------------------------------------------------------- + + +class TestWaitUntilRunning: + """Async runlevel-awaitable. + + The fixture brings the device up to RUNNING before yielding the session, + so wait_until_running() returns immediately without registering a + listener; covering the registration path requires fault injection that + isn't worth the test complexity. + """ + + def test_returns_immediately_when_running(self, nplay_session): + import asyncio + from pycbsdk.session import RUNLEVEL_RUNNING + + async def run() -> None: + assert nplay_session.runlevel >= RUNLEVEL_RUNNING + await nplay_session.wait_until_running(timeout=1.0) + + asyncio.run(run()) + + def test_wait_for_runlevel_immediate(self, nplay_session): + import asyncio + from pycbsdk.session import RUNLEVEL_STANDBY + + async def run() -> None: + await nplay_session.wait_for_runlevel(RUNLEVEL_STANDBY, timeout=1.0) + + asyncio.run(run()) + + # --------------------------------------------------------------------------- # Per-channel configuration (configure_channel) # --------------------------------------------------------------------------- diff --git a/src/cbsdk/include/cbsdk/cbsdk.h b/src/cbsdk/include/cbsdk/cbsdk.h index 794c4693..ede313c8 100644 --- a/src/cbsdk/include/cbsdk/cbsdk.h +++ b/src/cbsdk/include/cbsdk/cbsdk.h @@ -340,6 +340,24 @@ CBSDK_API cbsdk_callback_handle_t cbsdk_session_register_config_callback( cbsdk_config_callback_fn callback, void* user_data); +/// Callback type for device-runlevel transitions. +typedef void (*cbsdk_runlevel_callback_fn)(uint32_t runlevel, void* user_data); + +/// Register a callback that fires whenever the device-reported runlevel +/// transitions to a new value (driven by SYSREP packets). Useful for +/// awaiting cbRUNLEVEL_RUNNING after a reset. The callback runs on the +/// receive thread (STANDALONE) or shmem-receive thread (CLIENT) — keep it +/// fast or post into a queue. +/// @param session Session handle (must not be NULL) +/// @param callback Function to call on each transition (must not be NULL) +/// @param user_data Opaque user pointer passed to @p callback +/// @return Callback handle for later cbsdk_session_unregister_callback, +/// or 0 on error +CBSDK_API cbsdk_callback_handle_t cbsdk_session_register_runlevel_callback( + cbsdk_session_t session, + cbsdk_runlevel_callback_fn callback, + void* user_data); + /// Unregister a previously registered callback /// @param session Session handle (must not be NULL) /// @param handle Handle returned by a register_*_callback function diff --git a/src/cbsdk/include/cbsdk/sdk_session.h b/src/cbsdk/include/cbsdk/sdk_session.h index 37e04f27..b0f33253 100644 --- a/src/cbsdk/include/cbsdk/sdk_session.h +++ b/src/cbsdk/include/cbsdk/sdk_session.h @@ -269,6 +269,11 @@ using GroupBatchCallback = std::function; +/// Runlevel-change callback. Fired when the device runlevel reported in a +/// SYSREP packet differs from the previously recorded value. +/// @param runlevel The new runlevel (cbRUNLEVEL_*) +using RunlevelCallback = std::function; + /// Error callback for queue overflow and other errors /// @param error_message Description of the error using ErrorCallback = std::function; @@ -377,6 +382,16 @@ class SdkSession { /// @return Handle for unregistration CallbackHandle registerConfigCallback(uint16_t packet_type, ConfigCallback callback) const; + /// Register callback for device-runlevel transitions. + /// Fires when the runlevel reported in a SYSREP packet differs from the + /// previously recorded value. Useful for waiting on the device to reach + /// cbRUNLEVEL_RUNNING after a reset. Callbacks run on the receive + /// thread (STANDALONE) or shmem-receive thread (CLIENT) — keep them fast, + /// or post to your own queue. + /// @param callback Function to call on each transition (receives new runlevel) + /// @return Handle for unregistration + CallbackHandle registerRunlevelChangeCallback(RunlevelCallback callback) const; + /// Unregister a previously registered callback /// @param handle Handle returned by any register*Callback method void unregisterCallback(CallbackHandle handle) const; diff --git a/src/cbsdk/src/cbsdk.cpp b/src/cbsdk/src/cbsdk.cpp index bb0df2ea..96fdc107 100644 --- a/src/cbsdk/src/cbsdk.cpp +++ b/src/cbsdk/src/cbsdk.cpp @@ -578,6 +578,24 @@ cbsdk_callback_handle_t cbsdk_session_register_config_callback( } } +cbsdk_callback_handle_t cbsdk_session_register_runlevel_callback( + cbsdk_session_t session, + cbsdk_runlevel_callback_fn callback, + void* user_data) { + if (!session || !session->cpp_session || !callback) { + return 0; + } + try { + return session->cpp_session->registerRunlevelChangeCallback( + [callback, user_data](uint32_t runlevel) { + callback(runlevel, user_data); + } + ); + } catch (...) { + return 0; + } +} + void cbsdk_session_unregister_callback(cbsdk_session_t session, cbsdk_callback_handle_t handle) { if (!session || !session->cpp_session || handle == 0) { diff --git a/src/cbsdk/src/sdk_session.cpp b/src/cbsdk/src/sdk_session.cpp index f4a9f0a7..10366b31 100644 --- a/src/cbsdk/src/sdk_session.cpp +++ b/src/cbsdk/src/sdk_session.cpp @@ -242,12 +242,30 @@ struct SdkSession::Impl { struct GroupCB { CallbackHandle handle; uint8_t group_id; GroupCallback cb; }; struct GroupBatchCB { CallbackHandle handle; uint8_t group_id; GroupBatchCallback cb; }; struct ConfigCB { CallbackHandle handle; uint16_t packet_type; ConfigCallback cb; }; + struct RunlevelCB { CallbackHandle handle; RunlevelCallback cb; }; std::vector packet_callbacks; std::vector event_callbacks; std::vector group_callbacks; std::vector group_batch_callbacks; std::vector config_callbacks; + std::vector runlevel_callbacks; + + /// Atomically update device_runlevel; fire registered callbacks if the + /// value changed. Called from the receive thread (STANDALONE) or the + /// shmem-receive thread (CLIENT) — both paths converge here. + void updateRunlevel(uint32_t new_runlevel) { + const uint32_t prev = device_runlevel.exchange(new_runlevel, std::memory_order_acq_rel); + if (prev == new_runlevel) return; + std::vector snap; + { + std::lock_guard lock(user_callback_mutex); + snap = runlevel_callbacks; + } + for (const auto& cb : snap) { + if (cb.cb) cb.cb(new_runlevel); + } + } CallbackHandle next_callback_handle = 1; ErrorCallback error_callback; @@ -872,7 +890,7 @@ Result SdkSession::start() { // Check for SYSREP packets (handshake responses) if ((pkt.cbpkt_header.type & 0xF0) == cbPKTTYPE_SYSREP) { const auto* sysinfo = reinterpret_cast(&pkt); - impl->device_runlevel.store(sysinfo->runlevel, std::memory_order_release); + impl->updateRunlevel(sysinfo->runlevel); impl->received_sysrep.store(true, std::memory_order_release); impl->handshake_cv.notify_all(); } @@ -1183,7 +1201,7 @@ Result SdkSession::start() { // Check for SYSREP packets (handshake responses) if ((packets[i].cbpkt_header.type & 0xF0) == cbPKTTYPE_SYSREP) { const auto* sysinfo = reinterpret_cast(&packets[i]); - impl->device_runlevel.store(sysinfo->runlevel, std::memory_order_release); + impl->updateRunlevel(sysinfo->runlevel); impl->received_sysrep.store(true, std::memory_order_release); impl->handshake_cv.notify_all(); } @@ -1320,6 +1338,13 @@ CallbackHandle SdkSession::registerConfigCallback(const uint16_t packet_type, Co return handle; } +CallbackHandle SdkSession::registerRunlevelChangeCallback(RunlevelCallback callback) const { + std::lock_guard lock(m_impl->user_callback_mutex); + const auto handle = m_impl->next_callback_handle++; + m_impl->runlevel_callbacks.push_back({handle, std::move(callback)}); + return handle; +} + void SdkSession::unregisterCallback(CallbackHandle handle) const { std::lock_guard lock(m_impl->user_callback_mutex); auto erase_by_handle = [handle](auto& vec) { @@ -1332,6 +1357,7 @@ void SdkSession::unregisterCallback(CallbackHandle handle) const { erase_by_handle(m_impl->group_callbacks); erase_by_handle(m_impl->group_batch_callbacks); erase_by_handle(m_impl->config_callbacks); + erase_by_handle(m_impl->runlevel_callbacks); } void SdkSession::setErrorCallback(ErrorCallback callback) { From 518d66c7631893b0683f3a7b0cff4c02500ba503 Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 27 Apr 2026 21:57:44 -0400 Subject: [PATCH 5/8] Bulk setters: support explicit channel-id list (#181 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pycbsdk/src/pycbsdk/_cdef.py | 25 +- pycbsdk/src/pycbsdk/session.py | 78 +++-- pycbsdk/tests/test_configuration.py | 111 +++++++ src/cbsdk/include/cbsdk/cbsdk.h | 41 ++- src/cbsdk/include/cbsdk/sdk_session.h | 61 +++- src/cbsdk/src/cbsdk.cpp | 12 +- src/cbsdk/src/sdk_session.cpp | 301 +++++++++--------- tests/integration/test_capi_configuration.cpp | 13 +- tests/unit/test_cbsdk_c_api.cpp | 2 +- 9 files changed, 429 insertions(+), 215 deletions(-) diff --git a/pycbsdk/src/pycbsdk/_cdef.py b/pycbsdk/src/pycbsdk/_cdef.py index 6841c24d..86c9a422 100644 --- a/pycbsdk/src/pycbsdk/_cdef.py +++ b/pycbsdk/src/pycbsdk/_cdef.py @@ -207,13 +207,18 @@ cbsdk_result_t cbsdk_session_get_group_list(cbsdk_session_t session, uint32_t group_id, uint16_t* list, uint32_t* count); -// Channel configuration +// Channel configuration. +// `chans` is an optional explicit list of 1-based channel ids. Pass NULL +// for the legacy "first n_chans matching" / "all matching" (n_chans=UINT32_MAX) +// behavior; pass a non-NULL pointer for the explicit-list mode. cbsdk_result_t cbsdk_session_set_sample_group( - cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, - cbproto_group_rate_t rate, _Bool disable_others, uint32_t* out_n_configured); + cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, + cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, + _Bool disable_others, uint32_t* out_n_configured); cbsdk_result_t cbsdk_session_set_ac_input_coupling( - cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, - _Bool enabled, uint32_t* out_n_configured); + cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, + cbproto_channel_type_t chan_type, _Bool enabled, + uint32_t* out_n_configured); // Per-channel getters cbproto_channel_type_t cbsdk_session_get_channel_type(cbsdk_session_t session, uint32_t chan_id); @@ -339,15 +344,17 @@ // Spike sorting cbsdk_result_t cbsdk_session_set_spike_sorting( - cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, - uint32_t sort_options, uint32_t* out_n_configured); + cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, + cbproto_channel_type_t chan_type, uint32_t sort_options, + uint32_t* out_n_configured); cbsdk_result_t cbsdk_session_set_channel_spike_sorting( cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options, int auto_sync); // Spike extraction (enable/disable cbAINPSPK_EXTRACT via CHANSETSPK) cbsdk_result_t cbsdk_session_set_spike_extraction( - cbsdk_session_t session, uint32_t n_chans, cbproto_channel_type_t chan_type, - bool enabled, uint32_t* out_n_configured); + cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, + cbproto_channel_type_t chan_type, bool enabled, + uint32_t* out_n_configured); // Clock synchronization cbsdk_result_t cbsdk_session_get_clock_offset(cbsdk_session_t session, int64_t* offset_ns); diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index b690e680..ca603f0e 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -987,18 +987,42 @@ def get_channels_positions( # --- Channel Configuration --- + @staticmethod + def _normalize_chans(chans): + """Resolve ``chans`` (``int | Iterable[int] | None``) for the C-API. + + Returns ``(n_chans, c_arr)`` where ``c_arr`` is either ``ffi.NULL`` + (count-based mode) or an ffi-owned ``uint32_t[]`` (explicit-list + mode). Caller must keep ``c_arr`` alive across the C call. + """ + if chans is None: + return _ALL_CHANS, ffi.NULL + if isinstance(chans, (bool,)): + raise TypeError("chans must be an int, an iterable of ints, or None") + if isinstance(chans, int): + return int(chans), ffi.NULL + chan_list = [int(c) for c in chans] + return len(chan_list), ffi.new("uint32_t[]", chan_list) + def set_sample_group( self, - chans: int | None, + chans: "int | list[int] | None", channel_type: ChannelType, rate: SampleRate, disable_others: bool = False, ) -> int: """Set sampling rate for channels of a specific type. - Configures the first *chans* channels matching *channel_type* (or - every matching channel when *chans* is ``None``). To configure a - specific channel by ID, use :meth:`set_channel_smpgroup`. + ``chans`` selects the channels to configure: + + - ``None``: every channel matching *channel_type*. + - ``int N``: the first N channels matching *channel_type* in + channel-id order. + - ``list[int]`` (or any iterable of ints): the explicit list of + 1-based channel IDs. Caller is trusted; no type filter is + applied to listed channels, but *channel_type* still defines + the "others" set for *disable_others* and the post-config + count. Self-synchronizing: runs :meth:`sync` before reading the local cache (so prior in-flight config has landed) and again after sending, so @@ -1015,24 +1039,25 @@ def set_sample_group( ``cbAINP_RAWSTREAM`` bit instead. Args: - chans: Number of channels to configure, or ``None`` for all - matching. - channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). + chans: Channel selection (see above). + channel_type: Channel type filter. rate: Sample rate (e.g., ``SampleRate.SR_30kHz``, ``SampleRate.NONE`` to disable). - disable_others: Disable sampling on unselected channels. + disable_others: Disable sampling on channels of *channel_type* + that are not in the configured set. Returns: Count of *channel_type* channels whose post-config state matches *rate*. """ _lib = _get_lib() - n_chans = _ALL_CHANS if chans is None else int(chans) + n_chans, c_arr = self._normalize_chans(chans) out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_sample_group( self._session, n_chans, + c_arr, int(_coerce_enum(ChannelType, channel_type)), int(_coerce_enum(SampleRate, rate, _RATE_ALIASES)), disable_others, @@ -1044,18 +1069,18 @@ def set_sample_group( def set_ac_input_coupling( self, - chans: int | None, + chans: "int | list[int] | None", channel_type: ChannelType, enabled: bool, ) -> int: """Set AC/DC input coupling for channels of a specific type. - Self-synchronizing — see :meth:`set_sample_group` for the contract. + Self-synchronizing — see :meth:`set_sample_group` for the channel + selection and contract. Args: - chans: Number of channels to configure, or ``None`` for all - matching. - channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). + chans: Channel selection (see :meth:`set_sample_group`). + channel_type: Channel type filter. enabled: ``True`` for AC coupling (offset correction on), ``False`` for DC coupling. @@ -1064,12 +1089,13 @@ def set_ac_input_coupling( ``cbAINP_OFFSET_CORRECT`` bit matches *enabled*. """ _lib = _get_lib() - n_chans = _ALL_CHANS if chans is None else int(chans) + n_chans, c_arr = self._normalize_chans(chans) out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_ac_input_coupling( self._session, n_chans, + c_arr, int(_coerce_enum(ChannelType, channel_type)), enabled, out_n, @@ -1520,17 +1546,17 @@ def close_central_file_dialog(self): def set_spike_sorting( self, - chans: int | None, + chans: "int | list[int] | None", channel_type: ChannelType, sort_options: int, ) -> int: """Set spike sorting options for channels of a specific type. - Self-synchronizing — see :meth:`set_sample_group` for the contract. + Self-synchronizing — see :meth:`set_sample_group` for the channel + selection and contract. Args: - chans: Number of channels to configure, or ``None`` for all - matching. + chans: Channel selection (see :meth:`set_sample_group`). channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). sort_options: Spike sorting option flags (cbAINPSPK_*). @@ -1539,12 +1565,13 @@ def set_spike_sorting( ``spkopts & cbAINPSPK_ALLSORT`` matches *sort_options*. """ _lib = _get_lib() - n_chans = _ALL_CHANS if chans is None else int(chans) + n_chans, c_arr = self._normalize_chans(chans) out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_spike_sorting( self._session, n_chans, + c_arr, int(_coerce_enum(ChannelType, channel_type)), sort_options, out_n, @@ -1578,7 +1605,7 @@ def set_channel_spike_sorting( def set_spike_extraction( self, - chans: int | None, + chans: "int | list[int] | None", channel_type: ChannelType, enabled: bool, ) -> int: @@ -1587,11 +1614,11 @@ def set_spike_extraction( Controls the ``cbAINPSPK_EXTRACT`` bit which determines whether the device emits spike event packets. Uses ``cbPKTTYPE_CHANSETSPK``. - Self-synchronizing — see :meth:`set_sample_group` for the contract. + Self-synchronizing — see :meth:`set_sample_group` for the channel + selection and contract. Args: - chans: Number of channels to configure, or ``None`` for all - matching. + chans: Channel selection (see :meth:`set_sample_group`). channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). enabled: ``True`` to enable spike extraction, ``False`` to disable. @@ -1600,12 +1627,13 @@ def set_spike_extraction( ``cbAINPSPK_EXTRACT`` bit matches *enabled*. """ _lib = _get_lib() - n_chans = _ALL_CHANS if chans is None else int(chans) + n_chans, c_arr = self._normalize_chans(chans) out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_spike_extraction( self._session, n_chans, + c_arr, int(_coerce_enum(ChannelType, channel_type)), enabled, out_n, diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index 2bddaf78..9a0232d5 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -472,6 +472,117 @@ def test_set_spike_extraction_returns_count(self, nplay_session): assert n_off == n_fe +# --------------------------------------------------------------------------- +# Explicit channel-list mode for bulk setters +# --------------------------------------------------------------------------- + + +class TestBulkSettersChanList: + """``chans`` accepts an explicit list of 1-based channel ids.""" + + def test_disjoint_ranges_via_two_calls(self, nplay_session): + """Configure chans 1-2 to one rate, chans 3-4 to another.""" + n_lo = nplay_session.set_sample_group( + [1, 2], ChannelType.FRONTEND, SampleRate.SR_1kHz, + disable_others=False, + ) + n_hi = nplay_session.set_sample_group( + [3, 4], ChannelType.FRONTEND, SampleRate.SR_2kHz, + disable_others=False, + ) + # Returned counts cover all channels of the type matching each rate. + # With only 4 chans in the nplay fixture, the second call's count + # is the count of FE chans at SR_2kHz post-config = 2. + assert n_lo >= 2 + assert n_hi >= 2 + assert 1 in nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) + assert 2 in nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) + assert 3 in nplay_session.get_group_channels(int(SampleRate.SR_2kHz)) + assert 4 in nplay_session.get_group_channels(int(SampleRate.SR_2kHz)) + + def test_disjoint_set(self, nplay_session): + """Non-contiguous list of channels is configured correctly.""" + # First clear any prior state on these chans. + nplay_session.set_sample_group( + None, ChannelType.FRONTEND, SampleRate.NONE, disable_others=True, + ) + nplay_session.set_sample_group( + [1, 3], ChannelType.FRONTEND, SampleRate.SR_30kHz, + disable_others=False, + ) + ch_30k = nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) + assert 1 in ch_30k + assert 3 in ch_30k + assert 2 not in ch_30k + + def test_disable_others_with_list(self, nplay_session): + """disable_others=True with an explicit list disables every FE chan + that isn't in the list.""" + n_fe = nplay_session.num_fe_chans() + # Enable everything at 30kHz first. + nplay_session.set_sample_group( + None, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, + ) + # Now keep only chans 1, 2 at 1kHz; disable everything else. + n = nplay_session.set_sample_group( + [1, 2], ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=True, + ) + assert n == 2 + ch_1k = nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) + ch_30k = nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) + assert sorted(ch_1k) == [1, 2] + assert ch_30k == [] or set(ch_30k).isdisjoint({1, 2}) and not ch_30k + if n_fe > 2: + # The not-listed FE chans should be disabled (smpgroup=0). + for chan_id in range(3, n_fe + 1): + assert chan_id not in ch_30k + + def test_list_with_set_ac_input_coupling(self, nplay_session): + """Explicit list works for set_ac_input_coupling.""" + n_on = nplay_session.set_ac_input_coupling( + [1, 3], ChannelType.FRONTEND, True, + ) + # post-config count = at least 2 chans with offset_correct on + assert n_on >= 2 + + def test_list_with_set_spike_extraction(self, nplay_session): + """Explicit list works for set_spike_extraction.""" + # First enable extraction everywhere + nplay_session.set_spike_extraction( + None, ChannelType.FRONTEND, True, + ) + # Then disable just chans 1, 3 + n_on = nplay_session.set_spike_extraction( + [1, 3], ChannelType.FRONTEND, False, + ) + # After: chans 1, 3 disabled; rest enabled. Returned count is the + # number of FE chans with EXTRACT == False (i.e., 2). + assert n_on == 2 + + def test_empty_list_with_disable_others(self, nplay_session): + """Empty list + disable_others disables all FE chans.""" + # Enable everything first + nplay_session.set_sample_group( + None, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, + ) + # Empty list with disable_others: nothing configured, all disabled. + n = nplay_session.set_sample_group( + [], ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=True, + ) + assert n == 0 + assert nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) == [] + assert nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) == [] + + def test_tuple_and_range_accepted(self, nplay_session): + """chans accepts any iterable of ints, not just list.""" + nplay_session.set_sample_group( + (1, 2), ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=False, + ) + nplay_session.set_sample_group( + range(1, 3), ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=False, + ) + + # --------------------------------------------------------------------------- # Async wait_until_running # --------------------------------------------------------------------------- diff --git a/src/cbsdk/include/cbsdk/cbsdk.h b/src/cbsdk/include/cbsdk/cbsdk.h index ede313c8..ae1dc8bd 100644 --- a/src/cbsdk/include/cbsdk/cbsdk.h +++ b/src/cbsdk/include/cbsdk/cbsdk.h @@ -569,27 +569,41 @@ CBSDK_API cbsdk_result_t cbsdk_session_get_group_list( /// for every in-scope channel — no skip-if-already-correct optimization, /// since the local cache may be stale due to a dropped CHANREP. /// +/// Channel selection has two modes: +/// - @p chans is NULL: configure the first @p n_chans channels of +/// @p chan_type in channel-id order. Use UINT32_MAX for all matching. +/// - @p chans is non-NULL: configure the explicit list of @p n_chans +/// 1-based channel ids. No type filtering on listed channels (caller +/// is trusted), but @p chan_type still selects the "others" set for +/// @p disable_others and the post-config count. +/// /// @param session Session handle (must not be NULL) -/// @param n_chans Channels of @p chan_type to configure, in ascending -/// channel-id order. Use UINT32_MAX for all matching. -/// @param chan_type Channel type filter +/// @param n_chans When @p chans is NULL: count to configure (UINT32_MAX +/// for all matching). When @p chans is non-NULL: list length. +/// @param chans Optional explicit list of 1-based channel ids; pass NULL +/// for the count/UINT32_MAX-based mode. +/// @param chan_type Channel type filter (matching when @p chans is NULL, +/// and "others" set + count regardless). /// @param rate Sample rate (CBPROTO_GROUP_RATE_NONE to disable, _500Hz through _RAW) -/// @param disable_others If true, disable sampling on unselected channels of this type +/// @param disable_others If true, disable sampling on unselected channels of @p chan_type. /// @param[out] out_n_configured Receives the count of @p chan_type channels /// whose post-config state matches @p rate. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_sample_group( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, bool disable_others, uint32_t* out_n_configured); /// Set AC input coupling (offset correction) for channels of a specific type. -/// See cbsdk_session_set_sample_group() for the auto-sync contract. +/// See cbsdk_session_set_sample_group() for the channel-selection and +/// auto-sync contract. /// @param session Session handle (must not be NULL) -/// @param n_chans Channels to configure (UINT32_MAX = all matching) +/// @param n_chans Count or list length (see cbsdk_session_set_sample_group) +/// @param chans Optional explicit list of 1-based channel ids /// @param chan_type Channel type filter /// @param enabled true = AC coupling, false = DC coupling /// @param[out] out_n_configured Count of @p chan_type channels whose @@ -598,6 +612,7 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_sample_group( CBSDK_API cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, bool enabled, uint32_t* out_n_configured); @@ -1040,9 +1055,11 @@ CBSDK_API cbsdk_result_t cbsdk_session_close_central_file_dialog(cbsdk_session_t /////////////////////////////////////////////////////////////////////////////////////////////////// /// Set spike sorting options for channels of a specific type. -/// See cbsdk_session_set_sample_group() for the auto-sync contract. +/// See cbsdk_session_set_sample_group() for the channel-selection and +/// auto-sync contract. /// @param session Session handle (must not be NULL) -/// @param n_chans Channels to configure (UINT32_MAX = all matching) +/// @param n_chans Count or list length (see cbsdk_session_set_sample_group) +/// @param chans Optional explicit list of 1-based channel ids /// @param chan_type Channel type filter /// @param sort_options Spike sorting option flags (cbAINPSPK_*) /// @param[out] out_n_configured Count of @p chan_type channels whose @@ -1051,6 +1068,7 @@ CBSDK_API cbsdk_result_t cbsdk_session_close_central_file_dialog(cbsdk_session_t CBSDK_API cbsdk_result_t cbsdk_session_set_spike_sorting( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, uint32_t sort_options, uint32_t* out_n_configured); @@ -1071,9 +1089,11 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spike_sorting( /// Enable or disable spike extraction for channels of a specific type. /// Controls the cbAINPSPK_EXTRACT bit via cbPKTTYPE_CHANSETSPK. /// When enabled, the device emits spike event packets for matching channels. -/// See cbsdk_session_set_sample_group() for the auto-sync contract. +/// See cbsdk_session_set_sample_group() for the channel-selection and +/// auto-sync contract. /// @param session Session handle (must not be NULL) -/// @param n_chans Channels to configure (UINT32_MAX = all matching) +/// @param n_chans Count or list length (see cbsdk_session_set_sample_group) +/// @param chans Optional explicit list of 1-based channel ids /// @param chan_type Channel type filter /// @param enabled true = enable spike extraction, false = disable /// @param[out] out_n_configured Count of @p chan_type channels whose @@ -1082,6 +1102,7 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spike_sorting( CBSDK_API cbsdk_result_t cbsdk_session_set_spike_extraction( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, bool enabled, uint32_t* out_n_configured); diff --git a/src/cbsdk/include/cbsdk/sdk_session.h b/src/cbsdk/include/cbsdk/sdk_session.h index b0f33253..bde93a3a 100644 --- a/src/cbsdk/include/cbsdk/sdk_session.h +++ b/src/cbsdk/include/cbsdk/sdk_session.h @@ -533,44 +533,66 @@ class SdkSession { /// look already configured, since the local cache may be stale due to /// dropped CHANREP packets from a concurrent client. /// - /// @param nChans Number of channels of @p chanType to configure, in - /// ascending channel-id order. Use UINT32_MAX for all matching. - /// @param chanType Channel type filter + /// Channel selection has two modes: + /// - @p chans is nullptr: configure the first @p nChans channels of + /// @p chanType in channel-id order. Use UINT32_MAX for all matching. + /// - @p chans is non-null: configure the explicit list of @p nChans + /// 1-based channel ids. No type filtering is performed on the listed + /// channels (caller is trusted), but @p chanType still selects the + /// "others" set for @p disableOthers and the post-config count. + /// + /// @param nChans When @p chans is nullptr: count to configure (UINT32_MAX + /// for all matching). When @p chans is non-null: list length. + /// @param chanType Channel type filter (used for matching when @p chans + /// is nullptr, and for the "others" set + count regardless). /// @param rate Desired sample rate (NONE to disable, SR_500 through SR_RAW) - /// @param disableOthers Disable sampling on channels not in the first - /// @p nChans of @p chanType + /// @param disableOthers Disable sampling on channels of @p chanType not + /// in the configured set. + /// @param chans Optional explicit list of 1-based channel ids; must be + /// non-null only when caller wants the explicit-list mode. /// @return Number of channels of @p chanType whose post-config state /// matches @p rate, or error. Result setSampleGroup(uint32_t nChans, ChannelType chanType, - SampleRate rate, bool disableOthers = false); + SampleRate rate, bool disableOthers = false, + const uint32_t* chans = nullptr); /// Set spike sorting options for channels of a specific type. - /// See setSampleGroup() for the auto-sync and idempotency contract. - /// @param nChans Channels to configure (UINT32_MAX = all matching) + /// See setSampleGroup() for the channel-selection and auto-sync contract. + /// @param nChans Count or list length (see setSampleGroup) /// @param chanType Channel type filter /// @param sortOptions Spike sorting option flags (cbAINPSPK_*) + /// @param chans Optional explicit list of 1-based channel ids /// @return Count of @p chanType channels whose spkopts ALLSORT bits /// match @p sortOptions post-sync, or error. Result setSpikeSorting(uint32_t nChans, ChannelType chanType, - uint32_t sortOptions); + uint32_t sortOptions, + const uint32_t* chans = nullptr); /// Enable or disable spike extraction (cbAINPSPK_EXTRACT) for channels - /// of a type. See setSampleGroup() for the auto-sync contract. - /// @param nChans Channels to configure (UINT32_MAX = all matching) + /// of a type. See setSampleGroup() for the channel-selection and + /// auto-sync contract. + /// @param nChans Count or list length (see setSampleGroup) /// @param chanType Channel type filter /// @param enabled true = enable spike extraction, false = disable + /// @param chans Optional explicit list of 1-based channel ids /// @return Count of @p chanType channels whose EXTRACT bit matches /// @p enabled post-sync, or error. - Result setSpikeExtraction(uint32_t nChans, ChannelType chanType, bool enabled); + Result setSpikeExtraction(uint32_t nChans, ChannelType chanType, + bool enabled, + const uint32_t* chans = nullptr); /// Set AC input coupling (offset correction) for channels of a specific - /// type. See setSampleGroup() for the auto-sync contract. - /// @param nChans Channels to configure (UINT32_MAX = all matching) + /// type. See setSampleGroup() for the channel-selection and auto-sync + /// contract. + /// @param nChans Count or list length (see setSampleGroup) /// @param chanType Channel type filter /// @param enabled true = AC coupling (offset correction on), false = DC coupling + /// @param chans Optional explicit list of 1-based channel ids /// @return Count of @p chanType channels whose OFFSET_CORRECT bit /// matches @p enabled post-sync, or error. - Result setACInputCoupling(uint32_t nChans, ChannelType chanType, bool enabled); + Result setACInputCoupling(uint32_t nChans, ChannelType chanType, + bool enabled, + const uint32_t* chans = nullptr); /// Set full channel configuration by packet (fire-and-forget). /// Call sync() before reading back state that depends on this configuration. @@ -773,6 +795,15 @@ class SdkSession { /// @return Result indicating success or error Result sendPacket(const cbPKT_GENERIC& pkt); + /// Bulk-send a vector of packets using the most efficient available path + /// (direct UDP via device_session in STANDALONE — coalesces with built-in + /// pacing — and per-packet shmem enqueue in CLIENT). Used by the bulk + /// by-type setters; useful directly when the caller has already built + /// the packet vector. + /// @param packets Packets to send (may be empty, in which case this is a no-op) + /// @return Result indicating success or error + Result sendBulkPackets(const std::vector& packets); + /// Send a runlevel command packet to the device /// @param runlevel Desired runlevel (cbRUNLEVEL_*) /// @param resetque Channel for reset to queue on (default: 0) diff --git a/src/cbsdk/src/cbsdk.cpp b/src/cbsdk/src/cbsdk.cpp index 96fdc107..f2ead381 100644 --- a/src/cbsdk/src/cbsdk.cpp +++ b/src/cbsdk/src/cbsdk.cpp @@ -909,6 +909,7 @@ cbsdk_result_t cbsdk_session_get_group_list( cbsdk_result_t cbsdk_session_set_sample_group( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, bool disable_others, @@ -919,7 +920,7 @@ cbsdk_result_t cbsdk_session_set_sample_group( try { auto result = session->cpp_session->setSampleGroup( n_chans, to_cpp_channel_type(chan_type), - static_cast(rate), disable_others); + static_cast(rate), disable_others, chans); if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; if (out_n_configured) *out_n_configured = result.value(); return CBSDK_RESULT_SUCCESS; @@ -1549,6 +1550,7 @@ cbsdk_result_t cbsdk_session_close_central_file_dialog(cbsdk_session_t session) cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, bool enabled, uint32_t* out_n_configured) { @@ -1557,7 +1559,7 @@ cbsdk_result_t cbsdk_session_set_ac_input_coupling( } try { auto result = session->cpp_session->setACInputCoupling( - n_chans, to_cpp_channel_type(chan_type), enabled); + n_chans, to_cpp_channel_type(chan_type), enabled, chans); if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; if (out_n_configured) *out_n_configured = result.value(); return CBSDK_RESULT_SUCCESS; @@ -1573,6 +1575,7 @@ cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_result_t cbsdk_session_set_spike_sorting( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, uint32_t sort_options, uint32_t* out_n_configured) { @@ -1581,7 +1584,7 @@ cbsdk_result_t cbsdk_session_set_spike_sorting( } try { auto result = session->cpp_session->setSpikeSorting( - n_chans, to_cpp_channel_type(chan_type), sort_options); + n_chans, to_cpp_channel_type(chan_type), sort_options, chans); if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; if (out_n_configured) *out_n_configured = result.value(); return CBSDK_RESULT_SUCCESS; @@ -1606,6 +1609,7 @@ cbsdk_result_t cbsdk_session_set_channel_spike_sorting( cbsdk_result_t cbsdk_session_set_spike_extraction( cbsdk_session_t session, uint32_t n_chans, + const uint32_t* chans, cbproto_channel_type_t chan_type, bool enabled, uint32_t* out_n_configured) { @@ -1614,7 +1618,7 @@ cbsdk_result_t cbsdk_session_set_spike_extraction( } try { auto result = session->cpp_session->setSpikeExtraction( - n_chans, to_cpp_channel_type(chan_type), enabled); + n_chans, to_cpp_channel_type(chan_type), enabled, chans); if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; if (out_n_configured) *out_n_configured = result.value(); return CBSDK_RESULT_SUCCESS; diff --git a/src/cbsdk/src/sdk_session.cpp b/src/cbsdk/src/sdk_session.cpp index 10366b31..0d10f33d 100644 --- a/src/cbsdk/src/sdk_session.cpp +++ b/src/cbsdk/src/sdk_session.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include "cbdev/clock_sync.h" #ifndef _WIN32 @@ -1705,63 +1706,93 @@ static uint32_t countChannelsMatching(const SdkSession& session, return count; } -Result SdkSession::setSampleGroup(const uint32_t nChans, const ChannelType chanType, - const SampleRate rate, const bool disableOthers) { +// Resolve (nChans, chans) into the explicit set of 1-based channel ids to +// configure. When @p chans is non-null, it's the verbatim list (length +// nChans). When @p chans is null, walk channel ids in ascending order and +// pick the first @p nChans that match @p chanType; nChans == UINT32_MAX +// selects all matching. +static std::vector resolveTargetChans( + const SdkSession& session, uint32_t nChans, ChannelType chanType, + const uint32_t* chans) { + if (chans != nullptr) { + return std::vector(chans, chans + nChans); + } + std::vector result; + if (nChans != UINT32_MAX) { + result.reserve(std::min(nChans, cbMAXCHANS)); + } + for (uint32_t chan = 1; chan <= cbMAXCHANS && result.size() < nChans; ++chan) { + const cbPKT_CHANINFO* ci = session.getChanInfo(chan); + if (ci && classifyChannelByCaps(*ci) == chanType) { + result.push_back(chan); + } + } + return result; +} + +Result SdkSession::setSampleGroup( + const uint32_t nChans, const ChannelType chanType, const SampleRate rate, + const bool disableOthers, const uint32_t* chans) { // Pre-sync: ensure local chaninfo cache is up to date before we seed // outgoing CHANSET* packets from it. Stale cache would re-send obsolete // values for fields we don't explicitly modify here. if (auto r = sync(5000); r.isError()) return Result::error(r.error()); const uint32_t group_id = static_cast(rate); - Result send_result = Result::error("No session available"); - if (m_impl->device_session) { - send_result = m_impl->device_session->setChannelsGroupByType( - nChans, toDevChannelType(chanType), - static_cast(group_id), disableOthers); - } else if (m_impl->shmem_session) { - // CLIENT mode: build packets from shmem chaninfo and send through xmt queue. - // Always send for every in-scope channel — don't skip "looks already correct" - // since a dropped CHANREP could leave us cached against a concurrent change - // we'd have no way to escape from except by sending the desired value. - uint32_t sent_count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS; ++chan) { - if (!disableOthers && sent_count >= nChans) - break; - - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) continue; - auto chaninfo = ci_result.value(); - - if (classifyChannelByCaps(chaninfo) != chanType) continue; - - const auto grp = sent_count < nChans ? group_id : 0u; - chaninfo.chan = chan; - - if (grp > 0 && grp < 6) { - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSMP; - chaninfo.smpgroup = grp; - constexpr uint32_t filter_map[] = {0, 5, 6, 7, 10, 0, 0}; - chaninfo.smpfilter = filter_map[grp]; - if (grp == 5) { - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; - chaninfo.ainpopts &= ~cbAINP_RAWSTREAM; - } - } else if (grp == 6) { - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; - chaninfo.ainpopts |= cbAINP_RAWSTREAM; - } else { + const std::vector targets = resolveTargetChans(*this, nChans, chanType, chans); + + // Build a packet for `chan` with `grp` as its target group. + // Always sends the full chaninfo (seeded from local cache), so a stale + // CHANREP can't leave us stuck against a concurrent change. + auto build_packet = [this](uint32_t chan, uint32_t grp) -> std::optional { + const cbPKT_CHANINFO* base = getChanInfo(chan); + if (!base || base->chan == 0) return std::nullopt; + cbPKT_CHANINFO chaninfo = *base; + chaninfo.chan = chan; + if (grp > 0 && grp < 6) { + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSMP; + chaninfo.smpgroup = grp; + constexpr uint32_t filter_map[] = {0, 5, 6, 7, 10, 0, 0}; + chaninfo.smpfilter = filter_map[grp]; + if (grp == 5) { chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; - chaninfo.smpgroup = 0; chaninfo.ainpopts &= ~cbAINP_RAWSTREAM; } + } else if (grp == 6) { + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; + chaninfo.ainpopts |= cbAINP_RAWSTREAM; + } else { + chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; + chaninfo.smpgroup = 0; + chaninfo.ainpopts &= ~cbAINP_RAWSTREAM; + } + return chaninfo; + }; - auto sr = sendPacket(reinterpret_cast(chaninfo)); - if (sr.isError()) return Result::error(sr.error()); - sent_count++; + // Build the packet vector. Targets first (configured to `rate`), then + // — if disableOthers — every other matching channel set to disabled. + std::vector packets; + packets.reserve(targets.size() + (disableOthers ? cbMAXCHANS : 0)); + for (uint32_t chan : targets) { + if (auto pkt = build_packet(chan, group_id); pkt) { + packets.push_back(reinterpret_cast(*pkt)); + } + } + if (disableOthers) { + std::set target_set(targets.begin(), targets.end()); + for (uint32_t chan = 1; chan <= cbMAXCHANS; ++chan) { + if (target_set.count(chan)) continue; + const cbPKT_CHANINFO* ci = getChanInfo(chan); + if (!ci || classifyChannelByCaps(*ci) != chanType) continue; + if (auto pkt = build_packet(chan, 0u); pkt) { + packets.push_back(reinterpret_cast(*pkt)); + } } - send_result = Result::ok(); } - if (send_result.isError()) return Result::error(send_result.error()); + + if (auto r = sendBulkPackets(packets); r.isError()) { + return Result::error(r.error()); + } // Post-sync: ensure the device has applied our changes before we count. if (auto r = sync(5000); r.isError()) return Result::error(r.error()); @@ -1781,127 +1812,105 @@ Result SdkSession::setSampleGroup(const uint32_t nChans, const Channel })); } -Result SdkSession::setSpikeSorting(const uint32_t nChans, const ChannelType chanType, - const uint32_t sortOptions) { - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - - Result send_result = Result::error("No session available"); +// Bulk-send a vector of packets using the most efficient path: +// - STANDALONE: device_session->sendPackets — direct UDP with built-in +// pacing, returns synchronously after every packet has been sent so a +// subsequent sync() barrier sees them in order. +// - CLIENT: per-packet shmem enqueue. The peer STANDALONE process drains +// the same FIFO xmt buffer the sync() SYSSETRUNLEV is queued into, so +// ordering is preserved regardless. +Result SdkSession::sendBulkPackets(const std::vector& packets) { + if (packets.empty()) return Result::ok(); if (m_impl->device_session) { - send_result = m_impl->device_session->setChannelsSpikeSortingByType( - nChans, toDevChannelType(chanType), sortOptions); - } else if (m_impl->shmem_session) { - uint32_t sent_count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS && sent_count < nChans; ++chan) { - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) continue; - auto chaninfo = ci_result.value(); + return m_impl->device_session->sendPackets(packets); + } + if (!m_impl->shmem_session) { + return Result::error("No session available"); + } + for (const auto& pkt : packets) { + if (auto r = sendPacket(pkt); r.isError()) return r; + } + return Result::ok(); +} - if (classifyChannelByCaps(chaninfo) != chanType) continue; +// Helper for the three "configure-each" bulk setters that don't have +// disable_others semantics: setSpikeSorting, setSpikeExtraction, +// setACInputCoupling. Iterates the resolved target list, builds a packet +// per chan via @p mutate, sends, syncs, and counts post-config matches via +// @p predicate. +template +static Result applyBulkSetter( + SdkSession& session, uint32_t nChans, ChannelType chanType, + const uint32_t* chans, Mutate&& mutate, Predicate&& predicate) { + if (auto r = session.sync(5000); r.isError()) return Result::error(r.error()); - // Use CHANSET so the device applies spkopts changes. CHANSETSPKTHR - // (used by older revisions) only reads spkthrlevel and would - // silently ignore the spkopts modification we want here. - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSET; - chaninfo.chan = chan; - chaninfo.spkopts &= ~cbAINPSPK_ALLSORT; - chaninfo.spkopts |= sortOptions; + const std::vector targets = resolveTargetChans(session, nChans, chanType, chans); + std::vector packets; + packets.reserve(targets.size()); + for (uint32_t chan : targets) { + const cbPKT_CHANINFO* base = session.getChanInfo(chan); + if (!base || base->chan == 0) continue; + cbPKT_CHANINFO chaninfo = *base; + chaninfo.chan = chan; + mutate(chaninfo); + packets.push_back(reinterpret_cast(chaninfo)); + } - auto sr = sendPacket(reinterpret_cast(chaninfo)); - if (sr.isError()) return Result::error(sr.error()); - sent_count++; - } - send_result = Result::ok(); + if (auto r = session.sendBulkPackets(packets); r.isError()) { + return Result::error(r.error()); } - if (send_result.isError()) return Result::error(send_result.error()); - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + if (auto r = session.sync(5000); r.isError()) return Result::error(r.error()); + + return Result::ok(countChannelsMatching(session, chanType, predicate)); +} +Result SdkSession::setSpikeSorting( + const uint32_t nChans, const ChannelType chanType, const uint32_t sortOptions, + const uint32_t* chans) { const uint32_t want_sort = sortOptions & cbAINPSPK_ALLSORT; - return Result::ok(countChannelsMatching(*this, chanType, + return applyBulkSetter(*this, nChans, chanType, chans, + [sortOptions](cbPKT_CHANINFO& ci) { + // Use CHANSET so the firmware applies spkopts. CHANSETSPKTHR + // (used by older revisions) only reads spkthrlevel and would + // silently ignore the spkopts modification. + ci.cbpkt_header.type = cbPKTTYPE_CHANSET; + ci.spkopts &= ~cbAINPSPK_ALLSORT; + ci.spkopts |= sortOptions; + }, [want_sort](const cbPKT_CHANINFO& ci) { return (ci.spkopts & cbAINPSPK_ALLSORT) == want_sort; - })); + }); } -Result SdkSession::setSpikeExtraction(const uint32_t nChans, const ChannelType chanType, - const bool enabled) { - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - - Result send_result = Result::error("No session available"); - if (m_impl->device_session) { - send_result = m_impl->device_session->setSpikeExtraction( - nChans, toDevChannelType(chanType), enabled); - } else if (m_impl->shmem_session) { - uint32_t sent_count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS && sent_count < nChans; ++chan) { - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) continue; - auto chaninfo = ci_result.value(); - - if (classifyChannelByCaps(chaninfo) != chanType) continue; - - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETSPK; - chaninfo.chan = chan; - chaninfo.spkopts &= ~cbAINPSPK_EXTRACT; - if (enabled) - chaninfo.spkopts |= cbAINPSPK_EXTRACT; - - auto sr = sendPacket(reinterpret_cast(chaninfo)); - if (sr.isError()) return Result::error(sr.error()); - sent_count++; - } - send_result = Result::ok(); - } - if (send_result.isError()) return Result::error(send_result.error()); - - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - - return Result::ok(countChannelsMatching(*this, chanType, +Result SdkSession::setSpikeExtraction( + const uint32_t nChans, const ChannelType chanType, const bool enabled, + const uint32_t* chans) { + return applyBulkSetter(*this, nChans, chanType, chans, + [enabled](cbPKT_CHANINFO& ci) { + ci.cbpkt_header.type = cbPKTTYPE_CHANSETSPK; + ci.spkopts &= ~cbAINPSPK_EXTRACT; + if (enabled) ci.spkopts |= cbAINPSPK_EXTRACT; + }, [enabled](const cbPKT_CHANINFO& ci) { const bool is_extracting = (ci.spkopts & cbAINPSPK_EXTRACT) != 0; return is_extracting == enabled; - })); + }); } -Result SdkSession::setACInputCoupling(const uint32_t nChans, const ChannelType chanType, - const bool enabled) { - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - - Result send_result = Result::error("No session available"); - if (m_impl->device_session) { - send_result = m_impl->device_session->setChannelsACInputCouplingByType( - nChans, toDevChannelType(chanType), enabled); - } else if (m_impl->shmem_session) { - uint32_t sent_count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS && sent_count < nChans; ++chan) { - auto ci_result = m_impl->shmem_session->getChanInfo(chan - 1); - if (ci_result.isError()) continue; - auto chaninfo = ci_result.value(); - - if (classifyChannelByCaps(chaninfo) != chanType) continue; - - chaninfo.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; - chaninfo.chan = chan; - if (enabled) - chaninfo.ainpopts |= cbAINP_OFFSET_CORRECT; - else - chaninfo.ainpopts &= ~cbAINP_OFFSET_CORRECT; - - auto sr = sendPacket(reinterpret_cast(chaninfo)); - if (sr.isError()) return Result::error(sr.error()); - sent_count++; - } - send_result = Result::ok(); - } - if (send_result.isError()) return Result::error(send_result.error()); - - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - - return Result::ok(countChannelsMatching(*this, chanType, +Result SdkSession::setACInputCoupling( + const uint32_t nChans, const ChannelType chanType, const bool enabled, + const uint32_t* chans) { + return applyBulkSetter(*this, nChans, chanType, chans, + [enabled](cbPKT_CHANINFO& ci) { + ci.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; + if (enabled) ci.ainpopts |= cbAINP_OFFSET_CORRECT; + else ci.ainpopts &= ~cbAINP_OFFSET_CORRECT; + }, [enabled](const cbPKT_CHANINFO& ci) { const bool is_offset_correct = (ci.ainpopts & cbAINP_OFFSET_CORRECT) != 0; return is_offset_correct == enabled; - })); + }); } Result SdkSession::setChannelConfig(const cbPKT_CHANINFO& chaninfo) { diff --git a/tests/integration/test_capi_configuration.cpp b/tests/integration/test_capi_configuration.cpp index 2f4779ca..c59a0d0b 100644 --- a/tests/integration/test_capi_configuration.cpp +++ b/tests/integration/test_capi_configuration.cpp @@ -133,7 +133,7 @@ TEST_F(CApiSampleGroupTest, SetFrontend30kHz) { uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_sample_group( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_30000Hz, true, &n_configured), CBSDK_RESULT_SUCCESS); EXPECT_EQ(n_configured, 4u); @@ -151,7 +151,7 @@ TEST_F(CApiSampleGroupTest, SetAndVerifyField) { uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_sample_group( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_10000Hz, true, &n_configured), CBSDK_RESULT_SUCCESS); EXPECT_EQ(n_configured, 4u); @@ -269,7 +269,8 @@ TEST_F(CApiACCouplingTest, SetACCoupling) { uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, true, &n_configured), + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, true, + &n_configured), CBSDK_RESULT_SUCCESS); EXPECT_EQ(n_configured, 4u); } @@ -280,7 +281,8 @@ TEST_F(CApiACCouplingTest, SetDCCoupling) { uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, false, &n_configured), + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, false, + &n_configured), CBSDK_RESULT_SUCCESS); EXPECT_EQ(n_configured, 4u); } @@ -297,7 +299,8 @@ TEST_F(CApiSpikeSortingTest, SetSpikeSorting) { uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_spike_sorting( - sg.session, 4, CBPROTO_CHANNEL_TYPE_FRONTEND, 0, &n_configured), + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, 0, + &n_configured), CBSDK_RESULT_SUCCESS); EXPECT_EQ(n_configured, 4u); } diff --git a/tests/unit/test_cbsdk_c_api.cpp b/tests/unit/test_cbsdk_c_api.cpp index 497298f1..b731e516 100644 --- a/tests/unit/test_cbsdk_c_api.cpp +++ b/tests/unit/test_cbsdk_c_api.cpp @@ -385,7 +385,7 @@ TEST_F(CbsdkCApiTest, ConfigAccess_WithSession) { /////////////////////////////////////////////////////////////////////////////////////////////////// TEST_F(CbsdkCApiTest, SetChannelSampleGroup_NullSession) { - EXPECT_EQ(cbsdk_session_set_sample_group(nullptr, 256u, + EXPECT_EQ(cbsdk_session_set_sample_group(nullptr, 256u, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_30000Hz, false, nullptr), CBSDK_RESULT_INVALID_PARAMETER); } From 5dd4315f35b38c740ccdf43cf13aeb98c52d72db Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Mon, 27 Apr 2026 23:04:49 -0400 Subject: [PATCH 6/8] Fix CApi AC/spike-sort tests: establish known state before counting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tests/integration/test_capi_configuration.cpp | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/integration/test_capi_configuration.cpp b/tests/integration/test_capi_configuration.cpp index c59a0d0b..ecb2c767 100644 --- a/tests/integration/test_capi_configuration.cpp +++ b/tests/integration/test_capi_configuration.cpp @@ -267,6 +267,16 @@ TEST_F(CApiACCouplingTest, SetACCoupling) { SessionGuard sg; ASSERT_TRUE(sg.create()); + // Establish a known state: all FE chans DC-coupled. n_configured is + // "post-config FE chans whose OFFSET_CORRECT matches `enabled`", so we + // can't assert on its value without controlling the starting state. + uint32_t n_dc = 0; + ASSERT_EQ(cbsdk_session_set_ac_input_coupling( + sg.session, UINT32_MAX, /*chans=*/nullptr, + CBPROTO_CHANNEL_TYPE_FRONTEND, false, &n_dc), + CBSDK_RESULT_SUCCESS); + + // Now AC-couple the first 4 FE chans. Returned count should be exactly 4. uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, true, @@ -279,6 +289,14 @@ TEST_F(CApiACCouplingTest, SetDCCoupling) { SessionGuard sg; ASSERT_TRUE(sg.create()); + // Establish a known state: all FE chans AC-coupled, then DC-couple 4 and + // expect the returned post-config count to be 4. + uint32_t n_ac = 0; + ASSERT_EQ(cbsdk_session_set_ac_input_coupling( + sg.session, UINT32_MAX, /*chans=*/nullptr, + CBPROTO_CHANNEL_TYPE_FRONTEND, true, &n_ac), + CBSDK_RESULT_SUCCESS); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, false, @@ -297,6 +315,15 @@ TEST_F(CApiSpikeSortingTest, SetSpikeSorting) { SessionGuard sg; ASSERT_TRUE(sg.create()); + // Establish a known state — all FE chans with HOOPSORT — so that + // setting 4 to NOSORT yields a post-config count of exactly 4 FE + // chans whose spkopts ALLSORT bits == 0. + uint32_t n_init = 0; + ASSERT_EQ(cbsdk_session_set_spike_sorting( + sg.session, UINT32_MAX, /*chans=*/nullptr, + CBPROTO_CHANNEL_TYPE_FRONTEND, cbAINPSPK_HOOPSORT, &n_init), + CBSDK_RESULT_SUCCESS); + uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_spike_sorting( sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, 0, From dde42fc25069700b189f22b4c3c133f89b519e8f Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Tue, 28 Apr 2026 19:00:16 -0400 Subject: [PATCH 7/8] Add repro test for ezmsg-blackrock disable_others=True hang report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- pycbsdk/tests/test_configuration.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index 9a0232d5..034becb7 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -149,6 +149,21 @@ def test_disable_others(self, nplay_session): channels_1k = nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) assert len(channels_1k) == 2 + def test_disable_others_30k_no_preamble(self, nplay_session): + """Repro for ezmsg-blackrock hang: disable_others=True at SR_30kHz + called immediately after session creation, no preamble. + + Must complete within the per-test timeout (60 s). An indefinite + hang here points to a sync-vs-bulk-send race in the bulk setter. + """ + n = nplay_session.set_sample_group( + 2, ChannelType.FRONTEND, SampleRate.SR_30kHz, + disable_others=True, + ) + assert n == 2 + ch_30k = nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) + assert sorted(ch_30k) == [1, 2] + # --- Per-channel setter (set_channel_smpgroup) --- # # These tests use only per-channel setters for setup (no batch clear) From 9d883af8d66f49f5493078523bcc488a826515ef Mon Sep 17 00:00:00 2001 From: Chadwick Boulay Date: Tue, 28 Apr 2026 21:42:20 -0400 Subject: [PATCH 8/8] Revert n_configured return; bulk setters fire-and-forget on response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 instead of Result. 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) --- pycbsdk/examples/repro_disable_others.py | 66 +++++++++++++ pycbsdk/src/pycbsdk/_cdef.py | 11 +-- pycbsdk/src/pycbsdk/session.py | 82 +++++----------- pycbsdk/tests/test_configuration.py | 94 +++---------------- src/cbsdk/include/cbsdk/cbsdk.h | 60 +++++------- src/cbsdk/include/cbsdk/sdk_session.h | 71 +++++++------- src/cbsdk/src/cbsdk.cpp | 28 ++---- src/cbsdk/src/sdk_session.cpp | 90 ++++-------------- tests/integration/test_capi_configuration.cpp | 52 ++-------- tests/unit/test_cbsdk_c_api.cpp | 2 +- 10 files changed, 203 insertions(+), 353 deletions(-) create mode 100644 pycbsdk/examples/repro_disable_others.py diff --git a/pycbsdk/examples/repro_disable_others.py b/pycbsdk/examples/repro_disable_others.py new file mode 100644 index 00000000..a8d5d10b --- /dev/null +++ b/pycbsdk/examples/repro_disable_others.py @@ -0,0 +1,66 @@ +"""Repro for the bulk-setter `disable_others=True` hang report. + +Mirrors the call sequence reported as hanging indefinitely in +ezmsg-blackrock: + + session.set_sample_group( + 2, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True + ) + +Each phase is timed and logged so a hang shows up as the last printed +line. Run with: + + python repro_disable_others.py NPLAY + python repro_disable_others.py HUB2 + python repro_disable_others.py HUB1 +""" + +import asyncio +import sys +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) + +from pycbsdk import ChannelType, DeviceType, SampleRate, Session + + +def _stamp(t0: float, label: str) -> None: + print(f" +{time.monotonic() - t0:6.3f}s {label}", flush=True) + + +async def main(device_type: str) -> None: + print(f"Connecting to {device_type}...", flush=True) + t0 = time.monotonic() + + with Session(device_type=device_type) as session: + _stamp(t0, "session created") + + await session.wait_until_running(timeout=10.0) + _stamp(t0, f"runlevel reached RUNNING ({session.runlevel})") + + n_fe = session.num_fe_chans() + _stamp(t0, f"num_fe_chans={n_fe}") + + # The exact call reported as hanging. + session.set_sample_group( + 2, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True + ) + _stamp(t0, "set_sample_group returned") + + session.sync() + _stamp(t0, "sync returned") + + ch_30k = session.get_group_channels(int(SampleRate.SR_30kHz)) + _stamp(t0, f"30kHz group: {sorted(ch_30k)}") + + # Sanity: only chans 1, 2 should be at 30kHz. + assert sorted(ch_30k) == [1, 2], ( + f"Expected [1, 2] at 30kHz, got {sorted(ch_30k)}" + ) + print("OK", flush=True) + + +if __name__ == "__main__": + device_type = sys.argv[1] if len(sys.argv) > 1 else "NPLAY" + asyncio.run(main(device_type)) diff --git a/pycbsdk/src/pycbsdk/_cdef.py b/pycbsdk/src/pycbsdk/_cdef.py index 86c9a422..ae7be8c1 100644 --- a/pycbsdk/src/pycbsdk/_cdef.py +++ b/pycbsdk/src/pycbsdk/_cdef.py @@ -214,11 +214,10 @@ cbsdk_result_t cbsdk_session_set_sample_group( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, - _Bool disable_others, uint32_t* out_n_configured); + _Bool disable_others); cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, - cbproto_channel_type_t chan_type, _Bool enabled, - uint32_t* out_n_configured); + cbproto_channel_type_t chan_type, _Bool enabled); // Per-channel getters cbproto_channel_type_t cbsdk_session_get_channel_type(cbsdk_session_t session, uint32_t chan_id); @@ -345,16 +344,14 @@ // Spike sorting cbsdk_result_t cbsdk_session_set_spike_sorting( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, - cbproto_channel_type_t chan_type, uint32_t sort_options, - uint32_t* out_n_configured); + cbproto_channel_type_t chan_type, uint32_t sort_options); cbsdk_result_t cbsdk_session_set_channel_spike_sorting( cbsdk_session_t session, uint32_t chan_id, uint32_t sort_options, int auto_sync); // Spike extraction (enable/disable cbAINPSPK_EXTRACT via CHANSETSPK) cbsdk_result_t cbsdk_session_set_spike_extraction( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, - cbproto_channel_type_t chan_type, bool enabled, - uint32_t* out_n_configured); + cbproto_channel_type_t chan_type, bool enabled); // Clock synchronization cbsdk_result_t cbsdk_session_get_clock_offset(cbsdk_session_t session, int64_t* offset_ns); diff --git a/pycbsdk/src/pycbsdk/session.py b/pycbsdk/src/pycbsdk/session.py index ca603f0e..2c2c39a0 100644 --- a/pycbsdk/src/pycbsdk/session.py +++ b/pycbsdk/src/pycbsdk/session.py @@ -1010,8 +1010,8 @@ def set_sample_group( channel_type: ChannelType, rate: SampleRate, disable_others: bool = False, - ) -> int: - """Set sampling rate for channels of a specific type. + ) -> None: + """Set sampling rate for channels of a specific type (fire-and-forget). ``chans`` selects the channels to configure: @@ -1021,22 +1021,15 @@ def set_sample_group( - ``list[int]`` (or any iterable of ints): the explicit list of 1-based channel IDs. Caller is trusted; no type filter is applied to listed channels, but *channel_type* still defines - the "others" set for *disable_others* and the post-config - count. + the "others" set for *disable_others*. - Self-synchronizing: runs :meth:`sync` before reading the local cache - (so prior in-flight config has landed) and again after sending, so - the returned count reflects the post-config state. Always sends a - CHANSET* packet for every in-scope channel — never skips channels - that look already configured (the local cache may be stale due to - a dropped CHANREP). - - .. note:: - - The device does not update the ``smpgroup`` field for raw - channels. When *rate* is ``SampleRate.SR_RAW`` the returned - count and :meth:`get_group_channels` use the - ``cbAINP_RAWSTREAM`` bit instead. + Runs :meth:`sync` before reading the local cache so any prior + in-flight config from this process has landed before we seed + CHANSET* packets from it. Does NOT sync after sending — call + :meth:`sync` before reading back state that depends on the new + configuration. Always sends a CHANSET* packet for every in-scope + channel — never skips channels that look already configured + (the local cache may be stale due to a dropped CHANREP). Args: chans: Channel selection (see above). @@ -1045,14 +1038,9 @@ def set_sample_group( to disable). disable_others: Disable sampling on channels of *channel_type* that are not in the configured set. - - Returns: - Count of *channel_type* channels whose post-config state matches - *rate*. """ _lib = _get_lib() n_chans, c_arr = self._normalize_chans(chans) - out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_sample_group( self._session, @@ -1061,36 +1049,30 @@ def set_sample_group( int(_coerce_enum(ChannelType, channel_type)), int(_coerce_enum(SampleRate, rate, _RATE_ALIASES)), disable_others, - out_n, ), "Failed to set channel sample group", ) - return int(out_n[0]) def set_ac_input_coupling( self, chans: "int | list[int] | None", channel_type: ChannelType, enabled: bool, - ) -> int: - """Set AC/DC input coupling for channels of a specific type. + ) -> None: + """Set AC/DC input coupling for channels of a specific type + (fire-and-forget). - Self-synchronizing — see :meth:`set_sample_group` for the channel - selection and contract. + See :meth:`set_sample_group` for the channel selection and + pre-sync contract. Args: chans: Channel selection (see :meth:`set_sample_group`). channel_type: Channel type filter. enabled: ``True`` for AC coupling (offset correction on), ``False`` for DC coupling. - - Returns: - Count of *channel_type* channels whose post-config - ``cbAINP_OFFSET_CORRECT`` bit matches *enabled*. """ _lib = _get_lib() n_chans, c_arr = self._normalize_chans(chans) - out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_ac_input_coupling( self._session, @@ -1098,11 +1080,9 @@ def set_ac_input_coupling( c_arr, int(_coerce_enum(ChannelType, channel_type)), enabled, - out_n, ), "Failed to set AC input coupling", ) - return int(out_n[0]) def set_channel_label(self, chan_id: int, label: str, auto_sync: bool = False): """Set a channel's label. @@ -1549,24 +1529,20 @@ def set_spike_sorting( chans: "int | list[int] | None", channel_type: ChannelType, sort_options: int, - ) -> int: - """Set spike sorting options for channels of a specific type. + ) -> None: + """Set spike sorting options for channels of a specific type + (fire-and-forget). - Self-synchronizing — see :meth:`set_sample_group` for the channel - selection and contract. + See :meth:`set_sample_group` for the channel selection and + pre-sync contract. Args: chans: Channel selection (see :meth:`set_sample_group`). channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). sort_options: Spike sorting option flags (cbAINPSPK_*). - - Returns: - Count of *channel_type* channels whose post-config - ``spkopts & cbAINPSPK_ALLSORT`` matches *sort_options*. """ _lib = _get_lib() n_chans, c_arr = self._normalize_chans(chans) - out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_spike_sorting( self._session, @@ -1574,11 +1550,9 @@ def set_spike_sorting( c_arr, int(_coerce_enum(ChannelType, channel_type)), sort_options, - out_n, ), "Failed to set spike sorting", ) - return int(out_n[0]) def set_channel_spike_sorting( self, chan_id: int, sort_options: int, auto_sync: bool = False @@ -1608,27 +1582,23 @@ def set_spike_extraction( chans: "int | list[int] | None", channel_type: ChannelType, enabled: bool, - ) -> int: - """Enable or disable spike extraction for channels of a specific type. + ) -> None: + """Enable or disable spike extraction for channels of a specific type + (fire-and-forget). Controls the ``cbAINPSPK_EXTRACT`` bit which determines whether the device emits spike event packets. Uses ``cbPKTTYPE_CHANSETSPK``. - Self-synchronizing — see :meth:`set_sample_group` for the channel - selection and contract. + See :meth:`set_sample_group` for the channel selection and + pre-sync contract. Args: chans: Channel selection (see :meth:`set_sample_group`). channel_type: Channel type filter (e.g., ``ChannelType.FRONTEND``). enabled: ``True`` to enable spike extraction, ``False`` to disable. - - Returns: - Count of *channel_type* channels whose post-config - ``cbAINPSPK_EXTRACT`` bit matches *enabled*. """ _lib = _get_lib() n_chans, c_arr = self._normalize_chans(chans) - out_n = ffi.new("uint32_t *") _check( _lib.cbsdk_session_set_spike_extraction( self._session, @@ -1636,11 +1606,9 @@ def set_spike_extraction( c_arr, int(_coerce_enum(ChannelType, channel_type)), enabled, - out_n, ), "Failed to set spike extraction", ) - return int(out_n[0]) # --- Clock Synchronization --- diff --git a/pycbsdk/tests/test_configuration.py b/pycbsdk/tests/test_configuration.py index 034becb7..1edbf37c 100644 --- a/pycbsdk/tests/test_configuration.py +++ b/pycbsdk/tests/test_configuration.py @@ -156,11 +156,11 @@ def test_disable_others_30k_no_preamble(self, nplay_session): Must complete within the per-test timeout (60 s). An indefinite hang here points to a sync-vs-bulk-send race in the bulk setter. """ - n = nplay_session.set_sample_group( + nplay_session.set_sample_group( 2, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, ) - assert n == 2 + nplay_session.sync() ch_30k = nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) assert sorted(ch_30k) == [1, 2] @@ -432,61 +432,6 @@ def test_set_spike_sorting(self, nplay_session): nplay_session.sync() -# --------------------------------------------------------------------------- -# n_configured return + chans=None semantics -# --------------------------------------------------------------------------- - - -class TestNConfigured: - """Bulk setters return the count of channels in the post-config state.""" - - def test_set_sample_group_returns_count(self, nplay_session): - n_fe = nplay_session.num_fe_chans() - n = nplay_session.set_sample_group( - n_fe, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, - ) - assert n == n_fe - # And matches a fresh re-scan via get_group_channels. - assert n == len(nplay_session.get_group_channels(int(SampleRate.SR_30kHz))) - - def test_set_sample_group_chans_none_is_all(self, nplay_session): - n_fe = nplay_session.num_fe_chans() - n = nplay_session.set_sample_group( - None, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, - ) - assert n == n_fe - - def test_set_sample_group_partial(self, nplay_session): - # Configure only the first 2 channels at 1kHz, leave the rest untouched. - n = nplay_session.set_sample_group( - 2, ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=False, - ) - # Returned count is "channels at SR_1kHz post-config", i.e. exactly 2. - assert n == 2 - - def test_set_ac_input_coupling_returns_count(self, nplay_session): - n_fe = nplay_session.num_fe_chans() - n_on = nplay_session.set_ac_input_coupling( - None, ChannelType.FRONTEND, True, - ) - assert n_on == n_fe - n_off = nplay_session.set_ac_input_coupling( - None, ChannelType.FRONTEND, False, - ) - assert n_off == n_fe - - def test_set_spike_extraction_returns_count(self, nplay_session): - n_fe = nplay_session.num_fe_chans() - n_on = nplay_session.set_spike_extraction( - None, ChannelType.FRONTEND, True, - ) - assert n_on == n_fe - n_off = nplay_session.set_spike_extraction( - None, ChannelType.FRONTEND, False, - ) - assert n_off == n_fe - - # --------------------------------------------------------------------------- # Explicit channel-list mode for bulk setters # --------------------------------------------------------------------------- @@ -497,19 +442,15 @@ class TestBulkSettersChanList: def test_disjoint_ranges_via_two_calls(self, nplay_session): """Configure chans 1-2 to one rate, chans 3-4 to another.""" - n_lo = nplay_session.set_sample_group( + nplay_session.set_sample_group( [1, 2], ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=False, ) - n_hi = nplay_session.set_sample_group( + nplay_session.set_sample_group( [3, 4], ChannelType.FRONTEND, SampleRate.SR_2kHz, disable_others=False, ) - # Returned counts cover all channels of the type matching each rate. - # With only 4 chans in the nplay fixture, the second call's count - # is the count of FE chans at SR_2kHz post-config = 2. - assert n_lo >= 2 - assert n_hi >= 2 + nplay_session.sync() assert 1 in nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) assert 2 in nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) assert 3 in nplay_session.get_group_channels(int(SampleRate.SR_2kHz)) @@ -525,6 +466,7 @@ def test_disjoint_set(self, nplay_session): [1, 3], ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=False, ) + nplay_session.sync() ch_30k = nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) assert 1 in ch_30k assert 3 in ch_30k @@ -539,40 +481,32 @@ def test_disable_others_with_list(self, nplay_session): None, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, ) # Now keep only chans 1, 2 at 1kHz; disable everything else. - n = nplay_session.set_sample_group( + nplay_session.set_sample_group( [1, 2], ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=True, ) - assert n == 2 + nplay_session.sync() ch_1k = nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) ch_30k = nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) assert sorted(ch_1k) == [1, 2] - assert ch_30k == [] or set(ch_30k).isdisjoint({1, 2}) and not ch_30k if n_fe > 2: # The not-listed FE chans should be disabled (smpgroup=0). for chan_id in range(3, n_fe + 1): assert chan_id not in ch_30k def test_list_with_set_ac_input_coupling(self, nplay_session): - """Explicit list works for set_ac_input_coupling.""" - n_on = nplay_session.set_ac_input_coupling( + """Explicit list works for set_ac_input_coupling — smoke test.""" + nplay_session.set_ac_input_coupling( [1, 3], ChannelType.FRONTEND, True, ) - # post-config count = at least 2 chans with offset_correct on - assert n_on >= 2 def test_list_with_set_spike_extraction(self, nplay_session): - """Explicit list works for set_spike_extraction.""" - # First enable extraction everywhere + """Explicit list works for set_spike_extraction — smoke test.""" nplay_session.set_spike_extraction( None, ChannelType.FRONTEND, True, ) - # Then disable just chans 1, 3 - n_on = nplay_session.set_spike_extraction( + nplay_session.set_spike_extraction( [1, 3], ChannelType.FRONTEND, False, ) - # After: chans 1, 3 disabled; rest enabled. Returned count is the - # number of FE chans with EXTRACT == False (i.e., 2). - assert n_on == 2 def test_empty_list_with_disable_others(self, nplay_session): """Empty list + disable_others disables all FE chans.""" @@ -581,10 +515,10 @@ def test_empty_list_with_disable_others(self, nplay_session): None, ChannelType.FRONTEND, SampleRate.SR_30kHz, disable_others=True, ) # Empty list with disable_others: nothing configured, all disabled. - n = nplay_session.set_sample_group( + nplay_session.set_sample_group( [], ChannelType.FRONTEND, SampleRate.SR_1kHz, disable_others=True, ) - assert n == 0 + nplay_session.sync() assert nplay_session.get_group_channels(int(SampleRate.SR_30kHz)) == [] assert nplay_session.get_group_channels(int(SampleRate.SR_1kHz)) == [] diff --git a/src/cbsdk/include/cbsdk/cbsdk.h b/src/cbsdk/include/cbsdk/cbsdk.h index ae1dc8bd..0ca0f21a 100644 --- a/src/cbsdk/include/cbsdk/cbsdk.h +++ b/src/cbsdk/include/cbsdk/cbsdk.h @@ -561,13 +561,15 @@ CBSDK_API cbsdk_result_t cbsdk_session_get_group_list( // Channel Configuration /////////////////////////////////////////////////////////////////////////////////////////////////// -/// Set sampling rate for channels of a specific type. +/// Set sampling rate for channels of a specific type (fire-and-forget). /// -/// Internally syncs before reading the local cache and again after sending, -/// so @p out_n_configured reflects the channels' post-config state rather -/// than the count just changed by this call. Always sends a CHANSET* packet -/// for every in-scope channel — no skip-if-already-correct optimization, -/// since the local cache may be stale due to a dropped CHANREP. +/// Runs sync() before reading the local cache so any prior in-flight config +/// from this process has landed before we seed CHANSET* packets from it. +/// Does NOT sync after sending — the caller should call cbsdk_session_sync() +/// before reading back state that depends on the new configuration. Always +/// sends a CHANSET* packet for every in-scope channel — no +/// skip-if-already-correct optimization, since the local cache may be stale +/// due to a dropped CHANREP. /// /// Channel selection has two modes: /// - @p chans is NULL: configure the first @p n_chans channels of @@ -575,7 +577,7 @@ CBSDK_API cbsdk_result_t cbsdk_session_get_group_list( /// - @p chans is non-NULL: configure the explicit list of @p n_chans /// 1-based channel ids. No type filtering on listed channels (caller /// is trusted), but @p chan_type still selects the "others" set for -/// @p disable_others and the post-config count. +/// @p disable_others. /// /// @param session Session handle (must not be NULL) /// @param n_chans When @p chans is NULL: count to configure (UINT32_MAX @@ -583,11 +585,9 @@ CBSDK_API cbsdk_result_t cbsdk_session_get_group_list( /// @param chans Optional explicit list of 1-based channel ids; pass NULL /// for the count/UINT32_MAX-based mode. /// @param chan_type Channel type filter (matching when @p chans is NULL, -/// and "others" set + count regardless). +/// "others" set when @p disable_others). /// @param rate Sample rate (CBPROTO_GROUP_RATE_NONE to disable, _500Hz through _RAW) /// @param disable_others If true, disable sampling on unselected channels of @p chan_type. -/// @param[out] out_n_configured Receives the count of @p chan_type channels -/// whose post-config state matches @p rate. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_sample_group( cbsdk_session_t session, @@ -595,27 +595,23 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_sample_group( const uint32_t* chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, - bool disable_others, - uint32_t* out_n_configured); + bool disable_others); -/// Set AC input coupling (offset correction) for channels of a specific type. -/// See cbsdk_session_set_sample_group() for the channel-selection and -/// auto-sync contract. +/// Set AC input coupling (offset correction) for channels of a specific type +/// (fire-and-forget). See cbsdk_session_set_sample_group() for the +/// channel-selection and pre-sync contract. /// @param session Session handle (must not be NULL) /// @param n_chans Count or list length (see cbsdk_session_set_sample_group) /// @param chans Optional explicit list of 1-based channel ids /// @param chan_type Channel type filter /// @param enabled true = AC coupling, false = DC coupling -/// @param[out] out_n_configured Count of @p chan_type channels whose -/// post-config OFFSET_CORRECT bit matches @p enabled. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_ac_input_coupling( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, - bool enabled, - uint32_t* out_n_configured); + bool enabled); /// Set full channel configuration by sending a CHANINFO packet /// @param session Session handle (must not be NULL) @@ -1054,24 +1050,21 @@ CBSDK_API cbsdk_result_t cbsdk_session_close_central_file_dialog(cbsdk_session_t // Spike Sorting /////////////////////////////////////////////////////////////////////////////////////////////////// -/// Set spike sorting options for channels of a specific type. -/// See cbsdk_session_set_sample_group() for the channel-selection and -/// auto-sync contract. +/// Set spike sorting options for channels of a specific type +/// (fire-and-forget). See cbsdk_session_set_sample_group() for the +/// channel-selection and pre-sync contract. /// @param session Session handle (must not be NULL) /// @param n_chans Count or list length (see cbsdk_session_set_sample_group) /// @param chans Optional explicit list of 1-based channel ids /// @param chan_type Channel type filter /// @param sort_options Spike sorting option flags (cbAINPSPK_*) -/// @param[out] out_n_configured Count of @p chan_type channels whose -/// post-config spkopts ALLSORT bits match @p sort_options. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_spike_sorting( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, - uint32_t sort_options, - uint32_t* out_n_configured); + uint32_t sort_options); /// Set spike sorting options for a single channel (fire-and-forget). /// Clears cbAINPSPK_ALLSORT bits then sets sort_options. @@ -1086,26 +1079,23 @@ CBSDK_API cbsdk_result_t cbsdk_session_set_channel_spike_sorting( uint32_t sort_options, int auto_sync); -/// Enable or disable spike extraction for channels of a specific type. -/// Controls the cbAINPSPK_EXTRACT bit via cbPKTTYPE_CHANSETSPK. -/// When enabled, the device emits spike event packets for matching channels. -/// See cbsdk_session_set_sample_group() for the channel-selection and -/// auto-sync contract. +/// Enable or disable spike extraction for channels of a specific type +/// (fire-and-forget). Controls the cbAINPSPK_EXTRACT bit via +/// cbPKTTYPE_CHANSETSPK. When enabled, the device emits spike event +/// packets for matching channels. See cbsdk_session_set_sample_group() +/// for the channel-selection and pre-sync contract. /// @param session Session handle (must not be NULL) /// @param n_chans Count or list length (see cbsdk_session_set_sample_group) /// @param chans Optional explicit list of 1-based channel ids /// @param chan_type Channel type filter /// @param enabled true = enable spike extraction, false = disable -/// @param[out] out_n_configured Count of @p chan_type channels whose -/// post-config EXTRACT bit matches @p enabled. May be NULL. /// @return CBSDK_RESULT_SUCCESS on success, error code on failure CBSDK_API cbsdk_result_t cbsdk_session_set_spike_extraction( cbsdk_session_t session, uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, - bool enabled, - uint32_t* out_n_configured); + bool enabled); /////////////////////////////////////////////////////////////////////////////////////////////////// // Clock Synchronization diff --git a/src/cbsdk/include/cbsdk/sdk_session.h b/src/cbsdk/include/cbsdk/sdk_session.h index bde93a3a..3f518a00 100644 --- a/src/cbsdk/include/cbsdk/sdk_session.h +++ b/src/cbsdk/include/cbsdk/sdk_session.h @@ -524,14 +524,16 @@ class SdkSession { /// Channel Configuration ///-------------------------------------------------------------------------------------------- - /// Set sampling rate for channels of a specific type. + /// Set sampling rate for channels of a specific type (fire-and-forget). /// - /// Internally runs sync() before reading the local cache (so any prior - /// in-flight config from this process has landed) and again after sending - /// (so the returned count reflects post-config state). Always sends a - /// CHANSET* packet for every in-scope channel — never skips channels that - /// look already configured, since the local cache may be stale due to - /// dropped CHANREP packets from a concurrent client. + /// Runs sync() before reading the local cache so any prior in-flight + /// config from this process has landed before we seed CHANSET* packets + /// from it (the #177 contract). Does NOT sync after sending — the + /// caller should call sync() before reading back state that depends on + /// the new configuration. Always sends a CHANSET* packet for every + /// in-scope channel — never skips channels that look already configured, + /// since the local cache may be stale due to dropped CHANREP packets + /// from a concurrent client. /// /// Channel selection has two modes: /// - @p chans is nullptr: configure the first @p nChans channels of @@ -539,60 +541,57 @@ class SdkSession { /// - @p chans is non-null: configure the explicit list of @p nChans /// 1-based channel ids. No type filtering is performed on the listed /// channels (caller is trusted), but @p chanType still selects the - /// "others" set for @p disableOthers and the post-config count. + /// "others" set for @p disableOthers. /// /// @param nChans When @p chans is nullptr: count to configure (UINT32_MAX /// for all matching). When @p chans is non-null: list length. /// @param chanType Channel type filter (used for matching when @p chans - /// is nullptr, and for the "others" set + count regardless). + /// is nullptr, and for the "others" set when @p disableOthers). /// @param rate Desired sample rate (NONE to disable, SR_500 through SR_RAW) /// @param disableOthers Disable sampling on channels of @p chanType not /// in the configured set. /// @param chans Optional explicit list of 1-based channel ids; must be /// non-null only when caller wants the explicit-list mode. - /// @return Number of channels of @p chanType whose post-config state - /// matches @p rate, or error. - Result setSampleGroup(uint32_t nChans, ChannelType chanType, - SampleRate rate, bool disableOthers = false, - const uint32_t* chans = nullptr); - - /// Set spike sorting options for channels of a specific type. - /// See setSampleGroup() for the channel-selection and auto-sync contract. + /// @return Result indicating success or error. + Result setSampleGroup(uint32_t nChans, ChannelType chanType, + SampleRate rate, bool disableOthers = false, + const uint32_t* chans = nullptr); + + /// Set spike sorting options for channels of a specific type + /// (fire-and-forget). See setSampleGroup() for the channel-selection + /// and pre-sync contract. /// @param nChans Count or list length (see setSampleGroup) /// @param chanType Channel type filter /// @param sortOptions Spike sorting option flags (cbAINPSPK_*) /// @param chans Optional explicit list of 1-based channel ids - /// @return Count of @p chanType channels whose spkopts ALLSORT bits - /// match @p sortOptions post-sync, or error. - Result setSpikeSorting(uint32_t nChans, ChannelType chanType, - uint32_t sortOptions, - const uint32_t* chans = nullptr); + /// @return Result indicating success or error. + Result setSpikeSorting(uint32_t nChans, ChannelType chanType, + uint32_t sortOptions, + const uint32_t* chans = nullptr); /// Enable or disable spike extraction (cbAINPSPK_EXTRACT) for channels - /// of a type. See setSampleGroup() for the channel-selection and - /// auto-sync contract. + /// of a type (fire-and-forget). See setSampleGroup() for the + /// channel-selection and pre-sync contract. /// @param nChans Count or list length (see setSampleGroup) /// @param chanType Channel type filter /// @param enabled true = enable spike extraction, false = disable /// @param chans Optional explicit list of 1-based channel ids - /// @return Count of @p chanType channels whose EXTRACT bit matches - /// @p enabled post-sync, or error. - Result setSpikeExtraction(uint32_t nChans, ChannelType chanType, - bool enabled, - const uint32_t* chans = nullptr); + /// @return Result indicating success or error. + Result setSpikeExtraction(uint32_t nChans, ChannelType chanType, + bool enabled, + const uint32_t* chans = nullptr); /// Set AC input coupling (offset correction) for channels of a specific - /// type. See setSampleGroup() for the channel-selection and auto-sync - /// contract. + /// type (fire-and-forget). See setSampleGroup() for the + /// channel-selection and pre-sync contract. /// @param nChans Count or list length (see setSampleGroup) /// @param chanType Channel type filter /// @param enabled true = AC coupling (offset correction on), false = DC coupling /// @param chans Optional explicit list of 1-based channel ids - /// @return Count of @p chanType channels whose OFFSET_CORRECT bit - /// matches @p enabled post-sync, or error. - Result setACInputCoupling(uint32_t nChans, ChannelType chanType, - bool enabled, - const uint32_t* chans = nullptr); + /// @return Result indicating success or error. + Result setACInputCoupling(uint32_t nChans, ChannelType chanType, + bool enabled, + const uint32_t* chans = nullptr); /// Set full channel configuration by packet (fire-and-forget). /// Call sync() before reading back state that depends on this configuration. diff --git a/src/cbsdk/src/cbsdk.cpp b/src/cbsdk/src/cbsdk.cpp index f2ead381..99533dd5 100644 --- a/src/cbsdk/src/cbsdk.cpp +++ b/src/cbsdk/src/cbsdk.cpp @@ -912,8 +912,7 @@ cbsdk_result_t cbsdk_session_set_sample_group( const uint32_t* chans, cbproto_channel_type_t chan_type, cbproto_group_rate_t rate, - bool disable_others, - uint32_t* out_n_configured) { + bool disable_others) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } @@ -921,9 +920,7 @@ cbsdk_result_t cbsdk_session_set_sample_group( auto result = session->cpp_session->setSampleGroup( n_chans, to_cpp_channel_type(chan_type), static_cast(rate), disable_others, chans); - if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; - if (out_n_configured) *out_n_configured = result.value(); - return CBSDK_RESULT_SUCCESS; + return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } @@ -1552,17 +1549,14 @@ cbsdk_result_t cbsdk_session_set_ac_input_coupling( uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, - bool enabled, - uint32_t* out_n_configured) { + bool enabled) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setACInputCoupling( n_chans, to_cpp_channel_type(chan_type), enabled, chans); - if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; - if (out_n_configured) *out_n_configured = result.value(); - return CBSDK_RESULT_SUCCESS; + return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } @@ -1577,17 +1571,14 @@ cbsdk_result_t cbsdk_session_set_spike_sorting( uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, - uint32_t sort_options, - uint32_t* out_n_configured) { + uint32_t sort_options) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setSpikeSorting( n_chans, to_cpp_channel_type(chan_type), sort_options, chans); - if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; - if (out_n_configured) *out_n_configured = result.value(); - return CBSDK_RESULT_SUCCESS; + return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } @@ -1611,17 +1602,14 @@ cbsdk_result_t cbsdk_session_set_spike_extraction( uint32_t n_chans, const uint32_t* chans, cbproto_channel_type_t chan_type, - bool enabled, - uint32_t* out_n_configured) { + bool enabled) { if (!session || !session->cpp_session) { return CBSDK_RESULT_INVALID_PARAMETER; } try { auto result = session->cpp_session->setSpikeExtraction( n_chans, to_cpp_channel_type(chan_type), enabled, chans); - if (result.isError()) return CBSDK_RESULT_INTERNAL_ERROR; - if (out_n_configured) *out_n_configured = result.value(); - return CBSDK_RESULT_SUCCESS; + return result.isOk() ? CBSDK_RESULT_SUCCESS : CBSDK_RESULT_INTERNAL_ERROR; } catch (...) { return CBSDK_RESULT_INTERNAL_ERROR; } diff --git a/src/cbsdk/src/sdk_session.cpp b/src/cbsdk/src/sdk_session.cpp index 0d10f33d..2a4d2cf8 100644 --- a/src/cbsdk/src/sdk_session.cpp +++ b/src/cbsdk/src/sdk_session.cpp @@ -1690,22 +1690,6 @@ static cbdev::ChannelType toDevChannelType(const ChannelType chanType) { } -// Count channels of `chanType` for which `predicate(chaninfo)` is true. -// Caller must ensure the local chaninfo cache is fresh (typically by calling -// sync() first) so the count reflects post-config state rather than pre-config. -template -static uint32_t countChannelsMatching(const SdkSession& session, - ChannelType chanType, Predicate&& predicate) { - uint32_t count = 0; - for (uint32_t chan = 1; chan <= cbMAXCHANS; ++chan) { - const cbPKT_CHANINFO* ci = session.getChanInfo(chan); - if (!ci) continue; - if (classifyChannelByCaps(*ci) != chanType) continue; - if (predicate(*ci)) count++; - } - return count; -} - // Resolve (nChans, chans) into the explicit set of 1-based channel ids to // configure. When @p chans is non-null, it's the verbatim list (length // nChans). When @p chans is null, walk channel ids in ascending order and @@ -1730,13 +1714,13 @@ static std::vector resolveTargetChans( return result; } -Result SdkSession::setSampleGroup( +Result SdkSession::setSampleGroup( const uint32_t nChans, const ChannelType chanType, const SampleRate rate, const bool disableOthers, const uint32_t* chans) { // Pre-sync: ensure local chaninfo cache is up to date before we seed // outgoing CHANSET* packets from it. Stale cache would re-send obsolete // values for fields we don't explicitly modify here. - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); + if (auto r = sync(5000); r.isError()) return r; const uint32_t group_id = static_cast(rate); const std::vector targets = resolveTargetChans(*this, nChans, chanType, chans); @@ -1790,35 +1774,14 @@ Result SdkSession::setSampleGroup( } } - if (auto r = sendBulkPackets(packets); r.isError()) { - return Result::error(r.error()); - } - - // Post-sync: ensure the device has applied our changes before we count. - if (auto r = sync(5000); r.isError()) return Result::error(r.error()); - - // Count channels of chanType whose post-config state matches the request. - // SR_RAW is the special case: the device does not update smpgroup for raw - // channels, so check the RAWSTREAM bit instead. - if (rate == SampleRate::SR_RAW) { - return Result::ok(countChannelsMatching(*this, chanType, - [](const cbPKT_CHANINFO& ci) { - return (ci.ainpopts & cbAINP_RAWSTREAM) != 0; - })); - } - return Result::ok(countChannelsMatching(*this, chanType, - [group_id](const cbPKT_CHANINFO& ci) { - return ci.smpgroup == group_id; - })); + return sendBulkPackets(packets); } // Bulk-send a vector of packets using the most efficient path: // - STANDALONE: device_session->sendPackets — direct UDP with built-in -// pacing, returns synchronously after every packet has been sent so a -// subsequent sync() barrier sees them in order. -// - CLIENT: per-packet shmem enqueue. The peer STANDALONE process drains -// the same FIFO xmt buffer the sync() SYSSETRUNLEV is queued into, so -// ordering is preserved regardless. +// pacing. +// - CLIENT: per-packet shmem enqueue, drained in FIFO order by the peer +// STANDALONE process. Result SdkSession::sendBulkPackets(const std::vector& packets) { if (packets.empty()) return Result::ok(); if (m_impl->device_session) { @@ -1835,14 +1798,15 @@ Result SdkSession::sendBulkPackets(const std::vector& packe // Helper for the three "configure-each" bulk setters that don't have // disable_others semantics: setSpikeSorting, setSpikeExtraction, -// setACInputCoupling. Iterates the resolved target list, builds a packet -// per chan via @p mutate, sends, syncs, and counts post-config matches via -// @p predicate. -template -static Result applyBulkSetter( +// setACInputCoupling. Pre-syncs, iterates the resolved target list, +// builds a packet per chan via @p mutate, and sends. Fire-and-forget +// on the response side — caller can call sync() if it needs to read +// back state. +template +static Result applyBulkSetter( SdkSession& session, uint32_t nChans, ChannelType chanType, - const uint32_t* chans, Mutate&& mutate, Predicate&& predicate) { - if (auto r = session.sync(5000); r.isError()) return Result::error(r.error()); + const uint32_t* chans, Mutate&& mutate) { + if (auto r = session.sync(5000); r.isError()) return r; const std::vector targets = resolveTargetChans(session, nChans, chanType, chans); std::vector packets; @@ -1856,19 +1820,12 @@ static Result applyBulkSetter( packets.push_back(reinterpret_cast(chaninfo)); } - if (auto r = session.sendBulkPackets(packets); r.isError()) { - return Result::error(r.error()); - } - - if (auto r = session.sync(5000); r.isError()) return Result::error(r.error()); - - return Result::ok(countChannelsMatching(session, chanType, predicate)); + return session.sendBulkPackets(packets); } -Result SdkSession::setSpikeSorting( +Result SdkSession::setSpikeSorting( const uint32_t nChans, const ChannelType chanType, const uint32_t sortOptions, const uint32_t* chans) { - const uint32_t want_sort = sortOptions & cbAINPSPK_ALLSORT; return applyBulkSetter(*this, nChans, chanType, chans, [sortOptions](cbPKT_CHANINFO& ci) { // Use CHANSET so the firmware applies spkopts. CHANSETSPKTHR @@ -1877,13 +1834,10 @@ Result SdkSession::setSpikeSorting( ci.cbpkt_header.type = cbPKTTYPE_CHANSET; ci.spkopts &= ~cbAINPSPK_ALLSORT; ci.spkopts |= sortOptions; - }, - [want_sort](const cbPKT_CHANINFO& ci) { - return (ci.spkopts & cbAINPSPK_ALLSORT) == want_sort; }); } -Result SdkSession::setSpikeExtraction( +Result SdkSession::setSpikeExtraction( const uint32_t nChans, const ChannelType chanType, const bool enabled, const uint32_t* chans) { return applyBulkSetter(*this, nChans, chanType, chans, @@ -1891,14 +1845,10 @@ Result SdkSession::setSpikeExtraction( ci.cbpkt_header.type = cbPKTTYPE_CHANSETSPK; ci.spkopts &= ~cbAINPSPK_EXTRACT; if (enabled) ci.spkopts |= cbAINPSPK_EXTRACT; - }, - [enabled](const cbPKT_CHANINFO& ci) { - const bool is_extracting = (ci.spkopts & cbAINPSPK_EXTRACT) != 0; - return is_extracting == enabled; }); } -Result SdkSession::setACInputCoupling( +Result SdkSession::setACInputCoupling( const uint32_t nChans, const ChannelType chanType, const bool enabled, const uint32_t* chans) { return applyBulkSetter(*this, nChans, chanType, chans, @@ -1906,10 +1856,6 @@ Result SdkSession::setACInputCoupling( ci.cbpkt_header.type = cbPKTTYPE_CHANSETAINP; if (enabled) ci.ainpopts |= cbAINP_OFFSET_CORRECT; else ci.ainpopts &= ~cbAINP_OFFSET_CORRECT; - }, - [enabled](const cbPKT_CHANINFO& ci) { - const bool is_offset_correct = (ci.ainpopts & cbAINP_OFFSET_CORRECT) != 0; - return is_offset_correct == enabled; }); } diff --git a/tests/integration/test_capi_configuration.cpp b/tests/integration/test_capi_configuration.cpp index ecb2c767..d835ef66 100644 --- a/tests/integration/test_capi_configuration.cpp +++ b/tests/integration/test_capi_configuration.cpp @@ -131,11 +131,10 @@ TEST_F(CApiSampleGroupTest, SetFrontend30kHz) { SessionGuard sg; ASSERT_TRUE(sg.create()); - uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_sample_group( sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, - CBPROTO_GROUP_RATE_30000Hz, true, &n_configured), CBSDK_RESULT_SUCCESS); - EXPECT_EQ(n_configured, 4u); + CBPROTO_GROUP_RATE_30000Hz, true), CBSDK_RESULT_SUCCESS); + EXPECT_EQ(cbsdk_session_sync(sg.session, 5000), CBSDK_RESULT_SUCCESS); // Verify via group list uint16_t list[256]; @@ -149,11 +148,10 @@ TEST_F(CApiSampleGroupTest, SetAndVerifyField) { SessionGuard sg; ASSERT_TRUE(sg.create()); - uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_sample_group( sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, - CBPROTO_GROUP_RATE_10000Hz, true, &n_configured), CBSDK_RESULT_SUCCESS); - EXPECT_EQ(n_configured, 4u); + CBPROTO_GROUP_RATE_10000Hz, true), CBSDK_RESULT_SUCCESS); + EXPECT_EQ(cbsdk_session_sync(sg.session, 5000), CBSDK_RESULT_SUCCESS); // Query via bulk field getter int64_t values[512]; @@ -267,42 +265,18 @@ TEST_F(CApiACCouplingTest, SetACCoupling) { SessionGuard sg; ASSERT_TRUE(sg.create()); - // Establish a known state: all FE chans DC-coupled. n_configured is - // "post-config FE chans whose OFFSET_CORRECT matches `enabled`", so we - // can't assert on its value without controlling the starting state. - uint32_t n_dc = 0; - ASSERT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, UINT32_MAX, /*chans=*/nullptr, - CBPROTO_CHANNEL_TYPE_FRONTEND, false, &n_dc), - CBSDK_RESULT_SUCCESS); - - // Now AC-couple the first 4 FE chans. Returned count should be exactly 4. - uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, true, - &n_configured), + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, true), CBSDK_RESULT_SUCCESS); - EXPECT_EQ(n_configured, 4u); } TEST_F(CApiACCouplingTest, SetDCCoupling) { SessionGuard sg; ASSERT_TRUE(sg.create()); - // Establish a known state: all FE chans AC-coupled, then DC-couple 4 and - // expect the returned post-config count to be 4. - uint32_t n_ac = 0; - ASSERT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, UINT32_MAX, /*chans=*/nullptr, - CBPROTO_CHANNEL_TYPE_FRONTEND, true, &n_ac), - CBSDK_RESULT_SUCCESS); - - uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_ac_input_coupling( - sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, false, - &n_configured), + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, false), CBSDK_RESULT_SUCCESS); - EXPECT_EQ(n_configured, 4u); } /////////////////////////////////////////////////////////////////////////////////////////////////// @@ -315,21 +289,9 @@ TEST_F(CApiSpikeSortingTest, SetSpikeSorting) { SessionGuard sg; ASSERT_TRUE(sg.create()); - // Establish a known state — all FE chans with HOOPSORT — so that - // setting 4 to NOSORT yields a post-config count of exactly 4 FE - // chans whose spkopts ALLSORT bits == 0. - uint32_t n_init = 0; - ASSERT_EQ(cbsdk_session_set_spike_sorting( - sg.session, UINT32_MAX, /*chans=*/nullptr, - CBPROTO_CHANNEL_TYPE_FRONTEND, cbAINPSPK_HOOPSORT, &n_init), - CBSDK_RESULT_SUCCESS); - - uint32_t n_configured = 0; EXPECT_EQ(cbsdk_session_set_spike_sorting( - sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, 0, - &n_configured), + sg.session, 4, /*chans=*/nullptr, CBPROTO_CHANNEL_TYPE_FRONTEND, 0), CBSDK_RESULT_SUCCESS); - EXPECT_EQ(n_configured, 4u); } /////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/tests/unit/test_cbsdk_c_api.cpp b/tests/unit/test_cbsdk_c_api.cpp index b731e516..729e46a3 100644 --- a/tests/unit/test_cbsdk_c_api.cpp +++ b/tests/unit/test_cbsdk_c_api.cpp @@ -386,7 +386,7 @@ TEST_F(CbsdkCApiTest, ConfigAccess_WithSession) { TEST_F(CbsdkCApiTest, SetChannelSampleGroup_NullSession) { EXPECT_EQ(cbsdk_session_set_sample_group(nullptr, 256u, /*chans=*/nullptr, - CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_30000Hz, false, nullptr), + CBPROTO_CHANNEL_TYPE_FRONTEND, CBPROTO_GROUP_RATE_30000Hz, false), CBSDK_RESULT_INVALID_PARAMETER); }