Repository navigation
feat: Implement main loop - #31
Merged
Merged
Conversation
Closed
MichalSzandar
marked this pull request as draft
July 1, 2026 16:20
MichalSzandar
marked this pull request as ready for review
August 4, 2026 17:37
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical and moderate issues remain in portability, file safety, acquisition failure handling, window closure, and CLI validation.
4 open findings
What changed in this PR
Implements the runtime main loop for EEG-gated experiments, rendering, recording, and shutdown.
Changes:
- Adds runtime orchestration, acquisition tracking, and CLI handling.
- Adds display/output configuration and format strategies.
- Adds CSV writing, validation, tests, documentation, and build integration.
| File | Summary |
|---|---|
tests/unit_tests/runtime_test.cpp |
Runtime lifecycle and acquisition tests. |
tests/unit_tests/lslreader_test.cpp |
Acquisition state transition tests. |
tests/unit_tests/data_writer_test.cpp |
CSV overwrite protection tests. |
tests/unit_tests/data_format_strategy_factory_test.cpp |
Format strategy creation tests. |
tests/unit_tests/config_validation_test.cpp |
Configuration validation tests. |
tests/unit_tests/config_parser_test.cpp |
Configuration parsing tests. |
src/Runtime.cpp |
Runtime orchestration. Critical, 1 vote: POSIX-only localtime_r prevents Windows compilation. Nit, 2 votes: timeout text truncates sub-second durations. Moderate, 1 vote: close-button events are not handled during acquisition or rendering. |
src/main.cpp |
CLI argument handling. Moderate, 4 votes: accepts and ignores extra arguments. |
src/lslreader/LSLReader.cpp |
Acquisition state management. Critical, 1 vote: lost streams can retry indefinitely without transitioning to failure. |
src/datawriter/DataFormatStrategyFactory.cpp |
Output strategy selection. |
src/datawriter/CSVFormatStrategy.cpp |
Exclusive CSV creation. Critical, 2 votes: closing the exclusive handle before reopening creates a TOCTOU overwrite gap. |
src/datawriter/CMakeLists.txt |
Registers the strategy factory source. |
src/config/ConfigValidation.cpp |
Validates display and output configuration. |
src/config/ConfigParser.cpp |
Parses display and output configuration. |
README.md |
Documents runtime and configuration behavior. |
include/Runtime.hpp |
Runtime API and lifecycle definitions. |
include/lslreader/LSLReader.hpp |
Acquisition state API. |
include/datawriter/IDataFormatStrategy.hpp |
Data writer strategy interface. |
include/datawriter/DataFormatStrategyFactory.hpp |
Strategy factory declaration. |
include/datawriter/CSVFormatStrategy.hpp |
CSV strategy metadata. |
include/config/OutputConfig.hpp |
Output configuration model. |
include/config/DisplayConfig.hpp |
Display configuration model. |
include/config/DeviceConfig.hpp |
Device configuration updates. |
.gitignore |
Ignores local JSON configuration files. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
InfoTCube
added a commit
that referenced
this pull request
Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- wait for first EEG sample before rendering; stop run if acquisition fails - make run() single-shot, never overwrite output files, readable file names - require vsync and add optional "display" section to config.json - validate config before opening the window, enforce run() thread - fix scene/renderer teardown order, update tests and README
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…able
- write recordings through the handle created with fopen("wx") (new ExclusiveOutputFile)
instead of reopening the path, which could truncate another file
- format the file-name timestamp with std::chrono time zones instead of POSIX localtime_r
MichalSzandar
force-pushed
the
feat/main-loop
branch
from
October 9, 2026 15:47
1a0ce1b to
9751e5a
Compare
MichalSzandar
pushed a commit
that referenced
this pull request
Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 <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
Adds the runtime main loop.
NeuronIDE config.json experiment.neurozwaits for the EEGstream, records EEG and markers to a new file, and runs the experiment until the window
is closed.
Changes
Runtime: checks the whole config before opening the window, then:<experiment>_<YYYYMMDDTHHMMSS>.csvand rendersconfig.json: new optionaldisplay(monitor, fullscreen, size) andoutput.formatLSLReader: reports its acquisition state soRuntimecan wait for samples or detect failureDataWriter: newDataFormatStrategyFactory; CSV output never overwrites an existing fileNotes
"display": { "fullscreen": false }for development.