Skip to content

feat: Implement main loop - #31

Merged
MichalSzandar merged 5 commits into
mainfrom
feat/main-loop
Oct 9, 2026
Merged

MichalSzandar merged 5 commits into
mainfrom
feat/main-loop

Conversation

@MichalSzandar

@MichalSzandar MichalSzandar commented Jul 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds the runtime main loop. NeuronIDE config.json experiment.neuroz waits for the EEG
stream, 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:
    1. waits for the first EEG sample (30 s timeout); the experiment never starts without EEG
    2. records to <experiment>_<YYYYMMDDTHHMMSS>.csv and renders
    3. stops early and throws if EEG acquisition fails mid-run
  • config.json: new optional display (monitor, fullscreen, size) and output.format
  • LSLReader: reports its acquisition state so Runtime can wait for samples or detect failure
  • DataWriter: new DataFormatStrategyFactory; CSV output never overwrites an existing file

Notes

  • Fullscreen by default; use "display": { "fullscreen": false } for development.
  • Won't start without vsync, because marker timestamps depend on it.

@MichalSzandar MichalSzandar linked an issue Jul 1, 2026 that may be closed by this pull request
@MichalSzandar MichalSzandar self-assigned this Jul 1, 2026
@MichalSzandar
MichalSzandar marked this pull request as draft July 1, 2026 16:20
@MichalSzandar
MichalSzandar marked this pull request as ready for review August 4, 2026 17:37
@MichalSzandar MichalSzandar changed the title Feat/main loop Implement main loop Aug 4, 2026
@MichalSzandar MichalSzandar changed the title Implement main loop feat: Implement main loop Oct 9, 2026
@MichalSzandar
MichalSzandar requested a lite review from Copilot October 9, 2026 15:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread src/Runtime.cpp Outdated
Comment thread src/datawriter/CSVFormatStrategy.cpp Outdated
Comment thread src/main.cpp Outdated
Comment thread src/Runtime.cpp Outdated
InfoTCube added a commit that referenced this pull request Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MichalSzandar and others added 5 commits October 9, 2026 17:42
- 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
MichalSzandar merged commit 7219de8 into main Oct 9, 2026
@MichalSzandar
MichalSzandar deleted the feat/main-loop branch October 9, 2026 15:55
MichalSzandar pushed a commit that referenced this pull request Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

Implement main loop

2 participants