General brightness improvements: new brightnessctl backend, set by percentage, and pick device
#60
Conversation
This file contains 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
|
Hi there, @ErikReider ! Can I get some feedback on this one? I'll be happy to make any adjustments! |
ErikReider
requested changes
Nov 22, 2023
ErikReider
requested changes
Dec 8, 2023
src/brightness_backend/mod.rs
Outdated
Comment on lines
1
to
37
| use self::{brightnessctl::BrightnessCtl, blight::Blight}; | ||
|
|
||
| mod blight; | ||
|
|
||
| mod brightnessctl; | ||
|
|
||
| pub type BrightnessBackendResult = anyhow::Result<Box<dyn BrightnessBackend>>; | ||
|
|
||
| pub trait BrightnessBackendConstructor: BrightnessBackend + Sized + 'static { | ||
| fn try_new(device_name: Option<String>) -> anyhow::Result<Self>; | ||
|
|
||
| fn try_new_boxed(device_name: Option<String>) -> BrightnessBackendResult { | ||
| let backend = Self::try_new(device_name); | ||
| match backend { | ||
| Ok(backend) => Ok(Box::new(backend)), | ||
| Err(e) => Err(e), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub trait BrightnessBackend { | ||
| fn get_current(&mut self) -> u32; | ||
| fn get_max(&mut self) -> u32; | ||
|
|
||
| fn lower(&mut self, by: u32) -> anyhow::Result<()>; | ||
| fn raise(&mut self, by: u32) -> anyhow::Result<()>; | ||
| fn set(&mut self, val: u32) -> anyhow::Result<()>; | ||
| } | ||
|
|
||
| pub fn get_preferred_backend(device_name: Option<String>) -> BrightnessBackendResult { | ||
| println!("Trying BrightnessCtl Backend..."); | ||
| BrightnessCtl::try_new_boxed(device_name.clone()).or_else(|_| { | ||
| println!("...Command failed! Falling back to Blight"); | ||
| Blight::try_new_boxed(device_name) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Make sure to run cargo fmt --all
There was a problem hiding this comment.
Addressed in ba06c46
|
@ErikReider I've addressed the review comments! |
chore: remove debug statements
partially related to ErikReider#37
partially related to ErikReider#27
Syndelis
force-pushed
the
feat/brightnessctl-backend
branch
from
ba06c46 to
f7751ec
Compare
ErikReider
approved these changes
Dec 9, 2023
3 tasks
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.
Resolves #56
Resolves #37
Partially Resolves #27
About this PR
This PR separates the code that controls brightness into a new module. This module exports a trait that different backends can implement in order to control brightness like SwayOSD expects.
Besides the existing "blight" backend, a new backend has been introduced, one that is backed by the command line util
brightnessctl(#56). This backend is attempted first and "blight" serves as a fallback if the command is not available. I've made it this way because "blight" requires some setting up, whilebrightnessctlrequires only the command to be installed.The brightness command also accepts
brightness={val}to set the brightness directly in percentages (#37).Finally, the PR also allows for the user to pick a different device to have its brightness set, via the already present
--deviceparameter. I believe this partially resolves #27 because the user could now make a script to call SwayOSD for each device, which was previously impossible.