Repository navigation
Conversation
Add cross-platform support so the keyboard LED driver builds and runs on Linux using the hidapi library (hidraw backend) for HID communication. Changes: - Add platform.h compatibility header (DeviceHandle, UINT8 typedefs) - Replace Windows headers with platform.h in all public headers - Split USB implementation: usb_functions_win.cpp / usb_functions_linux.cpp - Linux impl maps HidD_SetFeature -> hid_send_feature_report (byte-perfect) - Linux impl maps IOCTL_HID_GET_FEATURE -> hid_get_feature_report - Update CMakeLists.txt: conditional sources, pkg-config hidapi, MSVC guards - Add Linux CMake presets (linux-debug, linux-release) - Add setup.sh for Linux builds - Add udev rules (99-keyboard-leds.rules) for non-root access - Add .github/workflows/linux.yml CI workflow - Port demo apps: dlopen/dlsym on Linux, terminal raw input for key polling - Fix missing includes (cstring, cstddef) exposed by stricter GCC - Platform-conditional tests for device_path assertions Resolves: #1 (Linux Platform Support epic) Resolves: #2 (Enable Linux build support via hidapi) Resolves: #3, #4, #5, #6, #7, #8, #9, #10, #11, #12, #13, #14, #15
- README.md: Add Linux CI badge, replace Windows-only description with cross-platform language (blink.dll / libblink.so), add Linux build section with setup.sh and udev rule instructions - docs/env_setup_and_building.md: Rewrite as cross-platform doc with Linux-first section (setup.sh, manual cmake commands, udev rules, integration test env var), retain Windows section with streamlined troubleshooting
…h that impl when I did this on windows...
…un at once, one will have to bail; quietter errors
… macro Both MSVC legs of the CI matrix (x86_64-windows-msvc and aarch64-windows-msvc) failed at the Build step with: test/recording_usb_io.h(11): error C2236: unexpected token 'struct' test/recording_usb_io.h(11): error C2332: 'struct': missing tag name test/gif_to_sk80_test.cpp(82): error C2059: syntax error: ',' The Windows SDK's combaseapi.h defines `interface` as a macro expanding to `struct`, and it reaches these translation units through platform.h -> windows.h. That turned the `std::string interface;` member of blink::OpEntry into `std::string struct;`, which cascaded into the push_back and EXPECT_EQ errors further down. Renamed the member to `iface` and added a comment so it doesn't come back. Verified by compiling test/gif_to_sk80_test.cpp with -Dinterface=struct to simulate the Windows SDK: the previous revision fails, this one compiles clean. The full ci preset (configure, build, ctest) also passes on Linux.
Every release run since v1.0.1-rc.1 died at the Configure step, on all four
matrix legs, with:
CMake Error at CMakeLists.txt:7 (project):
VERSION "1.0.1-rc.3" format invalid.
release.yaml derives BLINK_VERSION from the tag (v1.0.1-rc.3 -> 1.0.1-rc.3) and
passes it straight to project(VERSION), which only accepts a numeric
major[.minor[.patch[.tweak]]]. Any -rc/-beta/-alpha tag was therefore
unbuildable; the pipeline could only ever ship final versions.
Split the two meanings of "version" that were being conflated:
* BLINK_VERSION_CORE is the numeric prefix (1.0.1). It drives project(),
SOVERSION, blinkConfigVersion.cmake and blink.pc, all of which require
numeric components and perform version comparisons.
* BLINK_VERSION stays the full string (1.0.1-rc.3) and is what
BLINK_VERSION_STRING, and so blink_version_string(), reports. A release
candidate therefore stays identifiable at run time instead of
masquerading as the final release.
A BLINK_VERSION that does not start with a numeric component now fails with an
explicit message rather than CMake's terse one.
Verified locally with the exact failing version: configure succeeds, the
library is libblink.so.1.0.1 with SOVERSION 1, blink_version_string() returns
"1.0.1-rc.3", and blinkConfigVersion.cmake/blink.pc both carry 1.0.1. The
default 0.2.0 path is unchanged.
Bind the public C ABI in blink.h from Rust, in the conventional -sys plus
wrapper arrangement. Only the C ABI is bound; the internal C++ headers that
BLINK_INSTALL_CXX_HEADERS installs are deliberately out of scope.
rust/blink-sys
Raw FFI declarations plus the build script that locates the native library:
BLINK_LIB_DIR + BLINK_INCLUDE_DIR, then pkg-config, then the `vendored`
feature which builds the CMake project from source. All four modes are
exercised locally.
The declarations in src/bindings.rs are written by hand so that building the
crate needs no libclang; the `bindgen` feature regenerates them from the
header instead. CI runs the whole suite against both, so a signature or
layout that drifts fails to compile or trips tests/abi.rs.
Two things the build script has to get right:
* VCPKG_MANIFEST_NO_DEFAULT_FEATURES, or the vendored build also compiles
gtest and google-benchmark, which the root vcpkg.json enables by default.
* rerun-if-changed on the library file itself. Cargo tracks a build
script's declared inputs, not the link line, so reinstalling a different
build of blink into the same prefix would otherwise leave cargo "fresh"
and silently relink the stale archive. Reproduced before fixing.
rust/blink
RAII handles, Result-based errors, slices in place of the ABI's
caller-allocated buffer protocols, and a UsbIo trait for supplying a USB
transport in Rust.
Contract details the wrapper enforces:
* Keyboard and Lcd are Send but not Sync: the library takes no locks, so a
handle must not be driven from two threads at once.
* The thread-local last-error message is copied out before returning; the
pointer is only valid until the next call on that thread.
* Every UsbIo trampoline body runs inside catch_unwind, because unwinding
into C is undefined behaviour and the header forbids it.
* blink_key_value is OFF=0/ON=1, the opposite of the internal enum, so
KeyState maps it explicitly and never casts.
Note the enumeration probe returns BLINK_ERR_BUFFER_TOO_SMALL rather than
BLINK_OK whenever the result is non-empty, since a zero capacity cannot hold
it; out_count is written either way. The header's prose does not say so and
a naive reading of it fails.
pkg-config
cmake/blink.pc.in and BLINK_INSTALL_PKGCONFIG (on except under MSVC) install
lib/pkgconfig/blink.pc, so consumers that cannot run CMake can find the
library. prefix is derived from ${pcfiledir} with a computed hop count
rather than baked in, so the release archives stay usable wherever they are
extracted and `cmake --install --prefix` is honoured. portfile.cmake calls
vcpkg_fixup_pkgconfig() so vcpkg's post-build check accepts it.
CI
The x86_64 legs build and test the crates against the tree already staged for
the consumer check, rather than rebuilding from source, and a Linux-only step
regenerates the bindings with libclang and re-runs the suite.
Docs
README.md said the public API "takes std::vector and std::string. That makes
it a C++ ABI rather than a true C one". That has been false since 0.2.0 and
is the premise these bindings rest on, so it is replaced with an accurate
description; the toolchain caveat is kept but scoped to the static archive,
where it still holds.
docs/env_setup_and_building.md claimed the ci preset "reads the triplet from
$VCPKG_TARGET_TRIPLET". It deliberately does not -- CI passes
-DVCPKG_TARGET_TRIPLET on the command line, because $env{} expands to an
empty cache entry when unset and vcpkg rejects it. Following that text would
have reintroduced the bug the presets were changed to avoid. The local repro
it documents also exported VCPKG_ROOT, the same anti-pattern.
link the shared library from the Rust CI steps
v1.0.1-rc.4 got the C++ half of every leg green -- the pre-release version fix
works -- but both x86_64 legs then failed in the new Rust steps, each for a
different reason.
Linux: "unable to find library -lhidapi-libusb".
blink_static exposes hidapi as a PUBLIC usage requirement, so a static link
has to resolve it on the final link line. blink-sys asks pkg-config, which
works locally against a system hidapi but not on the runner, where hidapi is
built by vcpkg and never installed system-wide. Pointing PKG_CONFIG_PATH at
the vcpkg tree would not have been enough either: its hidapi-libusb.pc
declares no Requires.private or Libs.private, so libusb and libudev would
have had to be guessed at too.
Link the shared library instead. libblink.so encapsulates hidapi behind
--exclude-libs, which is exactly what it exists for, and is also the artefact
most consumers actually link. LD_LIBRARY_PATH covers the run-time lookup.
Windows: link.exe failed with "extra operand".
Git for Windows ships a coreutils link.exe in /usr/bin. This workflow
defaults to the bash shell, where that directory precedes the MSVC toolchain,
and rustc invokes `link.exe` by bare name -- so it got coreutils' link rather
than the linker. The C++ build is immune because CMake resolves the linker to
an absolute path at configure time, which is why this only ever showed up in
the Rust steps. Prepend the directory holding cl.exe so the right link.exe
wins, and put stage/bin on PATH so blink.dll is findable at run time.
rust/README.md claimed static linkage was the path of least resistance and what
CI used. Both halves are now wrong, and the reason static is awkward -- the
public hidapi usage requirement -- is worth stating, so the section is rewritten
rather than patched.
Verified locally against a staged install: fmt, clippy -D warnings, 42 tests and
the examples all pass dynamically linked, with ldd confirming libblink.so.1.
Troubleshooting Fixes: make the README discovery examples runnable
The shared-library switch left both worked examples without LD_LIBRARY_PATH, so
they would build and then fail at run time exactly as written. Both now carry it
and are verified verbatim.
Troubleshooting Fixes: convert $WS to a POSIX path before putting it on PATH
v1.0.1-rc.5 fixed the Windows linker shadowing and both Linux legs went green,
but the x86_64 Windows tests then died with STATUS_DLL_NOT_FOUND: blink.dll was
not on PATH despite the previous commit adding it.
$WS is a Windows path (D:/a/...) because it comes from `pwd -W`, and PATH in
the bash shell this workflow uses is colon-separated. The drive-letter colon
therefore split the entry into "D" and "/a/.../stage/bin", neither of which
exists, so the loader never found the DLL. cygpath -u converts it to the POSIX
form bash expects.
Only the test binaries that actually reference a blink_ symbol failed: with
nothing referenced, MSVC drops the unused import entirely and the binary has no
DLL dependency to resolve, which is why the lib unittests passed and tests/
hardware.rs did not.
The staged bin directory is listed as well, so a future failure here shows
immediately whether the DLL is present or merely unreachable.
Troubleshooting Fixes: stop asserting a Linux-only LCD detail on Windows
rc.6 got the Windows DLL loading -- stage/bin was listed and blink.dll was
found, hardware.rs and offline.rs both ran clean -- leaving one real test
failure:
assertion failed: !defaults.data_interface.is_empty()
That assertion was wrong, not the library. sk80's lcd_data_device_path is ""
on Windows by design: the HID path for the LCD data interface has never been
captured and the Windows LCD backend is still a stub, so the constant exists
only so the shared code compiles. Only the Linux branch carries a real value.
Assert the platform-specific truth in both directions rather than relaxing the
check, so whichever side changes first trips the test instead of quietly
diverging. The Windows branch carries a message pointing at the backend work
that would have to happen alongside it.
Troubleshooting Fixes: publish -rc tags as pre-releases
v1.0.1-rc.7 built and published cleanly, but landed as a full release: it took
over the repository's "Latest" badge and became what `gh release download`
resolves to by default, so a release candidate would be handed to anyone
following the README's download instructions.
Mark the release as a pre-release whenever the resolved version carries a SemVer
pre-release identifier, which is anything after a hyphen. Final tags are
unaffected.
…ry named libkeylt, scrapping the gif_to_sk80 binary
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.
No description provided.