Add uhid service and alternative cli #35

felfert · 2023-04-26T19:56:16Z

Description

As announced in #22 , I implemented a uhid driver service together with an alternative cli that acts as a drop-in replacement for cherryrgb_cli. See docs/UHID-driver.md.

Fixes # 22

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Adds/updates Documentation

Code Checklist

I have performed a self-review of my own code
I have made corresponding changes to the documentation
My changes generate no new warnings
I have implemented respective test(s)
I have run lints and tests (cargo fmt && cargo clippy && cargo test) that prove my fix is effective or that my feature works

- Change uhid-virt source temporarily to yet unreleased fix.

Adding strum properties to members enables more sophisticated validation of cmdline parameters.

felfert · 2023-04-26T20:05:29Z

Strange, that all except fmt failed. builds fine here?!

felfert · 2023-04-27T06:09:26Z

Looks like the build system is missing llvm-config and libclang:

2023-04-26T20:10:00.8926365Z warning: could not execute `llvm-config` one or more times, if the LLVM_CONFIG_PATH environment variable is set to a full path to valid `llvm-config` executable it will be used to try to find an instance of `libclang` on your system: "couldn't execute `llvm-config --prefix` (path=llvm-config) (error: No such file or directory (os error 2))"
2023-04-26T20:10:00.8928739Z 
2023-04-26T20:10:00.8929255Z error: failed to run custom build command for `clang-sys v1.6.1`
2023-04-26T20:10:00.8929867Z 
2023-04-26T20:10:00.8930107Z Caused by:
2023-04-26T20:10:00.8931102Z   process didn't exit successfully: `/home/runner/work/cherryrgb-rs/cherryrgb-rs/target/debug/build/clang-sys-dad454f8793fe135/build-script-build` (exit status: 101)
2023-04-26T20:10:00.8931743Z   --- stdout
2023-04-26T20:10:00.8933133Z   cargo:warning=could not execute `llvm-config` one or more times, if the LLVM_CONFIG_PATH environment variable is set to a full path to valid `llvm-config` executable it will be used to try to find an instance of `libclang` on your system: "couldn't execute `llvm-config --prefix` (path=llvm-config) (error: No such file or directory (os error 2))"
2023-04-26T20:10:00.8933853Z 
2023-04-26T20:10:00.8934329Z   --- stderr
2023-04-26T20:10:00.8951960Z   thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "couldn't find any valid shared libraries matching: ['libclang.so', 'libclang-*.so'], set the `LIBCLANG_PATH` environment variable to a path where one of these files can be found (invalid: [])"', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clang-sys-1.6.1/build/dynamic.rs:206:45
2023-04-26T20:10:00.8954113Z   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2023-04-26T20:10:00.8954661Z warning: build failed, waiting for other jobs to finish...
2023-04-26T20:10:01.1370052Z ##[error]Process completed with exit code 101.
2023-04-26T20:10:01.1466111Z Post job cleanup.

This new dependency is introduced indirectly by uhid-virt:

   └── uhid-virt v0.0.6
│       ├── enumflags2 v0.7.7
│       │   └── enumflags2_derive v0.7.7 (proc-macro)
│       │       ├── proc-macro2 v1.0.56 (*)
│       │       ├── quote v1.0.26 (*)
│       │       └── syn v2.0.15 (*)
│       ├── libc v0.2.142
│       └── uhidrs-sys v1.0.2
│           [build-dependencies]
│           └── bindgen v0.63.0
│               ├── bitflags v1.3.2
│               ├── cexpr v0.6.0
│               │   └── nom v7.1.3
│               │       ├── memchr v2.5.0
│               │       └── minimal-lexical v0.2.1
│               ├── clang-sys v1.6.1

felfert · 2023-04-27T06:28:08Z

Forget that:

~~Have a look here:~~

The step named "Install dependencies" apparently does the trick

clippy:
    name: Clippy
    runs-on: ubuntu-latest
    steps:
      - name: Install dependencies
        run: sudo apt-get update && sudo apt-get install -y libclang-dev clang llvm
      - uses: actions/checkout@v3
      - uses: dtolnay/rust-toolchain@stable
        with:
          components: clippy
      - name: Cargo Clippy
        run: cargo clippy -- -D warnings
      - name: Cargo Clippy Example
        run: cargo clippy --manifest-path example/Cargo.toml -- -D warnings

skraus-dev · 2023-04-27T14:51:09Z

Thanks alot for doing this major task!

Now, to get this merged, there needs to be some conditional compiling happening - so non-linux user can still build the project.

Checklist:

Conditionally compile uhid-exclusive functionality in the main lib related to target_os (target_os: linux, reference: https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg-attribute)
- module: vkbd
- structs: models.RpcAnimation
Conditionally include the dependency uhid-virt, only on the linux-platform
CI: Ensure build dependencies get installed (natively for x86_64, inside cross container for i386, arm)
CI: Ensure correct build command is used per platform (build --all for linux, plain build for the others)
CI: Ensure correct binaries and auxilary files (udev rules, service file, config file) are bundled in build artifact / release bundle

With these changes it should already work from what I can tell.

Linux: cargo build --all
Non-Linux: cargo build (it should not be attempting to build everything, aka. only builds the CLI)

felfert · 2023-04-27T15:51:08Z

Thanks alot for doing this major task!

Now, to get this merged, there needs to be some conditional compiling happening - so non-linux user can still build the project.

Checklist:
* [x]  Conditionally compile uhid-exclusive functionality in the main lib related to target_os (target_os: linux, reference: https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg-attribute)
  
  * [x]  module: vkbd
  * [x]  structs: models.RpcAnimation

* [x]  Conditionally include the dependency `uhid-virt`, only on the linux-platform

Wahh, you are too quick, I have the above already - just not yet committed to this PR's branch

* [ ]  CI: Ensure build dependencies get installed (natively for x86_64, inside cross container for i386, arm)

I tried github's official rust action instead and it works nicely. (not yet tested for i36 and arm)

* [ ]  CI: Ensure correct build command is used per platform (`build --all` for linux, plain `build` for the others)

That's what I am struggling with right now: I cannot believe, that there is NO way of disabling build of specific workspace members based on target-os or features. What a pity! (There are some Issues open in cargo's repo, but no solution exists yet. I experimented with conditionals in both service and ncli, but this is ridiculously ugly. This are 2 very weak spots in cargo and rustc - to say the least.

* [ ]  CI: Ensure correct binaries and auxilary files (udev rules, service file, config file) are bundled in build artifact / release bundle

Huh: Did you mean CD?

With these changes it should already work from what I can tell.

Linux: cargo build --all Non-Linux: cargo build (it should not be attempting to build everything, aka. only builds the CLI)

felfert · 2023-04-27T17:12:13Z

I think, triggering the CD action on any pull request is NOT a good idea, because - as far as i understand it - the
publishing to releases and cargo runs in YOUR context (using the secrets you provide in your configuration).
Instead, the matrix-build and the copying to an output directory should be moved to the CI action and the resulting
output directory then stashed for later retrieval by the CD action. Then the CD action, just retrieves the previouslly
stashed artifacts and publishes them.

felfert · 2023-04-27T20:37:56Z

* [ ]  CI: Ensure build dependencies get installed (natively for x86_64, inside cross container for i386, arm)

llvm and clang are capable of cross-compiling per se, so no problem there, but libclang-dev (and possible dependencies)
is going to be tricky, because there are no pre-made packages:

a complete cross env would have to be created
libclang-dev installed inside of that one

This is a) much bloat, b) quite time-consuming if done in the regular CI action. I will try to generate a separate action
that perfoms this and then stashes the files of the lib for retrieve by the CI action. This might take some time.

skraus-dev · 2023-04-27T21:50:12Z

@felfert no worries, the build dependency task is a tiny one, involving the cross rust toolchain

https://github.com/cross-rs/cross/wiki/Configuration

We just need to decide whether we want a seperate Cross.toml or we put the pre-build dependency installation step inside the existing Cargo.toml

I think, triggering the CD action on any pull request is NOT a good idea, because - as far as i understand it - the publishing to releases and cargo runs in YOUR context (using the secrets you provide in your configuration). Instead, the matrix-build and the copying to an output directory should be moved to the CI action and the resulting output directory then stashed for later retrieval by the CD action. Then the CD action, just retrieves the previouslly stashed artifacts and publishes them.

Kind of like that, yes.
My plan is to have the main build action.. and then I will introduce a seperate actions that uses it as "re-usable workflow" that will only run on "main" and "tags".

reference: https://docs.github.com/en/actions/using-workflows/reusing-workflows

felfert · 2023-04-28T12:32:08Z

Oh, and can we please keep CI/CD related commits out of this PR. I think, these are better collected in a separate PR.
I'm closing this, and re-submit a new PR without those.

For now, this is what I have No PR yet (still without the cross stuff, but artifacts are already stashed):
https://github.com/felfert/cherryrgb-rs/actions/runs/4830348450

Notes:

Intentionally NOT named CI, so we can have this in addition to the existing one. If everything is finalized, we can replace the original CI with the new one.
Intentionally only manually triggered, so it does not waste resources all the time.

Question:
Why do you use dtolnay/rust-toolchain@stable instead of the default that comes preinstalled on githubs native platforms?

felfert · 2023-04-28T20:40:58Z

Ok, after wasting a whole evening for testing research etc, it turns out, that libclang on ubuntu is seriously broken in
regards of multiarch installs. (cross uses ubuntu-16.04 (ancient!) as base-image). Unfortunately, cross also is
not very userfriendly in that it does not show any output of the pre-build comand in Cross.toml. So I had to
resort to manually entering the container locally and figuring out what went wrong:

Basically, the problem is similar like this:
https://askubuntu.com/questions/1446189/problem-doing-multiarch-installs-on-ubuntu
The packages of libclang for different architectures all install to the same location. The native amd64 lib is at
/usr/lib/llvm-3.8/lib/libclang-3.8.so.1 and the arm64 and i686 packages try to overwrite this - which is obviously
fatal:

Snippet from the apt-get install libclang-dev:i386
Unpacking libclang1-3.8:i386 (1:3.8-2ubuntu4) ...
dpkg: error processing archive /var/cache/apt/archives/libclang1-3.8_1%3a3.8-2ubuntu4_i386.deb (--unpack):
trying to overwrite shared '/usr/lib/llvm-3.8/lib/libclang-3.8.so.1', which is different from other instances of package libclang1-3.8:i386

Same happens with arm64

The correct path would be somewhere below /usr/lib/i386-linux-gnu/ resp. /usr/lib/arm64-linux-gnu/

So unless you know of a different container for cross-builds, I give up on that. It's simply not worth the effort,
especially since both architectures are not really widely used.

Well one last try would be manually unpacking the .deb in temporary dir and then moving it to a different location.
Given that ubuntu 16.04 is ancient, I dont even think about bug report.

felfert · 2023-04-29T08:34:44Z

So for now, I have added a featue named uhid and changed the conditional compile directives.
In order to build, both target_os must be "linux" AND feature must be "uhid"
So in the matrix CI action we use --all --features uhid on native ubuntu latest only

If the libclang mess gets sorted out for the cross platforms, we can add these flags for them later.

The new ~~CI is ready~~, (see here). Currently working on new CD. PR for both coming soon...

Cheers
-Fritz

felfert · 2023-04-29T11:56:57Z

Turns out, artifacts can only be shared in the same Action. So switching to cache. Keeping the artifact for manual checks, if eferything is in order.

felfert · 2023-04-30T06:15:52Z

Turns out, artifacts can only be shared in the same Action. So switching to cache. Keeping the artifact for manual checks, if eferything is in order.

Also, the github deployment action insists being triggered by a git tag event which makes my idea of how this should work completely useless. I must admit, I never had used github workflow/actions before. Professionally, I am used to a jenkins environment which is quite different in that perspective and I was a probably a bit naive in assuming that the github stuff works in a similar way.

So: PR coming up with fixes for the existing workflows ...

- Install build dependencies when building the UHID stuff. - Build UHID stuff on native linux only for now.

felfert added 8 commits April 25, 2023 18:13

Implement service daemon and new client

367ca25

Add support for systemd.journal in service

a778cad

Fix future-incompatibility warning in uhid-virt

bc4f175

- Change uhid-virt source temporarily to yet unreleased fix.

Restart service via udev rule, if keyboard gets plugged in

da226e0

Update uhid-virt to latest version

7e64e01

Augment LightingMode enum

f924df3

Adding strum properties to members enables more sophisticated validation of cmdline parameters.

Attempt to improve error handling

d94cdf5

Add doc, split udev rules into separate files

f8523a6

Accidently exchanged rules files

16cb746

felfert mentioned this pull request Apr 27, 2023

Add install of new dependencies in CI action #36

Closed

skraus-dev added the enhancement New feature or request label Apr 27, 2023

felfert force-pushed the uhid-service branch 2 times, most recently from 05e64d1 to 04c0eb6 Compare April 29, 2023 05:32

Conditional compile UHID

8bbe858

felfert force-pushed the uhid-service branch from 04c0eb6 to 8bbe858 Compare April 29, 2023 05:43

Add feature uhid, Inherit workspace properties

aad4a45

felfert force-pushed the uhid-service branch from c9d3c12 to aad4a45 Compare April 29, 2023 07:47

Update docs

9ee6bb8

felfert added a commit to felfert/cherryrgb-rs that referenced this pull request Apr 30, 2023

Adapt workflows to skraus-dev#35

e9f04a7

- Install build dependencies when building the UHID stuff. - Build UHID stuff on native linux only for now.

felfert mentioned this pull request Apr 30, 2023

Adapt workflows to #35 #38

Closed

1 task

Upgrade systemd-journal-logger to 1.0.0

87e7a42

skraus-dev merged commit 8da0d14 into skraus-dev:master May 1, 2023

felfert deleted the uhid-service branch May 10, 2023 06:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add uhid service and alternative cli #35

Add uhid service and alternative cli #35

felfert commented Apr 26, 2023

felfert commented Apr 26, 2023

felfert commented Apr 27, 2023

felfert commented Apr 27, 2023 •

edited

Loading

skraus-dev commented Apr 27, 2023 •

edited

Loading

felfert commented Apr 27, 2023

felfert commented Apr 27, 2023

felfert commented Apr 27, 2023

skraus-dev commented Apr 27, 2023 •

edited

Loading

felfert commented Apr 28, 2023

felfert commented Apr 28, 2023 •

edited

Loading

felfert commented Apr 29, 2023 •

edited

Loading

felfert commented Apr 29, 2023 •

edited

Loading

felfert commented Apr 30, 2023

Add uhid service and alternative cli #35

Add uhid service and alternative cli #35

Conversation

felfert commented Apr 26, 2023

Description

Type of change

Code Checklist

felfert commented Apr 26, 2023

felfert commented Apr 27, 2023

felfert commented Apr 27, 2023 • edited Loading

skraus-dev commented Apr 27, 2023 • edited Loading

felfert commented Apr 27, 2023

felfert commented Apr 27, 2023

felfert commented Apr 27, 2023

skraus-dev commented Apr 27, 2023 • edited Loading

felfert commented Apr 28, 2023

felfert commented Apr 28, 2023 • edited Loading

felfert commented Apr 29, 2023 • edited Loading

felfert commented Apr 29, 2023 • edited Loading

felfert commented Apr 30, 2023

felfert commented Apr 27, 2023 •

edited

Loading

skraus-dev commented Apr 27, 2023 •

edited

Loading

skraus-dev commented Apr 27, 2023 •

edited

Loading

felfert commented Apr 28, 2023 •

edited

Loading

felfert commented Apr 29, 2023 •

edited

Loading

felfert commented Apr 29, 2023 •

edited

Loading