-
Notifications
You must be signed in to change notification settings - Fork 10
Update dependency on sysfs_gpio and fix clippy warnings #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There are still several dependencies that could be updated or replaced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much!
Do you know of any users that would need the MSRV to be kept at 1.75?
Otherwise, given that this crate is so stale (we even have to port the CI to GHA), I would be fine with raising it even higher if that eases the breakage potential here, since pinning transitive dependencies is already necessary with this PR.
The latest Yocto Long-Term-Support release Scarthgap ships with Rust 1.75.0. This is the environment where at least I'm dealing with sysfs GPIOs. And yes, having a CI set up for this crate would be great. I already thought about it and now I'm on the verge letting myself getting nerdnsniped into actually doing it. |
Alright. I checked this on a Raspberry Pi 3 with Debian 11 Bullseye (as sysfs GPIO support is no longer available/active with Debian 12 Bookworm). There are still some rough edges which don't look like being related to the changes made here:
I tried to remodel CLI parsing when transitioning from Clap 2 to Clap 4 without causing breakage. However, I did not manage to invoke the tool in its current state 02b0658 due to constantly getting errors from CLI parsing and having no clue how to actually invoke $ ./target/debug/gpio --config examples/raspberrypi.toml export-all --symlink-root /tmp/gpio
error: Found argument '--symlink-root' which wasn't expected, or isn't valid in this context
Did you mean to put '--symlink-root' after the subcommand 'export'?
USAGE:
gpio --config <config>... <SUBCOMMAND>
For more information try --help I also added some quick and dirty CI builds and link checks. Link checks fail due to the current state. But I would include them nevertheless to increase the pressure on getting them fixed. But with the rough edges but aside, things are ready to merge from my side. |
Created #33 to tackle the actually broken links. |
Transitioning from the error-chain crate to thiserror gives me exactly the same error message output in this example:
|
As mentioned on Matrix, this is because the old version required
With this PR, both variants work. There are some rough edges as you mentioned, and I agree they are not caused by the update. For everything I tried, the old and the new version reported the same errors. So I didn't find any regression, and if you are holding it right, the tool does what it should. Tested on a Raspberry Pi 3, with Raspberry Pi OS based on bookworm, but kernel downgraded to 6.1.0 to get the old sysfs gpio interface back. That said, I don't have much experience with this tool, so all I actually did was exporting and unexporting the GPIO pins, manually toggling one of them, and verifying that the voltage on the pin actually changes. |
Nowaday's Rust requires way less verbosity here and allows to specify the macros to import into scope explicitly.
This inlcludes updating some formatting and pinning backtrace to the latest version supporting this Rust version.
This requires pinning some transitive dependencies whose MSRV got bumped beyond 1.75.0.
And let the value name more prominently hint at that a filename is expected here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.