Skip to content

Multi-version Central compatibility#190

Open
cshmookler wants to merge 72 commits into
masterfrom
multi-version-central-compat
Open

Multi-version Central compatibility#190
cshmookler wants to merge 72 commits into
masterfrom
multi-version-central-compat

Conversation

@cshmookler

@cshmookler cshmookler commented Jul 2, 2026

Copy link
Copy Markdown

Allow Cerelink to connect to older versions of Central.

Summary

Previously, CereLink could only interoperate with the single latest version of Central's shared
memory. This branch introduces a versioned compatibility layer that lets CereLink attach to
Central's shared memory across multiple protocol versions (7.0, 7.5, 7.6, 7.7, and 7.8/current),
automatically detecting the running Central version and translating its structs to and from
CereLink's native types.

The change also reworks ShmemSession into a per-instrument, layout-aware session and modernizes its
API across cbsdk and the examples/tests.

Motivation

Central exposes its configuration, status, and spike buffers through shared memory, but the binary
layout of those structs changes between protocol versions. Central's shared memory carries no usable
version field (the version field is a magic constant, and procinfo[].version's byte offset shifts
between versions), so CereLink previously assumed the newest layout and broke silently against any
other version. This work makes cross-version compatibility explicit and maintainable.

Approach

Adapter layer. Each supported protocol version has a BootstrapAdapter and Adapter. The
BootstrapAdapter reads the sizes of Central's shared-memory structs and instantiates an Adapter
holding raw pointers into each buffer. Each Adapter translates version-specific Central structs
to/from the native CereLink equivalents (cbproto + native_types.h), which are the common language
ShmemSession operates on.

  • Types live in src/cbshm/include/cbshm/central_types/{v7_0,v7_5,v7_6,v7_7,v7_8}.h
  • Adapter interfaces live in src/cbshm/include/cbshm/central_adapters/* (kept nearly identical
    across versions so they diff cleanly)
  • Translation logic lives in src/cbshm/src/central_adapters/*.cpp

Splitting the previous monolithic central_types.h/adapters.h into per-version files makes
version-to-version differences reviewable via simple diffs, and adding a version is a documented
copy-diff-rectify procedure (see docs/multi_version_central_compat/README.md).

Version detection. New central_version.{h,cpp} detect Central's version on Windows by inspecting
Central.exe's ProductVersion (via the running-process list) and mapping the application version to a
protocol version. Detection was deliberately moved out of ShmemSession so the version is resolved
once and the correct adapter is selected at open. getCompatProtocolVersion() exposes the resolved
version to callers (returns CBPROTO_PROTOCOL_CURRENT for the NATIVE layout).

Per-instrument, layout-aware ShmemSession. The session now takes an explicit ShmemLayout (CENTRAL vs
NATIVE) and is created per instrument:

ShmemSession::create(Mode mode, ShmemLayout layout,
const std::string& name_qualifier, cbproto::InstrumentId id);

Segment names are synthesized internally from the layout and qualifier (Central's fixed well-known
names + instance suffix for CENTRAL; cbshm__ for NATIVE). Indexing always uses
packet.instrument so standalone-written packets land in the slot a later CLIENT would read.

Scope / Limitations

The compatibility layer covers Central's configuration, status, and spike buffers. The
receive/transmit buffers are still handled by version-specific logic spread across cbdev, cbproto,
and cbshm; folding these into an adapter is noted as future work in the docs.

Key Changes

  • New: central_adapters/{base,v7_0,v7_5,v7_6,v7_7,v7_8} (interfaces + implementations),
    central_types/v7_* headers, central_version.{h,cpp}, central_current.h
  • Reworked: shmem_session.{h,cpp} (per-instrument, layout-aware API; spike-cache access;
    multi-instrument-safe cbPcStatus), large cbproto/types.h cleanup (macros → global constants; type
    names matched to Central)
  • Call sites: cbsdk (sdk_session.cpp, cbsdk.cpp, cmp_parser) migrated to the new ShmemSession API
  • Fixes: 7.7.x/7.8.x Central use both cbproto and cbhwlib constants; Central 7.7.0 → protocol 4.1;
    stack overflow when translating large structs; id/idx discrepancies; MSVC/Windows build fixes for
    the adapters
  • Docs: new docs/multi_version_central_compat/README.md (architecture + "add/remove a version"
    runbook); updated shared-memory architecture doc and cbshm/cbsdk READMEs
  • Tests: all ShmemSession, native-types, cmp-parser, and SDK integration/unit tests migrated to the
    new API and expanded
  • Examples: CentralClient, SimpleDevice, CCFTest, GeminiExample, and pycbsdk examples updated for
    the new API

Compatibility Notes

  • Breaking API change: SdkSession methods that previously returned pointers to configuration structs now return copies.
  • Breaking API change: ShmemSession::create(...) signature changed (now takes Mode, ShmemLayout,
    name_qualifier, and instrument id). All in-repo callers, examples, and tests are updated.
  • Version detection is currently Windows-only (relies on Central.exe product-version inspection).

Testing

  • Functional compatibility verified for v7.8, v7.7, v7.6, v7.5, and v7.0. Versions 7.1-7.4 of Central do not exist and are not accounted for.
  • Existing and new C++ unit/integration tests and pycbsdk tests updated to the new API.

cshmookler and others added 30 commits May 8, 2026 16:33
Implemented both factory functions that construct a CentralAdapter.
Implemented a workaround for fetching Central's application and protocol versions.
The translation units containing the factory functions now link with
the correct Windows libraries.
Remove the example for the CentralAdapter now that it's an implementation
detail of ShmemSession.

Add new compatibility version detection logic to ShmemSession::Impl.

Split detectCompatProtocol into two separate methods: one for inspecting
Central's binary for version information and another for inspecting the shared
memory as the original function did. Compatability version detection for a
STANDALONE + CENTRAL_COMPAT CereLink instance is (theoretically) supported,
but this logic is dead because this configuration is never created by
SdkSession::create.
…+ CENTRAL_COMPAT CereLink instances

This configuration (STANDALONE + CENTRAL_COMPAT) appears to only be
theoretical and is never instantiated by SdkSession.
Add central types for 4.2, 4.1, and 4.0.

Move CentralConfigBuffer into config_buffer.h.

Place version-specific constants and structs in namespaces and remove CENTRAL_ prefixes.
Macros in cbproto collide with names in central_types/ matching those in
Central's source, so switch to variables to respect namespaces.

Add annotations of version differences with 'VER:'
…yCFGBUFF.

These structs ensure that each version of CentralLegacyCFGBUFF maps
exactly to the corresponding version of Central's configuration buffer.

Structure differences between versions are annotated with VER:
Each translator file can be diffed with another to see translation differences between versions.
Translations are two-way between legacy versions and the current protocol version.
Diffable adapter files for quickly seeing differences between versions.
Implement primitive get/set methods for all planned legacy versions (v3.11, v4.0, v4.1, v4.2).
Partially implement integration with ShmemSession.
Functional v4.2 and v4.1 compatiblity.
Theoretically functional v4.0 and v3.11 compatibility, but untested.
Each Central adapter has access to all raw pointer buffers used by ShmemSession.
These pointers are provided to the Adapter constructor.

Buffer size methods are within the BootstrapAdapter so the buffer pointers are
initialized before getting passed to the Adapter constructor.

All translator methods must have the translation context provided so the
signatures remains the same.

The translator for cbCFGBUFF has a fromLegacy translator but not a toLegacy
translator. This is due to cbCFGBUFF containing information for multiple
instruments which is lost upon translation, thus making reversing the
operation impossible without access to the original buffer. The translators
for cbPcStatus also result in loss of multi-instrument information. This issue
would be resolved by combining the translators with the central adapter.

Translators for the spike buffer and cache are necessary for full compatibility.
Removed the CENTRAL_COMPAT layout and replaced it with CENTRAL.

Spike buffer and cache operations with the CENTRAL layout are only performed
for the most recent version of Central.

In the future, most of the ShmemSession implementation should be moved into
the Central adapter.
Each ShmemSession operates on a specific instrument. With the CENTRAL layout,
the instrument filter is always active.

Multi-instrument control is possible by creating multiple SdkSession instances
(one per instrument).
Add translators for cbPKT_SPK, cbSPKBUFF, and cbSPKCACHE.
Implement getSpikeCache and getRecentSpike for ShmemSession with layout == CENTRAL.
Rework how Blackrock .cmp channel-map files are applied to a device's
in-memory channel config.

Matching: rows are now matched to live channels by the device's own
(bank, term) read from chaninfo, instead of by an ordinal channel ID
(the old sort-and-assign start_chan+N scheme). start_chan shifts the
CMP's bank letters by start_chan/32 banks to target a headstage's banks
(129 -> +4 -> bank A maps to device bank E), which makes (bank, term) a
globally-unique join key. This is robust to incomplete CMPs.

position[]: the four cbPKT_CHANINFO.position slots now carry real
geometry {x, y, size, headstage_id} instead of {col, row, bank, elec}.

Columns: the //-comment header line immediately before the data (when
present) is the ground truth for column order, matched by name
(col/c, row/r, bank/b, elec/e, size/s, label/l); no header falls back
to the legacy "col row bank electrode [label]" order, so existing files
parse unchanged.

Units: with no size column and a unit-spaced index grid (every non-zero
col/row delta is exactly 1), values are electrode indices -> size 1 and
x/y/size scaled by the 400 um Utah-array pitch. With a size column, or
non-uniform spacing, values are taken at face value.

Labels: stored verbatim; the hs{N}- prefix is dropped since the stored
headstage id disambiguates reused labels.

pycbsdk/cmp.py is kept in sync with the C++ parser; session.py
docstrings corrected. C++ and Python unit tests updated; the
hardware-gated nplay integration tests join device readback to parser
output via ChanInfoField.BANK/TERM.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng, because some arrays will have gaps in their grid and therefore have >1 spacing in some dimensions.
@cshmookler cshmookler self-assigned this Jul 2, 2026
@cshmookler
cshmookler marked this pull request as ready for review July 2, 2026 13:58
cboulay and others added 2 commits July 2, 2026 17:11
Common methods for all Adapter methods are tested.
All copyArr and copyArr2D methods from base.h are tested.

detectCentralVersion does not have unit tests and must be tested manually.
@cboulay

cboulay commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

pycbsdk-windows-x64.zip
Attaching build artifact so I can test on a different machine...

@cboulay

cboulay commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

I couldn't get this to work on my lab setup running a LEGACY_NSP with firmware 7.6.0 and Central 7.6.1.
That being said, I couldn't get master to work either, so I don't think the fault is with your PR, I think there's just a malfunctioning path for LEGACY_NSP.
If you're able to test with that particular setup then that would be helpful.

cshmookler and others added 12 commits July 6, 2026 11:23
Label strings were previously returned as const char* types.
With multi-version-central-compat, the API for fetching certain configuration
structs was changed to return copies instead of pointers.  This resulted in
the getter methods for labels (const char*) pointing to objects allocated on
the stack instead of shared memory, so the pointers were left dangling after
the getter returned.

This fix requires callers of cbsdk to provide an allocated char buffer in
order to fetch a label string.
Fields m_nNspStatus, m_nNumNTrodesPerInstrument, and m_nGeminiSystem are now calculated according to Central's 7.0.6 source.

Versions 7.0, 7.5, 7.6, 7.7, and 7.8 have been tested with Central+NPLAY.
Versions 7.6, 7.7, and 7.8 have been tested with Central+LEGACY_NSP, Central+NSP or Central+Hub1.
…ng garbage

The rec ring publishes head_index and head_wrap as two separate words, and on
a wrap the producer bumps head_wrap before republishing head_index. No ordering
of two independent stores keeps the combined position (wrap*buflen + index)
monotonic across a wrap, so the CLIENT reader could observe a torn pair (old
index + new wrap). That corrupted the overrun check, dropped tail off a packet
boundary, and the bad-size path then byte-scanned through garbage, delivering
misinterpreted bytes to user callbacks — the intermittent
"malformed packets after a buffer wrap" failure seen on Windows under load.

Make the reader fail safe:
- Read (head_index, head_wrap) as a seqlock-style snapshot to shrink (but not
  fully close) the torn-pair window.
- On overrun, an implausible packet size, or a header that cannot belong to a
  real packet (type high byte set, or chid outside a channel / config range),
  resync tail to the current head and report data loss, instead of advancing by
  a garbage size and scanning more garbage.

Because the two-word update is fundamentally non-atomic (and the Central-
compatible layout forbids merging the fields), validation is the real safety
net: torn snapshots and genuine overruns now drop data cleanly and never reach
user callbacks. Verified against nPlayServer 7.8.0 (CLIENT data-reception
integration tests pass); no change on the normal path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n copyArr2D

CentralBootstrapAdapterBase is held and deleted through a
std::unique_ptr<CentralBootstrapAdapterBase> but had no virtual destructor,
so deleting a derived BootstrapAdapter through the base pointer was undefined
behavior (-Wdelete-abstract-non-virtual-dtor). Give it a virtual default
destructor, matching CentralAdapterBase.

Also drop the meaningless `const` on the `void` return type of the templated
copyArr2D overload (-Wignored-qualifiers).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chittti

chittti commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Following the fix for marker packets being expected in CENTRAL_COMPAT mode, here a full list of findings that could be addressed in this PR. Some of them are minor and certainly optional but a few are bugs that need to be fixed before merge.

1. Struct layout mismatches against Central

Shared-memory structs must match Central's byte layout exactly; an incorrect field size shifts the offset of every subsequent field in the containing struct.

  • v7_5 packet header is 15 bytes; protocol 4.0 uses 16. central_types/v7_5.h declares a single reserved byte; it should be uint8_t reserved[2];. Central 7.5.1's header declares a two-byte reserved field, and this repository's own cbPKT_HEADER_400 agrees at 16 bytes. With packed structs, everything embedding this header in the v7_5 config/status/spike buffers is read at shifted offsets against a real Central 7.5. The fix widens the field to two bytes, updates the two header translation sites, and adds a static_assert on the header size to prevent recurrence during the documented copy-diff-rectify workflow. About 7 lines.
  • v7_5 spike cache line count uses the wrong constant. central_types/v7_5.h uses cbNUM_ANALOG_CHANS; Central 7.5.1 uses cbMAXCHANS (its cbNUM_ANALOG_CHANS variant is commented out, and this line's own trailing comment already states cbMAXCHANS). This affects the spike buffer size and cache indexing. The fix switches the constant to cbMAXCHANS. 1 line.

2. Adapter translation defects

  • AOUT_WAVEFORM trigger mode is mapped from the wrong byte in v7_0.cpp and v7_5.cpp, plus the corresponding toLegacy sites. The legacy field is a plain 16-bit cbWAVEFORM_TRIGGER_* enum with values 0 through 6, so extracting the high byte always yields 0. The current-format layout is trig followed by trigInst, one byte each. The fix maps the mode into trig, zero into trigInst, and writes the plain mode back on the outbound path. About 8 lines across both versions.
  • Filter descriptor labels are never translated. The fromLegacy/toLegacy pair for cbFILTDESC (for example in v7_8.cpp) copies hpfreq through lptype but skips the label field, in all five versions and both directions. Both sides define the label as the first field (verified against Central 7.0 and 7.5), so getChanInfo() returns empty filter labels. Same defect class as 1d2cc65. The fix adds the label copy to both directions in all five versions. 10 lines.
  • The config-buffer translation leaves fields with no legacy counterpart indeterminate. In v7_8.cpp (mirrored in all versions), the clock sync fields and owner_pid are never written and optiontable/colortable are not copied. A caller that does not pre-zero the out parameter can observe a garbage clock_sync_valid. Currently reachable only from tests via getLegacyConfigBuffer(). The fix zeroes the destination at function entry. About 20 lines. The existing TODO in the same function about per-instrument slicing is unaffected.
  • monsource translation is inconsistent and ignores the union's mode-dependent interpretation. The legacy 32-bit monsource overlays a union whose meaning depends on the channel's digital output mode: monitoring (moninst/monchan) versus timed output (lowsamples/highsamples). The repository currently contains three mutually inconsistent mappings for this conversion: v7_5.cpp (and the v7_0 equivalent), packet_translator.cpp, and the byte-preserving mapping implied by the union layout. Each is correct for one union arm only. The fix selects the mapping from the channel's dout/aout option flags and applies the same logic at both translation sites. Roughly 30 to 60 lines, and it needs a design decision on the flag used to pick the arm.

3. Open-time validation

  • The instrument index is not validated against the detected version's instrument count. In shmem_session.cpp the index is passed straight into the adapters, which index procinfo, isSelection, m_nNspStatus and similar arrays without bounds checks. Against Central 7.0 (cbMAXPROCS = 1), any index above 0 reads, and on setter paths writes, past those arrays inside Central's live shared memory. The fix rejects an index at or above the adapter's getMaxProcs() during open(). About 11 lines.
  • Unrecognized major versions attach silently. central_version.cpp returns UNKNOWN for major versions above 7 (an error return at that site is present but commented out). getProtocolVersion(UNKNOWN) yields an unknown protocol value, which selects the 7.8 adapter through the default switch case and, because the value differs from CURRENT, routes reads through the 4.1 translator. The fix makes open() fail on an unknown protocol; the minor-version forward-compatibility policy (7.9+ treated as CURRENT, per the README) is unchanged. About 7 lines.

4. Resource management in the new version-detection code

  • detectCentralVersion() leaks the Toolhelp snapshot handle on both the early-return and success paths (central_version.cpp). Additionally, a two-component ProductVersion string such as "7.8" passes the existing dot checks and produces an invalid from_chars range for the minor version. The fix closes the handle on all paths and rejects version strings without two distinct dots. About 8 lines.
  • CentralBootstrapAdapterBase lacks a virtual destructor but is owned and deleted through a base-class unique_ptr (base.h). The compiler flags this. The fix adds a defaulted virtual destructor. 2 lines.

5. Public C API

  • types.h no longer compiles as C, which breaks cbsdk.h. The macro-to-constexpr conversion wrapped the entire body of types.h in #ifdef __cplusplus, so a C translation unit sees an empty header. cbsdk.h is documented as a C API (C usage example, extern "C", stdbool.h) and its callback typedefs reference cbPKT_GENERIC and cbPKT_GROUP. Verified: a C11 file including cbsdk.h compiles on master and fails on this branch. No in-repo consumer is affected (all are C++ or ctypes), so only external C users see the break. The fix is either restoring a C-visible subset of types.h (the constants and the packet structs cbsdk.h references) or dropping C support explicitly, with a clear preprocessor error and updated cbsdk.h documentation. This one needs a decision on which direction to take.

6. Test coverage

  • Add per-version golden-size assertions. The adapter tests are set/get round trips, which cannot detect layout errors because both directions share the same struct definitions; the file's own LIMITATION note in test_central_adapters.cpp and the TODO scaffolds around line 330 describe this gap. The fix adds assertions comparing each bootstrap adapter's buffer sizes to the known sizes for the corresponding Central version; these would have caught the section 1 findings immediately and will catch future copy-diff drift. About 50 to 100 lines, additive.

7. Nits

  • Stale accessor return docs. sdk_session.h and three siblings: the return documentation still describes the previous pointer-returning contract; these methods now return Result copies.
  • Unclosed Doxygen group. types.h: the group closer is a plain comment, so the group never closes.
  • Unused example includes. central_client.cpp: unused cbsdk includes; one quoted include among angle-bracket includes.
  • Nonexistent type list in the runbook. multi_version_central_compat/README.md (and line 234): references an NspWritableVersions type list that does not exist in test_central_adapters.cpp; only AllVersions is defined.
  • Stale value comments. central_types/v7_7.h (lines 692 and 695): value comments carried over from v7_8 (384 should be 256; 24 should be 18).
  • Switch default-case ordering. In central_version.cpp, the default case placed before case 8 in the minor-version switch compiles fine but is easy to misread.
  • Case-sensitive process-name match. The "Central.exe" comparison in central_version.cpp is case-sensitive; Windows process names are not.

There are also other issues found during this code review, but things that this PR didn't touch on but still affect correctness. If you have time to tackle those within this PR, I can list them as well. Otherwise I'll open an issue.

The 'reserved' field in v7_5 should be an array of two uint8_t instead of just one.
cbPKT_SPKCACHELINECNT should be cbMAXCHANS to match the comment, but it was left as cbNUM_ANALOG_CHANS by accident.
Add the missing cbFILTDESC label.
Zero out the clock sync fields in NativeConfigBuffer.
Remove optiontable and colortable from NativeConfigBuffer.
Legacy cbPKT_AOUT_WAVEFORM.trig maps to current trig with trigInst set to 0.
@cshmookler
cshmookler force-pushed the multi-version-central-compat branch from 2919497 to e3b3ff5 Compare July 14, 2026 22:15
Fix cbPKT_CHANINFO.monsource translation for Central 7.0 and 7.5 to match CCFUtilsBinary.
This fix affects the relevant Central adapters and PacketTranslator methods.
@cshmookler
cshmookler force-pushed the multi-version-central-compat branch from 76a1bfd to 9bc6fe3 Compare July 14, 2026 23:10
Move getMaxProcs from CentralAdapterBase to CentralBootstrapAdapterBase.
Add factory makeAdapter on the bootstrap base.
Add CentralAdapterArgs which bundles the 7 construction parameters.
…of Central

Reject unsupported or unrecognized Centrla versions with errors instead of returning UNKNOWN.
Match the name of Central's executable in a case-insentitive way.
Central constants prefixed with 'CENTRAL_'.
This workaround is necessary because types.h is inherited from cbproto and contains macros which collide with constants with the same name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants