Repository navigation
Add EGI NA400 AmpServer network acquisition - #884
Andrey1994 wants to merge 2 commits into
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c68343670
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int count = data_socket->recv (static_cast<uint8_t *> (data) + offset, int (size - offset)); | ||
| if (count <= 0) | ||
| return false; |
There was a problem hiding this comment.
When params.timeout exceeds five seconds, the data socket still inherits SocketClientTCP's fixed five-second receive timeout. Treating every nonpositive receive result as fatal therefore terminates acquisition before the configured first-sample deadline. With the loopback emulator configured for an eight-second timeout and a valid first sample delayed six seconds, start_stream() returns INCOMMING_MSG_ERROR after about five seconds instead of succeeding. Retry receive timeouts while preserving the partial-read offset, or configure the data socket to honor the acquisition deadline.
Useful? React with 👍 / 👎.
| !command ("cmd_Start", "0", response)) | ||
| { | ||
| data_socket.reset (); | ||
| return (int)BrainFlowExitCodes::BOARD_WRITE_ERROR; |
There was a problem hiding this comment.
If AmpServer executes cmd_Start but its acknowledgement does not arrive before the command timeout, this error path only closes the data socket. Since owns_stream has not been set and no worker exists, a subsequent release_session() also sends no cmd_Stop, leaving the amplifier acquiring after the caller tears down the failed session. This is reproducible by suppressing the emulator's start acknowledgement: start returns BOARD_WRITE_ERROR, but acquisition remains active after release. Perform a best-effort stop when the attempted start fails rather than relying on worker cleanup.
Useful? React with 👍 / 👎.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| def guarded(conn, handler): | ||
| try: | ||
| handler(conn) | ||
| except (BrokenPipeError, ConnectionResetError): |
Adds
EGI_AMP_SERVER_BOARD(72) for NA400 AmpServer Packet Format 2, a 256-channel net and 1000 Hz decimated acquisition. Preparation validates model/format/channel count before control commands; acquisition checks big-endian block framing, decodes unaligned little-endian samples, applies NA400-specific microvolt scaling, and preserves DIN, TR, net code and device timestamp fields. Duplicate sample counters are discarded and gaps are logged.The implementation controls the amplifier through its command port and reads its data port. It rejects NA500 and other profiles rather than assuming their different startup sequences or gains. Socket shutdown interrupts blocked readers, and socket writes suppress SIGPIPE on supported Unix platforms. All binding IDs and documentation are aligned.
Validation: warnings-as-errors Linux build; all 65 native tests pass. The included loopback test covers fragmented commands/data, model/net/ID rejection, duplicate suppression, timeout recovery and repeated sessions; Linux CI runs it. Also successfully acquired 151 samples and stopped against the independent App-EGIAmpServer Python mock. Physical NA400 validation remains outstanding.
Related to #643. Protocol and scaling reference. This initial PR does not implement NA500, smaller nets, impedance or PIB/physio channels.