Skip to content
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

feat(tuxedo_sysfs): implement charging profiles #85

Merged

Conversation

liketechnik
Copy link
Contributor

@liketechnik liketechnik commented May 1, 2024

This is my first draft for implementing the sysfs abstraction for the charging profiles discussed in #80.

The TODO notes in the diff mark stuff where I'm unsure which model fits best. Please make a suggestion on the approach you prefer.

Note that so far this implements only the read-part of the abstraction.
For the write-part: How 'thick/thin' is the sysfs abstraction supposed to be? E.g., when writing the charging thresholds, should the method itself check that constraints are met (i.e. charge_type == "Custom", value between either 0-100 or in the available_start_threholds list).

I have tested that this works with the tailor_hwcaps binary, although my hardware (/ drivers) only supports the charging profile (i.e. both the charge thresholds and the charging priority are correctly reported as "not available").

Copy link
Owner

@AaronErhardt AaronErhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, code looks very good overall. Sorry for the delay, I've been quite busy lately.

Comment on lines 4 to 5
// TODO: split ChargingProfile into Profile + Priority?
// this would make the double Option<> thing for available + file less awkward to work with...
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this sounds good and should also solve the open questions in charging_profile.rs

@liketechnik liketechnik force-pushed the feat/sysfs-charging-profiles branch from 3fdab50 to 9d41b95 Compare June 22, 2024 20:45
@liketechnik
Copy link
Contributor Author

No worries regarding and delays, that's just how it is :) (and you're not alone with that, it unfortunately took me quite some time to get back to this too)

I've finally gotten around to implement the aforementioned split between charging profile & priority. Additionally, I've added simple writers for all writable properties.

I believe that should be it for the sysfs abstraction part. Would you prefer the integration into the rest of the application to happen in a different PR or to keep this PR as a draft until the options are available in the UI?

Copy link
Owner

@AaronErhardt AaronErhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good! I think it's better to implement the GUI changes in another PR. So if you want, I can already merge this.

@liketechnik liketechnik marked this pull request as ready for review June 23, 2024 11:56
@liketechnik
Copy link
Contributor Author

I agree that it's better to implement the GUI part in another PR, so let's merge this 🚀

@AaronErhardt AaronErhardt merged commit 801683a into AaronErhardt:main Jun 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants