Start working #5
Merged
Conversation
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
- Introduced new source files for serial communication: `serial_open.cpp`, `serial_close.cpp`, `serial_read.cpp`, and `serial_write.cpp`. - Implemented error handling and callback mechanisms for serial operations. - Added a test suite for serial communication with Arduino, covering open/close, read/write operations, and timeout behavior. - Configured CMake to copy compile commands to the project root for easier debugging. - Removed the placeholder source file to clean up the project structure.
…or_code for improved error messages - Replaced usage of strerror(errno) with std::error_code for generating error messages in `serial_open.cpp`, `serial_close.cpp`, `serial_read.cpp`, and `serial_write.cpp`. - Updated includes to replace <cstring> with <system_error> for better error handling practices.
- Created a new `.gitignore` file to exclude Deno artifacts and test snapshots. - Added `deno.json` for task configuration and dependency management. - Introduced `deno.lock` to lock dependencies for consistent builds. - Implemented `ffi_bindings.ts` for FFI bindings to the cpp-bindings-linux library. - Developed `integration_test.ts` to cover various scenarios including error handling, invalid parameters, and real serial port interactions.
- Updated error callback comments in `ffi_bindings.ts` for better readability. - Reorganized status codes in `integration_test.ts` to use negative values for error handling. - Simplified assertions in tests to directly compare expected and actual values, enhancing test clarity. - Removed unnecessary comments and code to streamline the test suite.
- Simplified the `createErrorCallback` function by removing unnecessary destructuring of arguments. - Enhanced type definitions for better TypeScript support and readability.
…dability - Added formatting options in `deno.json` for consistent code style. - Refactored `ffi_bindings.ts` and `integration_test.ts` to improve code readability by standardizing indentation and line width. - Updated integration tests to maintain clarity and consistency in test structure.
Katze719
requested review from
Mqxx and
Copilot
and removed request for
Copilot
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements the core serial communication functionality for the cpp-bindings-linux library, providing C-compatible FFI bindings for serial port operations (open, close, read, write) on Linux systems. The implementation includes comprehensive test coverage through both C++ unit tests and Deno FFI integration tests, along with development tooling configuration.
- Implements POSIX-based serial communication with custom termios2 support for arbitrary baud rates
- Adds comprehensive test coverage with hardware-dependent tests that gracefully skip when devices are unavailable
- Configures development tooling (.clang-tidy, .clang-format) with Microsoft style guide and removes obsolete GPL-2.0 license
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/serial_open.cpp | Implements serialOpen with termios2 configuration for arbitrary baud rates and serial port parameters |
| src/serial_close.cpp | Implements serialClose with proper error handling for invalid handles |
| src/serial_read.cpp | Implements serialRead with timeout-based polling and progressive buffer filling |
| src/serial_write.cpp | Implements serialWrite with non-blocking I/O retry logic and tcdrain for completion |
| tests/test_serial_arduino.cpp | Adds C++ unit tests for serial operations expecting Arduino echo device |
| integration_tests/integration_test.ts | Adds Deno FFI integration tests for cross-language binding validation |
| integration_tests/ffi_bindings.ts | Provides TypeScript FFI bindings and utility functions for Deno tests |
| integration_tests/deno.json | Configures Deno test environment and dependencies |
| integration_tests/deno.lock | Locks Deno dependencies (@std/assert) |
| integration_tests/.gitignore | Adds Deno-specific gitignore patterns |
| CMakeLists.txt | Removes CTest integration and adds compile_commands.json copying |
| .clang-tidy | Adds comprehensive clang-tidy configuration with Microsoft conventions |
| .clang-format | Adds clang-format configuration for consistent code formatting |
| LICENSE | Removes GPL-2.0 license (339 lines) |
| src/placeholder.cpp | Removes placeholder namespace file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mqxx
requested changes
Dec 12, 2025
- Introduced a GitHub Actions CI workflow in `ci.yml` to automate C++ and Deno integration tests. - Configured separate jobs for C++ tests using GCC and Deno tests with specified versions. - Set up environment variables and build dependencies for both test environments. - Updated Deno integration tests to reflect minimal FFI bindings and removed unnecessary error handling functions. - Simplified integration test structure to focus on library loading and unloading.
…tion - Changed CI workflow to use a Fedora container for better CMake support. - Removed matrix strategy for GCC versions, simplifying the job configuration. - Updated package installation commands to use `dnf` instead of `apt-get` for Fedora compatibility. - Adjusted environment variable settings to reflect the new container setup.
- Added g++ to the list of build dependencies in the CI workflow. - Removed unnecessary comments related to GCC configuration for clarity. - Ensured environment variable settings for CMake are properly configured.
…th minimal environments - Defined BOTHER in serial_open.cpp if not already defined, ensuring compatibility with certain libcs or kernel headers that may not include it. - This change supports builds in minimal environments, such as Deno's Debian-based CI containers.
Mqxx
requested changes
Dec 16, 2025
- Introduced a GitHub Actions workflow for building a shared library for Deno, including steps for dependency installation, CMake configuration, and artifact collection. - Added a separate workflow for running C++ unit and integration tests, utilizing a Fedora container and configuring the environment for serial communication testing. - Both workflows are triggered on pushes and pull requests to the main branches, ensuring automated testing and builds.
- Replaced direct error handling in serial_open, serial_close, serial_read, and serial_write with utility functions from the new posix_helpers.hpp for improved code clarity and consistency. - Introduced UniqueFd class to manage file descriptors safely and prevent resource leaks. - Simplified error callback invocations to streamline error reporting across serial communication functions.
- Moved the waitFdReady function into an anonymous namespace for better encapsulation. - Simplified the serialRead function by introducing a lambda for reading data, enhancing clarity. - Streamlined the logic for checking data readiness and reading from the file descriptor, reducing complexity and improving maintainability.
…inability - Moved the waitFdReady function to posix_helpers.hpp for better code organization and reuse across serial_read and serial_write. - Updated serial_read and serial_write implementations to utilize the new waitFdReady function, improving readability and reducing code duplication. - Added detailed comments for parity and stop_bits configurations in serial_open.cpp to clarify their usage and constraints.
Co-authored-by: Mqx <62719703+Mqxx@users.noreply.github.com>
…atures readonly - Changed function signatures in the SerialLib type to readonly, ensuring immutability and enhancing type safety. - This update improves the clarity of the API by indicating that these functions should not be modified after their initial definition.
…llation - Changed the runner from ubuntu-20.04 to ubuntu-latest for improved compatibility and support. - Added a container specification to use Ubuntu 20.04. - Updated the dependency installation process to use non-interactive mode and removed unnecessary package lists to streamline the build process.
- Added checks in serial_close, serial_read, and serial_write to ensure that handles do not exceed the maximum limit for integers, improving error handling and robustness. - Included <limits> header to facilitate the validation of handle values across the functions.
…ed type safety - Updated the SerialLib type to utilize Deno.DynamicLibrary for better type inference and clarity. - Consolidated symbol definitions into a single object, enhancing organization and maintainability. - Adjusted the loadSerialLib function to reflect the new type structure, ensuring consistency across the codebase.
- Introduced a new GitHub Actions workflow for publishing the native Linux shared library as a JSON/base64 blob to JSR. - Added scripts for embedding the shared library and generating the corresponding JSON metadata. - Created TypeScript bindings for the shared library, enhancing usability and type safety. - Updated CMake configuration to generate JSR package metadata during the build process. - Included a README for usage instructions and permissions required for the library.
- Updated cmake_git_versioning GIT_TAG from v1.0.0 to v1.0.1 for the latest features and fixes. - Changed cpp_core GIT_TAG from main to bump-version-from-git-versioning to align with versioning strategy.
- Updated the publish-jsr.yml workflow to include permissions for reading contents and writing ID tokens, enhancing security and functionality during the publishing process.
- Introduced a new LICENSE file containing the GNU Lesser General Public License v3.0. - Updated jsr.json.in to specify the license as LGPL-3.0-only. - Added a license section in the README.md to inform users about the licensing terms.
… publishing - Modified the publish-jsr.yml workflow to include the --allow-dirty flag for deno publish, enabling the publishing process to proceed even when the worktree is dirty. - Added comments to clarify the impact of CMake and embed_so.ts on the worktree state, ensuring deterministic builds and generation in a single run.
- Updated the JSR package to use a new directory structure for binaries, changing the path from `jsr/binaries/linux-x86_64.json` to `jsr/bin/linux-x86_64.json`. - Deleted the `ffi.ts` and `mod.ts` files, removing the TypeScript bindings for the shared library. - Updated the README.md to reflect the new usage instructions for importing the JSON and writing the shared library to disk. - Added a new `linux-x86_64.json` file to store the shared library metadata.
- Changed references from `linux-x86_64` to `x84_64` in the JSR package configuration and scripts. - Updated the JSON metadata file to reflect the new naming and added the new `x84_64.json` file. - Modified the README.md and embed_so.ts script to align with the new import path for the shared library.
Mqxx
requested changes
Dec 16, 2025
Co-authored-by: Mqx <62719703+Mqxx@users.noreply.github.com>
- Adjusted formatting of the JSON/base64 blob description for clarity. - Added a new section to specify the package license as LGPL-3.0-only.
…om-git-versioning to main
Mqxx
approved these changes
Dec 16, 2025
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.
No description provided.