Address each instrument independently in CENTRAL CLIENT mode - #195
Merged
Conversation
A HUB2 CENTRAL CLIENT session read HUB1's configuration. Both sessions
returned byte-identical config -- same labels, same group membership --
and a HUB2 setter landed on HUB1's channel of the same number. Central's
own Hub{N}- label prefixes proved it: a HUB2 session read 'Hub1-chan1'.
cbsdk_get_max_chans() returns the cbproto wire constant, where
cbMAXPROCS is 1, giving 284. It is a free function with no session, so
it cannot know which device is attached. Central 7.8 lays out 880
channels across four instruments, so the 284 ceiling cut Hub2's block
short: its channels 1-28 aliased onto what CereLink believed were Hub1's
analog/Experiment I/O channels, and its remaining 228 were unreachable.
The mapping cannot be computed from constants. Central packs the
connected instruments densely and renumbers them when that set changes:
with two hubs the NSP's ainp1 starts at 513, with one hub at 257, with
none at 1. An earlier attempt using cbNUM_FE_CHANS as the I/O base was
wrong and made a HUB3 session alias onto the NSP's analog inputs.
Discover the window at runtime from procinfo instead. chancount is
authoritative per instrument, each instrument's channels are contiguous,
and the base is the running sum of preceding chancounts. Sessions are
now addressed 1..getMaxChans() in both CENTRAL and STANDALONE: a Gemini
hub reports 256, the I/O-only Gemini NSP reports 28 with ainp1 at
channel 1, and a disconnected device reports 0 and rejects every id
rather than reading another instrument's data.
procinfo.chanbase is device-local (always 1) and is deliberately not
used as the global base.
Packet chids are translated in dispatchBatch, so callbacks see the same
ids as STANDALONE. An unresolved or identity window skips the loop, so
the hot path is unaffected. Window resolution is lazy because procinfo
lags session setup unpredictably -- a Gemini NSP resolved on three opens
out of four even with a one-second wait.
Verified against two Gemini hubs and a Gemini NSP under Central 7.8 and
in STANDALONE: hubs 256 channels, NSP 28 (ainp1 bank 17, aout1 bank 18,
dout4 bank 22), absent HUB3 0. Soak harness scores 80/80 and 81/81 on
the hubs in STANDALONE. ctest 482/486, the four failures pre-existing.
Known limitation, tracked separately: sync() still times out for any
instrument other than 0 under CENTRAL. HUB1 succeeds in ~20 ms while
HUB2 (instrument 1) and the NSP (instrument 3) both fail 5/5 at the 5 s
timeout, so every auto_sync setter fails for them. That is a distinct
defect in the transmit/response path, unaffected by this change, and
both devices sync normally in STANDALONE.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 3, 2026
NSP clock conversion is wrong by ~170-280 ms under CENTRAL (no fallback, dishonest uncertainty)
#198
Open
sync() timed out for every instrument except 0 under CENTRAL, and since each auto_sync setter runs sync() first and maps its failure to CBSDK_RESULT_INTERNAL_ERROR, no device other than instrument 0 could be configured through Central. HUB1 succeeded in ~20 ms while HUB2 (instrument 1) and the NSP (instrument 3) failed 5/5 at the 5 s timeout. Both worked normally in STANDALONE, so it was neither the hardware nor the device type. Outbound CLIENT-mode packets never set cbpkt_header.instrument, so every one was addressed to instrument 0. A non-zero-index session's reply is then dropped by readReceiveBuffer's instrument filter and the wait times out -- which is why the failures were clean 5.0 s timeouts rather than errors. The clock probe already stamped the field and carried a comment describing exactly this, but the general case was missed. Stamp it in sendPacket() so every CLIENT send is covered at once: runlevel, REQCONFIGALL, channel config, comments, digital output, file control. That fixed sync() but writes still did not land. Only Central's shared-memory chaninfo[] is indexed globally; Central forwards transmit-buffer packets verbatim (InstNetwork drains straight into Instrument::Send, a plain UDP send with no per-instrument routing), so the device applies them in its own numbering -- Hub2's channels are 1..256 to Hub2 whatever Central calls them. Translating the outbound id sent Hub2 a channel 257 it does not have. Validate against the window but send the id unchanged; routing is by instrument. Verified with two Gemini hubs and a Gemini NSP under Central 7.8: sync() 5/5 in ~20 ms on all three, and a distinct label written to each device's channel 1 lands on that device and no other. ctest 482/486, the four failures pre-existing. Fixes #196 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two unrelated defects made every CCF save fail. makeTempCcfPath() in both integration test files built the template as "cerelink_..._XXXXXX.ccf" and passed it to _mktemp_s, which requires the six X's to be the last six characters. It failed with EINVAL and, per its contract, set the first character to '\0' -- so the tests asked the writer to create an unusable path and read the resulting CCFRESULT_ERR_OPENFAILEDWRITE as a CCF writer fault. Substitute into a template that ends in the X's, then append the extension. saveCCF() reached shared memory through getNativeConfigBuffer(), which returns nullptr unless the layout is NATIVE, so a CENTRAL CLIENT had no way to read its own configuration and every save failed. Fall back to getLegacyConfigBuffer(), which translates Central's layout into the native form. That alone produced a file with the wrong contents. Central indexes chaninfo[] globally across all instruments, and extractFromNativeConfig copies the first cbMAXCHANS entries verbatim: with two hubs the result held Hub1's 256 channels plus Hub2's first 28 sitting in the slots reserved for analog/Experiment I/O, silently dropping everything past 284 of Central's 880. Loading such a file back would have written one device's settings onto another's -- worse than the clean failure it replaced. A CCF now describes the device its session speaks for, matching the rest of the API: this instrument's channels, in this instrument's numbering. Everything else in the file (filters, sorting, LNC, waveforms, n-trodes, sysinfo) is system-wide and is kept as extracted. Verified under Central 7.8 with two Gemini hubs and a Gemini NSP: HUB1 saves 256 Hub1- channels, HUB2 saves 256 Hub2- channels, and the NSP saves exactly its 28 I/O channels (16 ainp, 4 aout, 2 audio, 1 digin, 1 serial, 4 dout). ctest 484/486, both remaining failures pre-existing. Fixes #197 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Gemini NSP read through Central converted device timestamps 170-280 ms into the future while reporting ~8 ms uncertainty. Hubs were fine, so it looked device-specific; it is really a gap in where estimates come from. The NSP transmits in ~8192-sample blocks and its probe replies are latched to those boundaries: consecutive probes 600 ms apart returned timestamps differing by exact multiples of 273.067 ms, and two probes inside one block returned the identical timestamp. Every offset it computes inherits that quantisation -- 187 ms of spread on 20 ms round trips -- and no filtering recovers timing information the reply never carried. Central's queue is not the cause; measured RTT is ~19 ms, which bounds any asymmetry bias at ~10 ms. STANDALONE already solves this: a session reads its peers' published offsets from their shared memory and votes, and an NSP explicitly borrows a hub. Under CENTRAL there is no peer CereLink process publishing anything, so the vote list is always empty and the borrow never fires. But Central's ring already carries every instrument's packets -- a CENTRAL CLIENT was simply discarding them. Add a generic pre-filter packet observer to ShmemSession. The instrument filter is unchanged and what readReceiveBuffer returns is byte-identical; the observer only sees packets already being walked to make that decision. The shm layer stays ignorant of packet meaning -- the SDK decides NPLAYREP is interesting -- so the demux path keeps a single rule rather than growing a packet-type exception. A CENTRAL session now probes every present instrument, keeps a per instrument estimate, and applies the same vote/borrow rules STANDALONE uses, so the two modes converge instead of drifting. setExternalOffset still sanity-checks before adopting, so a peer that disagrees is rejected -- no PTP assumption is hard-coded, which matters on legacy analog hardware whose instruments are not commonly disciplined. Also stop reporting rtt/2 as uncertainty when the samples disagree. rtt/2 bounds the error only if they agree; the NSP advertised ~10 ms confidence for a 187 ms-spread estimate, and a caller checking the uncertainty had no way to tell. Use max(rtt/2, spread/2) over the glitch-filtered set. Verified under Central 7.8 with two Gemini hubs and a Gemini NSP: the NSP's conversion moved from -169.8 ms to +8.4 ms, in line with the hubs (+6.6, +10.3). Instrumentation shows the peer hub estimates at 0.29-0.41 ms spread against the NSP's own 187 ms, with the median electing a hub value. ctest 485/486, the one failure pre-existing. Two caveats. The uncertainty change landed after the devices went offline, so its effect is unverified on hardware -- the 485/486 run predates it, and only the 60 hardware-independent cbproto/cbshm tests cover it. And CERELINK_CLOCK_DEBUG instrumentation in clock_sync.cpp is deliberately retained for further testing; it is inert unless the variable is set, and should be removed before merge. Refs #198 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A device whose probe replies are latched to its transmit blocks has no usable probe estimate: a Gemini NSP accumulated 80 probes spanning 269.7 ms, one whole 8192-sample block, and the spread does not shrink with more samples because the error is bounded by the block period rather than random. STANDALONE already handles this. DeviceSession feeds addDataPacketSample, the estimator rejects the probes and uses the data-packet floor, and on the same NSP that floor holds 320 samples agreeing to within 1.4 ms. Under CENTRAL the fallback was never fed, so a client had only the unusable probes. With hubs present the cross-instrument consensus covers it, but an NSP alone under Central has no peer to borrow from and nothing else to fall back to -- an unusual configuration, though not an impossible one. Feed the fallback once per batch from the CLIENT receive loop, mirroring DeviceSession. No tick->ns conversion is needed because readReceiveBuffer already normalises CLIENT timestamps. Selection order makes it self-limiting: reliable probes still win, so a device with good probes is unaffected and only one with unusable probes gains the floor. Also stop reporting a flat 0.7 ms uncertainty for that floor. The floor takes the least-delayed sample seen, so its error is whatever delay that sample still carried; the disagreement among samples is the evidence for it. Report max(ONE_WAY_DELAY_ESTIMATE_NS, spread/2), the same correction already applied to the probe path. On healthy data this changes nothing -- the NSP's 320 samples agree well inside the constant -- and it stops the estimate from claiming sub-millisecond confidence when they scatter. Correct the strategy comment that sent me looking for this in the wrong place: the data branch is the NSP path for a *device session*. Only DeviceSession feeds it, so it is unreachable for the CLIENT instance, where a NATIVE client inherits its owner's committed offset and a CENTRAL client derives one across instruments. Verified in STANDALONE: NSP unc=700,000 ns delta +1.7 ms, HUB1 unc=102,670 ns delta +0.9 ms. 60/60 hardware-independent cbproto/cbshm tests pass; the device-dependent suites were not run because the hubs were offline. Two things still unverified on hardware, both needing Central: the CENTRAL half of this change (NSP alone, hubs disconnected), and the max(rtt/2, spread/2) probe-path uncertainty from the previous commit, which only differs when spread exceeds rtt and so needs the NSP reading through Central. Refs #198 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CERELINK_CLOCK_DEBUG logging added while diagnosing #198 has served its purpose: it showed the NSP's probe spread saturating at one 8192-sample block, the peer hub estimates at sub-millisecond spread beside it, and data=0 proving the CLIENT fallback was never fed. All of that is recorded in #198 and in the commits that fixed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… fix/central-per-instrument-channel-window
… fix/central-per-instrument-channel-window
…trument-channel-window
ClockSyncTest.BestProbeIsMaxOffset caught this: max(rtt/2, spread/2) double-counts. Offsets legitimately spread when round trips differ -- a slower probe yields a lower offset -- and picking the max offset already compensates, so the whole spread is not evidence of error. The test's three probes (RTT 500/100/300, offsets 9750/9950/9850) spread by exactly what their round trips predict, and its expectation of rtt/2 = 50 is right. What rtt/2 cannot bound is spread the round trips do not explain: a Gemini NSP latches its reply timestamps to 8192-sample transmit blocks and produced 187 ms of spread on 20 ms round trips that varied by only ~10 ms. Charge the excess over (rtt_hi - rtt_lo) / 2 instead, which leaves a well-behaved path reporting rtt/2 and still flags an estimate whose samples disagree for reasons transit delay cannot account for. I had run only the cbproto/cbshm subset after the previous version of this change because the devices were offline, so the clock-sync unit tests never ran locally. ClockSync is 53/53 now, and 391/392 across all non-hardware suites (the one failure predates this branch). Refs #198 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes every instrument independently addressable through Central: channel access, packet ids, and configuration writes. Two commits — channel windowing, then packet routing.
1. A HUB2 session read HUB1's configuration
Both sessions returned byte-identical config, and a HUB2 setter landed on HUB1's channel of the same number. Central's own
Hub{N}-label prefixes made it unambiguous — a HUB2 session read'Hub1-chan1'.cbsdk_get_max_chans()returns the cbproto wire constant (cbMAXPROCS 1→ 284). It is a free function with no session parameter, so it cannot know which device is attached. Central 7.8 lays out 880 channels across four instruments, so the 284 ceiling cut Hub2's block short:Hub2's channels 1–28 aliased onto what CereLink believed were Hub1's analog/Experiment I/O channels; its remaining 228 were rejected outright.
Why constants can't fix it
Central packs the connected instruments densely and renumbers when that set changes. With two hubs the NSP's
ainp1starts at 513 (bank 17); with one hub at 257 (bank 9); with none at 1 (bank 1). An earlier attempt usingcbNUM_FE_CHANSas the I/O base was wrong, and made aHUB3session alias onto the NSP's analog inputs — the same bug in a new place.Fix
Discover the window at runtime from
procinfo:chancountis authoritative per instrument, each instrument's channels are contiguous, andbase = 1 + Σ chancount(j) for j < instrument.chanbaseis device-local (always 1) and deliberately unused.Sessions are addressed
1..getMaxChans()in both CENTRAL and STANDALONE. Packet chids are translated indispatchBatch; an unresolved or identity window skips the loop, leaving the hot path unaffected. Resolution is lazy becauseprocinfolags session setup unpredictably — a Gemini NSP resolved on three opens out of four even with a one-second wait.CentralChannelSpaceand its five implementations were removed in favour ofgetProcInfoAt; the net result is smaller than the code it replaces.2. Nothing but instrument 0 could be configured (#196)
sync()timed out for every instrument except 0, and since eachauto_syncsetter runs it first, all config writes failed for those devices.Outbound CLIENT-mode packets never set
cbpkt_header.instrument, so all went out as instrument 0; a non-zero-index session's reply is dropped byreadReceiveBuffer's instrument filter, hence clean 5.0 s timeouts. The clock probe already stamped the field and carried a comment describing this exact mechanism — it was fixed once there and missed generally. Now stamped insendPacket(), covering every CLIENT send.That fixed
sync()but writes still did not land. Only Central'schaninfo[]is indexed globally; Central forwards transmit-buffer packets verbatim (InstNetwork→Instrument::Send, a plain UDP send with no per-instrument routing), so the device applies them in its own numbering. Translating the outbound id sent Hub2 a channel 257 it does not have. Validate against the window, send the id unchanged, route by instrument.Verification
Two Gemini hubs + a Gemini NSP, Central 7.8 and STANDALONE:
max_chanssync()before → afterHub1-chan1Hub2-chan1ainp1NSP bank assignments match the hardware:
ainp1bank 17,aout1bank 18,dout4bank 22.--native --intense: 80/80 (HUB1), 81/81 (HUB2). NSP 75/1/2, both failures being harness assumptions since fixed on the base branch.ctest482/486; the four failures are pre-existing, confirmed by stashing this branch and re-running. Three are saveCCF fails for NPLAY and CENTRAL CLIENT sessions (succeeds in STANDALONE) #197.Base is
test/hardware-soak-harnessbecause this depends on the multi-version Central adapters and was validated with that branch's soak harness.Fixes #196
🤖 Generated with Claude Code