Refactor cmp#176
Merged
Merged
Conversation
Real-world CMP files have out-of-order rows, and one device may carry
multiple headstages whose label namespaces collide. Replace the
(bank, term)-keyed overlay with one keyed by absolute chan_id:
- parseCmpFile sorts rows by (bank, electrode) and assigns
chan_id = start_chan + sorted_index
- hs_id (default 0 = no prefix) prefixes labels "hs{hs_id}-..."
- loadChannelMap pushes prefixed labels via CHANSETLABEL so they
persist on the device; positions stay local but are re-applied
on the CHANREP receive path so device echoes can't clobber them
- bank_offset is gone — start_chan supersedes it
Updates the C API, C++ API, unit tests, and SDK/CAPI integration
tests in lockstep.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bundled DefaultMapping files have rows already in (bank, elec) order — useful for parser tests but not realistic. Real .cmp files from the manufacturer come unsorted. Add a sanitized 128-channel sample (description scrubbed of array IDs and serial number) and point the integration tests at it so they exercise the sort and chan_id assignment under genuine layouts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match cbsdk_session_load_channel_map's new (start_chan, hs_id) parameters and drop bank_offset. The Python wrapper keeps the same defaults as the C++ side (start_chan=1, hs_id=0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add pycbsdk.cmp with parse_cmp() (returns chan_id → CmpEntry) and a
CLI that connects to a device and applies one or more
FILE:START_CHAN:HS_ID specs. parse_cmp matches the C++ parser's
behavior: rows are sorted by (bank, electrode), assigned absolute
chan_ids from start_chan, and labels are prefixed "hs{hs_id}-..."
unless hs_id is 0 (no prefix).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TestCMP went from 3 mostly-smoke tests with one assertion to 7 with real correctness checks: positions match the parser, labels round- trip through the device (load → sync → read), prefix changes rewrite labels, two-headstage start_chan layouts agree, and the CHANREP-echo path doesn't erase the local overlay. Replaces time.sleep() with session.sync() — the proper barrier for the async CHANSETLABEL → CHANREP round-trip. Adds a manufacturer_cmp_path fixture for the 128-ch sample. Co-Authored-By: Claude Opus 4.7 (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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Real-world
.cmpfiles break two assumptions our previous loader made: rows are not in(bank, electrode)order, and one device may carry multiple headstages whose label namespaces (elec1-1…elec1-128) collide. This PR rewrites the loader around those facts.bank_offsetis gone.loadChannelMapnow takes(filepath, start_chan=1, hs_id=0). The parser sorts rows by(bank, electrode)and assignschan_id = start_chan + sorted_index, so each CMP describes one headstage applied to a contiguous channel range.hs_id != 0, every label is prefixed"hs{hs_id}-". The loader sendsCHANSETLABELper channel so the device persists the prefixed labels in chaninfo.hs_id=0skips the prefix (sensible for single-headstage rigs).chan_id. Both theCHANREPreceive path and the post-load batch re-apply look up by absolute channel number; positions are re-applied on every device-echoedCHANREPso a device refresh can't clobber locally-supplied geometry.loadChannelMaponce per headstage with disjointstart_chanranges (e.g. 1, 129, …). Subsequent calls merge into the overlay.API changes (breaking)
SdkSession::loadChannelMap(path, bank_offset=0)SdkSession::loadChannelMap(path, start_chan=1, hs_id=0)cbsdk_session_load_channel_map(s, path, bank_offset)cbsdk_session_load_channel_map(s, path, start_chan, hs_id)Session.load_channel_map(path, bank_offset=0)Session.load_channel_map(path, start_chan=1, hs_id=0)New:
pycbsdk.cmpparse_cmp(path, start_chan=1, hs_id=0)returnsdict[chan_id, CmpEntry]mirroring the C++ parser.python -m pycbsdk.cmp head1.cmp:1:1 head2.cmp:129:2 --device NSPapplies one or moreFILE:START_CHAN:HS_IDspecs to a live device, or--dumpprints the parsed entries without connecting.New test fixture
tests/128ChannelManufacturerMapping.cmp— sanitized 128-channel sample from a real manufacturer CMP (description scrubbed of array IDs and serial). The integration tests now point at this file so they exercise out-of-order rows.Test plan
cmp_parser_tests): 6 tests pass — sort order,start_chanoffset,hs_id=0no-prefix path,hs_id != 0prefix path, missing-file error.tests/test_cmp.py): 12 tests pass — same coverage on the Python parser plus CLI spec parsing and--dumpoutput.TestCMP, 7 tests, against nPlayServer): smoke load, error path, positions match parser, labels round-trip through device aftersync(),hs_idchange rewrites only the prefix, two-headstagestart_chanlayouts agree, CHANREP-echo doesn't erase the overlay.dnss256with the new manufacturer fixture asNPLAY_CMP_FILE.Migration notes
load_channel_map(path)(which used to applybank_offset=0) with the same call — no change at the call site, but device labels are now untouched (no prefix whenhs_id=0).load_channel_map(path, bank_offset=4*N)withload_channel_map(path, start_chan=1+N*chans_per_hs, hs_id=N+1).