--thermal: Return data instead of only printing it - #369
Open
blackdragoon26 wants to merge 1 commit into
Open
Conversation
print_thermal and print_thermal_thresholds read the EC, decoded the result and formatted it all in one function, so nothing but the commandline tool could use them. Split the reading and decoding into get_thermal and get_thermal_thresholds, which return the data, and leave the print functions as thin formatters on top. Their output is unchanged. The decoded values get proper types instead of raw memmap bytes: TempSensor is public now, fan speeds decode into FanSpeed instead of comparing against magic values at print time and disabled thresholds are None instead of a "-" string. print_thermal used to panic when the memmap read failed, now it prints an error instead. Add tests for the decoding, power.rs had none.
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.
Towards #357.
print_thermalandprint_thermal_thresholdsdid the EC reads, the decoding and the formatting in one function, so nothing but the commandline tool could get at the data.This splits them in two:
get_thermal(&CrosEc) -> EcResult<ThermalInfo>print_thermal(&CrosEc)get_thermal_thresholds(&CrosEc) -> Option<Vec<ThermalThresholds>>print_thermal_thresholds(&CrosEc) -> Option<()>The print functions keep their signatures and their output, they just call the getter and format what comes back.
The decoded values get types instead of raw memmap bytes:
TempSensorispubnow. It was private, so nothing outsidepower.rscould name a temperature reading even where one was already being returned.FanSpeed::{Rpm, Stalled, NotPresent}instead of comparing against0xFFFE/0xFFFFat print time.Nonerather than the string"-".Option<ApThrottleInfo>with real bools.So a consumer can now do:
One intentional behaviour change
print_thermalused.unwrap()on both memmap reads, so a failed read panicked. It now printsFailed to read thermal information: ...and returns.Verified
cargo fmt --all -- --checkis clean, and so iscargo clippy -p framework_lib -- -D warningscargo checkpasses for Linux, Windows, FreeBSD and UEFI (--features uefi)cargo docwithRUSTDOCFLAGS=-Dwarningsis cleancargo test -p framework_lib: 45 passed, 0 failedpower.rshad none before.Two things I could not check locally, both because I develop on macOS, which this project doesn't target: clippy on
framework_tool(itslibusb1-sysdependency needs a Linux C toolchain to cross-compile) and the EC reads themselves, since no machine here is a Framework one. CI covers the first. For the second, worth running--thermaland--thermalgetonce on real hardware before merging.Scope
I stopped at thermals because you said that file was the messiest and @mysticmalard mentioned it was where they were headed next. The same treatment still applies to
print_sensors,print_switches,get_and_print_power_info,get_and_print_pd_info,get_and_print_cypd_pd_infoand theprint_fw{12,13,16}_inputdeck_statusmethods.Happy to keep going if this shape looks right to you.