Skip to content

feat: implement cymon-lib — DSDL protocol, C++ device API, gtest suite, CI pipeline#1

Merged
Tecnologic merged 7 commits into
mainfrom
copilot/add-cyphal-node-tracing-ability
May 20, 2026
Merged

feat: implement cymon-lib — DSDL protocol, C++ device API, gtest suite, CI pipeline#1
Tecnologic merged 7 commits into
mainfrom
copilot/add-cyphal-node-tracing-ability

Conversation

Copilot AI commented May 19, 2026

Copy link
Copy Markdown
Contributor
  • .clang-format: ColumnLimit 80 → 140; reformat all C++ sources
  • GetBufferInfo.1.0.dsdl: rename max_samples_per_channelmax_frames
  • GetVariableList.1.0.dsdl: inline Variable fields; delete Variable.1.0.dsdl
  • ReadSamples.1.0.dsdl: remove redundant channel_ids; add frame_offset comment
  • include/cymon/config.hpp: add clarifying doc-comments; kMaxChannels default changed 16 → 8 with CYMON_MAX_CHANNELS preprocessor override; static_assert(kMaxVars <= 255) and static_assert(kMaxChannels <= 255) hard ceilings added
  • Add kBadConfig error code; add num_channels bounds checks in HandleSetupCapture; add two gtest cases
  • Fix end_of_data: use frame_offset + frames_read >= total only (drop || IsComplete()); add paging gtest
  • Fix ReadFrames() pre-trigger check: snapshot total_written_written_before_trigger_ at trigger time; add early-trigger zero-fill gtest
  • RegisterVariable(): remove unsafe static_cast<uint8_t>(kMaxVars) comparison; rely on static_assert to guarantee safe bounds
  • Build + 37/37 tests pass; clang-format and clang-tidy clean; CodeQL clean

Copilot AI and others added 3 commits May 19, 2026 19:43
- Add DSDL types: Variable, TriggerMode, GetVariableList, GetBufferInfo,
  SetupCapture, ArmTrigger, ReadSamples, TriggerEvent
- Add include/cymon/{config,trigger,sample_buffer,device}.hpp headers
- Add src/cymon/{trigger,sample_buffer,device}.cpp implementations
- Add CMakeLists.txt with gtest-based tests (FetchContent v1.14.0)
- Add cmake/nunavut.cmake for optional DSDL code generation
- Add .clang-format (Google style, 80 col) and .clang-tidy config
- Add .github/workflows/ci.yml with build/test/format/tidy steps
- Update README.md with CI badge, features, protocol table, build docs
- Update .gitignore with generated/ directory

All 33 tests pass; clang-format clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Tecnologic <1442404+Tecnologic@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Tecnologic <1442404+Tecnologic@users.noreply.github.com>
Comment thread .clang-format Outdated
@@ -0,0 +1,3 @@
---
BasedOnStyle: Google
ColumnLimit: 80

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot Change to 140.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — changed to ColumnLimit: 140 in 6c923af. All C++ source files were also reformatted with the new limit.

Comment thread dsdl/cymon/GetBufferInfo.1.0.dsdl Outdated
---
uint32 buffer_bytes
uint8 max_channels
uint16 max_samples_per_channel

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot how this should be calculated? Max samples for 1 active channel differs from max samples for 8 active channels

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. The field was renamed to max_frames (6c923af) and is now calculated as buffer_bytes / sizeof(float) — the maximum frame depth with a single channel. The monitor divides by its planned channel count to get the actual depth: max_frames_for_N = max_frames / N. That way the DSDL response is independent of the current configuration, and the monitor can plan any channel count it likes.

Comment thread dsdl/cymon/GetVariableList.1.0.dsdl Outdated
uint16 index
---
uint8 id
cymon.Variable.1.0 variable

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot is cymon variable used only here? If yes remove the type definition an add all values directly here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cymon.Variable.1.0 was only used here. Variable.1.0.dsdl has been deleted and its two fields (name, unit) are now inline in GetVariableList.1.0.dsdl (6c923af).

Comment thread dsdl/cymon/ReadSamples.1.0.dsdl Outdated

# Paged read-out of captured samples.

uint16 frame_offset

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@wahts the purpose of this offset

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

frame_offset is the page index for paged read-out. A CAN-FD payload is limited, so a single ReadSamples response carries only a few frames at a time (max_frames). The monitor fetches the full capture by incrementing the offset: first call with frame_offset=0 returns frames 0…N-1, next call with frame_offset=N returns frames N…2N-1, and so on. The end_of_data flag tells the monitor when it has received the last page. A comment has been added to the DSDL field in 6c923af.

Comment thread dsdl/cymon/ReadSamples.1.0.dsdl Outdated
bool triggered
bool end_of_data
uint8 num_channels
uint8[<=16] channel_ids

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why sending the channel IDs. They are known to the monitor node because it did the setup

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

channel_ids has been removed from the response in 6c923af. The monitor already knows which channels it configured, so echoing them back was redundant.

Comment thread include/cymon/config.hpp
…le, remove channel_ids, max_frames)

Agent-Logs-Url: https://github.com/Tecnologic/cymon-lib/sessions/b9b89e2e-4b92-4b01-a818-196f76d7d4cf

Co-authored-by: Tecnologic <1442404+Tecnologic@users.noreply.github.com>
Copilot AI requested a review from Tecnologic May 19, 2026 21:33
@Tecnologic Tecnologic marked this pull request as ready for review May 19, 2026 22:05
@Tecnologic Tecnologic requested a review from Copilot May 19, 2026 22:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Implements the initial cymon-lib C++17 device-side library and its Cyphal-facing DSDL protocol, including a gtest-based test suite and CI pipeline to build/test and enforce clang-format/clang-tidy.

Changes:

  • Added core library components: Trigger, SampleBuffer, and Device APIs + implementations.
  • Introduced unregulated DSDL service/message definitions for variable enumeration and capture (setup/arm/read/trigger event).
  • Added gtest suite + CMake integration and GitHub Actions CI for build/test/format/tidy.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/cymon/trigger.cpp Implements trigger evaluation state machine (rising/falling/hysteresis).
include/cymon/trigger.hpp Public trigger API/types.
src/cymon/sample_buffer.cpp Implements circular pre/post-trigger capture buffer and paged reads.
include/cymon/sample_buffer.hpp Public sample buffer API and fixed-size storage definition.
src/cymon/device.cpp Implements the high-level device API and capture/trigger service handlers.
include/cymon/device.hpp Public Device API, configs, and response structures.
include/cymon/config.hpp Compile-time limits for buffer/vars/channels/read paging.
tests/test_trigger.cpp Unit tests for trigger modes and arming/disarming semantics.
tests/test_sample_buffer.cpp Unit tests for buffer setup, trigger flow, and readout behavior.
tests/test_device.cpp Unit tests covering variable registry and capture/trigger/read handlers.
tests/CMakeLists.txt Fetches GoogleTest and wires up test target discovery.
dsdl/cymon/GetVariableList.1.0.dsdl DSDL iterator-style variable enumeration service.
dsdl/cymon/GetBufferInfo.1.0.dsdl DSDL service for reporting capture buffer limits.
dsdl/cymon/SetupCapture.1.0.dsdl DSDL capture configuration service.
dsdl/cymon/ArmTrigger.1.0.dsdl DSDL trigger arm/disarm service.
dsdl/cymon/ReadSamples.1.0.dsdl DSDL paged interleaved sample readout service.
dsdl/cymon/TriggerMode.1.0.dsdl DSDL enum for trigger mode selection.
dsdl/cymon/TriggerEvent.1.0.dsdl DSDL broadcast message for trigger events.
CMakeLists.txt Top-level library build, optional nunavut generation, and test toggle.
cmake/nunavut.cmake CMake helper to generate C++ types from DSDL using nunavut.
.github/workflows/ci.yml CI pipeline to build, test, and run format/tidy checks.
.clang-format clang-format configuration (Google-based).
.clang-tidy clang-tidy configuration (bugprone/google/modernize/performance/readability).
.gitignore Ignores generated artifacts (incl. nunavut output dir).
README.md Project overview, protocol summary, build instructions, and integration snippet.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/cymon/device.cpp
Comment thread src/cymon/device.cpp Outdated
resp.num_samples = static_cast<uint8_t>(static_cast<std::size_t>(frames_read) * ch);

const uint16_t total = buffer_.num_samples();
resp.end_of_data = (static_cast<uint32_t>(frame_offset) + frames_read >= total) || buffer_.IsComplete();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot add this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e1dc38b. end_of_data is now set purely from frame_offset + frames_read >= total, removing the || buffer_.IsComplete() that incorrectly terminated paging at the end of the first read. A new test HandleReadSamples_EndOfData_PagingIsCorrect verifies a 6-frame capture read in two 3-frame pages: first page returns end_of_data = false, second returns end_of_data = true.

Comment thread src/cymon/sample_buffer.cpp
Comment thread src/cymon/device.cpp Outdated
Comment on lines +19 to +27

namespace cymon {

Device::Device() = default;

void Device::set_actual_sample_period_us(float us) { actual_sample_period_us_ = us; }

bool Device::RegisterVariable(std::string_view name, std::string_view unit, std::function<float()> getter) {
if (var_count_ >= static_cast<uint8_t>(kMaxVars)) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot make 255 a hard limit. To ensure no one gets past it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in c12dad5. config.hpp now has static_assert(kMaxVars <= 255U, ...) immediately after the constant, so overriding CYMON_MAX_VARS above 255 is a compile error. The unsafe static_cast<uint8_t>(kMaxVars) in RegisterVariable() has also been removed — the direct comparison var_count_ >= kMaxVars is now safe since the assert guarantees the value fits.

Comment thread include/cymon/config.hpp
…fig error code

Agent-Logs-Url: https://github.com/Tecnologic/cymon-lib/sessions/283d72b1-50b1-4da3-98c8-a41bee013996

Co-authored-by: Tecnologic <1442404+Tecnologic@users.noreply.github.com>
…a paging logic

Agent-Logs-Url: https://github.com/Tecnologic/cymon-lib/sessions/f3b01c04-d66a-448a-b0fd-762d712e7795

Co-authored-by: Tecnologic <1442404+Tecnologic@users.noreply.github.com>
…ls=8 with CYMON_MAX_CHANNELS override

Agent-Logs-Url: https://github.com/Tecnologic/cymon-lib/sessions/801a3240-6068-428b-a551-5bbd5212769b

Co-authored-by: Tecnologic <1442404+Tecnologic@users.noreply.github.com>
@Tecnologic Tecnologic merged commit 7d9ffec into main May 20, 2026
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