Repository navigation
feat: add hwmon fan profiles and API v0.2 - #54
sknowledge1 wants to merge 9 commits into
Conversation
8c74ffe to
7264e1f
Compare
|
Please rebase on latest main |
|
This can't be merged or reviewed as is. The history is all messed up. I'd suggest you delete your local branch, check out an pull main, remake the branch, cherry pick your commits, and then force push |
58fcde8 to
9b82591
Compare
|
Rebuilt from 35 locked tests and 26 lifecycle assertions pass, along with formatting, Clippy using the existing upstream allowances, both release builds, and both distribution targets. Historical hardware results remain attributed to their tested build. Ready for another look. |
Keep CustomCurve separate from the selected preset curve and notify clients when the saved curve changes. Include the real PowerStation activation service in RPM/archive assets. Cover saved-curve preservation over preset/suspend transitions and wait for asynchronous state persistence in the fake lifecycle harness. Validated without device control: 36 locked Rust tests, formatting, Clippy, both release architectures, private D-Bus and 26 lifecycle assertions. Official image and physical thermal acceptance remain pending. Assisted-by: Codex:GPT-6
pastaq
left a comment
pastaq
left a comment
There was a problem hiding this comment.
I got through most of the readme file before I had to stop. I don't think this is the correct direction for the interface.
I think it should be much more simple. For curves, a yaml or toml that identifies the fan and its sensors, and a simple curve matrix of quant temp to %. hwmon internals don't need relaying, it should handle that transition directly. An average user should be able to read, write, and understand the configuration directly. We should have some hardware pre-built curves in /usr/powerstation/platform/fan_curves and allow for distros to ship overrides in /etc/powerstation/platform/fan_curves.d/. userspace should load a profile on boot if a user has customized it.
For the dbus, loading the profile or accepting one over JSON, listing the current profile name/path, listing available fans and sensors, and listing available/current modes (automatic, manual, fw curve, sw curve) should be sufficient.
Critical safety temps (100% fan, hw takeover) should be baked into the algorithm and not left up to the user.
Avoid logind property caching so an unrelated GetAll failure cannot prevent the fan monitor from starting. Request InhibitDelayMaxUSec with its exact D-Bus spelling and reproduce the failed bulk query in the private lifecycle fixture. Validated with 36 Rust tests, Clippy, the 28-assertion lifecycle harness, and an attended AYANEO 3 8840U startup in firmware Automatic. The previous binary fails the new fixture. Official image and thermal acceptance remain pending. Assisted-by: Codex:GPT-6
Separate editable profiles from trusted hardware and internal safety policy. Start every new service process in firmware Automatic while preserving only the live profile across protected suspend/resume. Validate complete selections before writes and roll all affected fans back on application or resume failure. Replace the v1 D-Bus controls with profile loading, fan and sensor enumeration, mode reporting and valid percentages. Update packaging, CI, bindings, documentation and failure-path regressions together.
sknowledge1
changed the title
|
Implemented the profile design from your overall review in cb75669. YAML/JSON profiles now contain fan/sensor IDs, modes and percentages; packaged Quiet and whole-file distro overrides are separate from trusted driver configuration. API v0.2 adds profile loading, fan/sensor enumeration and current profile/mode reporting, with valid 0–100% values and no public RPM/PWM/ownership/policy properties. The revised proposal and short examples are documented. Fresh starts, clean/crash/watchdog restarts and reboots enter firmware Automatic; user applications own profile persistence/reloading. Successful suspend/resume keeps only the live selection. Complete validation precedes hardware writes, and application or partial-resume failure rolls every affected fan back to Automatic. The provisional 95°C/98°C policy is internal, with thermal acceptance explicitly pending. The push-only workflow and custom-only generated fan docs address the inline review. I have replied in each original thread and left all threads unresolved. Actual local validation on For the earlier rebase request and history request, current upstream main remains The successful uninterrupted five-minute, 9 W, 352-sample, 64°C-peak device result is preserved as evidence for GitHub CI was inspected on this exact SHA at 2026-10-15 09:04 UTC: no runs, check runs or statuses were present in either repository. All validation claims above are from the completed local software gate. |
Record the first systemd invocation before service exposure so clients can guard normal-boot restoration without replaying after native recovery. Keep fan API v2 and thermal policy unchanged. Implemented with Codex assistance; 48 Rust tests, Clippy, 65 lifecycle assertions and both architecture builds/packages pass. Physical acceptance remains separate.
Create and sync an exclusive attempt directory before opening the first record. Later invocations require the original record and cannot promote themselves after file creation fails. Preserve valid in-boot evidence and add real-systemd regression coverage. Assisted-by: Codex
|
@pastaq, the follow-up is implemented at fffb4b9. Unprivileged clients can read startup identity/eligibility without accessing recorder files; the startup file/JSON load methods recheck it under the complete profile transaction lock, closing the status-to-write race. Existing profile validation, caller policy, partial-write Automatic fallback and 95°C/98°C policy remain service-owned. Local validation on that commit passes 52 locked tests, 77 real-systemd/private-D-Bus/fake-sysfs assertions, formatting, strict Clippy with the existing allowances, both release architectures and both archive/RPM payload checks. The actual OpenGamepadUI API v0.2 client additionally passes 32 Godot/native integration assertions with fake hardware. New-head GitHub inspection reports no native test check contexts; these are local results. The base/history requests are also rechecked: current main is Physical results remain pinned to 255e680 (352 samples / 64°C) and 1b7330e (342 samples / 65.5°C). Revised-code/device acceptance, the provisional high-temperature policy, native-curve hardware coverage and official testing/stable/ARM package qualification remain open release gates. Firmware handoff transfers responsibility; it does not prove stronger cooling. This is ready for another review. |
This is not necessary. As the API hasn't been accepted there is no v1/v2 yet. We're on v0 which in semantic versioning means there is no stable API. |
fair - I'll change it to API v0.2 |
sknowledge1
changed the title
pastaq
left a comment
pastaq
left a comment
There was a problem hiding this comment.
I stopped my review.
Please read this:
https://www.eff.org/about/opportunities/volunteer/coding-with-eff
This PR is over 6k lines and half of it is slop readme. I should not be the first person to read "your" PR.
| on: [push] | ||
|
|
||
| jobs: | ||
| fan-lifecycle: |
There was a problem hiding this comment.
This shouldn't be necessary. All tests need to run from the existing test declaration.
| <arg name="file" type="s" direction="in" /> | ||
| </method> | ||
| <!-- | ||
| Version 2 replaces the preset/custom-curve API with complete profiles. |
There was a problem hiding this comment.
There is no established API yet. Please don't indicate an API break.
| </interface> | ||
| <interface name="org.shadowblip.Fan"> | ||
| <!-- | ||
| Load a complete YAML profile by filename in the approved profile directories. A same-name distro file replaces the packaged file. Validate every fan before writing; any application failure returns all affected fans to Automatic. |
There was a problem hiding this comment.
The path needs to be arbitrary so userspace can save/load profiles.
Userspace should open the file and send the open fd. For security the fd should be checked for type/validity. As this is a root level daemon we need to prevent arbitrary code injection into the daemon.
| <arg name="file" type="s" direction="in" /> | ||
| </method> | ||
| <!-- | ||
| Load the same profile structure as JSON, without saving it. An all-automatic profile works during lifecycle restrictions and without telemetry. Clients own saving and reloading selections after service startup. |
There was a problem hiding this comment.
Load the same profile structure as JSON, without saving it.
Each description should stand on its own, not reference other defs.:
Load a profile as a JSON string.
An all-automatic profile works during lifecycle restrictions and without telemetry.
I don't see how this applies here?
Clients own saving and reloading selections after service startup.
This can read as as a broken sentence. Please make it more clear:
Clients must handle saving and reloading the profile after service startup.
| <arg name="json" type="s" direction="in" /> | ||
| </method> | ||
| <!-- | ||
| Return the fan object paths for this platform. Unsupported systems return an empty list. |
There was a problem hiding this comment.
Return the fan object paths for this platform. Unsupported systems return an empty list.
Return all discovered fans for this device.
| This database is separate from editable profiles and the TDP TOML database. | ||
| Profiles cannot supply paths, mode register values, or critical thresholds. | ||
|
|
||
| DMI alternatives are ORed; every populated field within an alternative must |
There was a problem hiding this comment.
We'll need to support device tree as well as DMI.
|
|
||
| The packaged systemd unit runs `powerstation --record-fan-startup` in mandatory ExecStartPre before the daemon. RuntimeDirectory `powerstation-fan-startup` has mode 0700 and RuntimeDirectoryPreserve=yes, preserving evidence through stop/start, crash and watchdog recovery until reboot. The helper writes no hardware and does not restore a profile. | ||
|
|
||
| `/run/powerstation-fan-startup/first.json` is a private, single-link regular file with mode 0600 owned by the service user (root in the packaged system unit). Its version-1 JSON contains exactly `version`, `boot_id` (kernel boot UUID) and `invocation_id` (32 lowercase hexadecimal characters from systemd INVOCATION_ID). The first creation is exclusive. Later invocations validate but never replace that first identity. |
|
|
||
| `/run/powerstation-fan-startup/first.json` is a private, single-link regular file with mode 0600 owned by the service user (root in the packaged system unit). Its version-1 JSON contains exactly `version`, `boot_id` (kernel boot UUID) and `invocation_id` (32 lowercase hexadecimal characters from systemd INVOCATION_ID). The first creation is exclusive. Later invocations validate but never replace that first identity. | ||
|
|
||
| Missing directories, symlinks, insecure ownership/modes, stale boot identity, malformed/partial/oversized records and unsupported versions fail the mandatory pre-start helper. The file is preserved. The daemon cannot accept requests from an unrecorded managed invocation; existing ExecStopPost Automatic cleanup remains active. A partial first record intentionally blocks later startup rather than misidentifying a restart as a normal boot. Reboot clears this runtime-only evidence; clients must not delete or repair it automatically. |
|
|
||
| Before opening the first record for creation, the helper exclusively creates and syncs root-owned 0700 `first.json.attempt`. An existing attempt requires a valid existing record; a missing record is a failed attempt and is never created later. This preserves the failure fence even when record-file creation itself fails. Existing valid first records remain unchanged during an in-boot package upgrade. | ||
|
|
||
| A restoring client must validate the root-owned record, compare both boot and current systemd InvocationID, bind the service's unique bus owner Unix PID to MainPID, and confirm fresh healthy telemetry, no fault and no active profile. Consume the client-side boot attempt durably before sending; uncertainty, a changed owner or invalid evidence forbids automatic replay. This record is evidence. Profile validation and native Automatic recovery remain authoritative. |
There was a problem hiding this comment.
A lot of this seems unnecessary. Please simplify everything after this. README docs need to be human readable. I also think the locking/privileged mechanism you describe is excessive. Just use a FCFS model.
| cargo generate-rpm --target aarch64-unknown-linux-gnu | ||
| ``` | ||
|
|
||
| The two Clippy allowances concern existing upstream code. Inspect each archive |
There was a problem hiding this comment.
Did you even read this? WTF
|
Appreciate the feedback - still new to this, not sure what to add / what to keep when reviewing the code. |
2 participants
PowerStation gains validated hwmon fan profiles over API v0.2, with firmware Automatic as the startup and recovery state. Profiles use stable fan/sensor IDs and complete manual or temperature/duty configurations; hardware matching, safety sensors and thermal limits remain service-owned. Addresses #29.
Profiles and API v0.2
LoadProfilereads packaged YAML profiles, includingquiet.yaml, with complete-file administrator overrides.LoadProfileJsonaccepts the same structure.EnumerateFans,EnumerateSensors, current profile identity and typed device/sensor properties support discovery. Modes areautomatic,manual,software_curveandfirmware_curveaccording to actual capabilities. Validate every selected fan before writing and return all affected fans to Automatic on partial application/resume failure.API v0.2 is the proposed, unstable fan API;
ApiVersionreports the integer 2 onorg.shadowblip.Fan. Controls use 0–100% duty;PercentValidand sensorTelemetryValididentify unavailable readings. The former preset API, public RPM/raw PWM and client ownership/controller-policy properties are removed. This revises the earlier unmerged fan API proposal. The separate Decky API v0.2 migration and private acceptance are complete. OpenGamepadUI #550 now supplies the separate native UI client and requires the guarded startup contract below.Startup and recovery
Every service process starts Automatic and ignores legacy saved fan state. Suspend/resume retains only the active profile in memory. Required safety telemetry, bounded logind inhibitors, owner monitoring, watchdog recovery and Automatic cleanup remain authoritative. An Automatic profile is usable without telemetry and cancels custom restoration during sleep.
The packaged unit now creates a root-owned 0700 runtime directory, preserves it through stops/restarts and runs mandatory
--record-fan-startupbefore accepting requests. An exclusive, synced 0700 attempt directory fences record creation first. Its 0600first.jsonrecords boot ID and the first systemd invocation ID; a missing record after a failed creation can never be created by a later invocation. Later starts validate and preserve it; partial/corrupt evidence blocks startup rather than being replaced. The helper writes no hardware. See startup record and client requirements.Unprivileged clients use read-only
GetStartupStatusfor the boot ID, current invocation ID and eligibility; private recorder files keep their existing permissions.LoadStartupProfileandLoadStartupProfileJsonrecheck identity, untouched healthy Automatic state and lifecycle protection under the existing whole-profile transaction/controller locks. Another selection, a fault, stale status or a later managed invocation is rejected before writing. These additive methods use the existing caller policy, profile validation and rollback; the proposal remains API v0.2.A restoring application pins the unique bus owner, compares the kernel boot identity and durably consumes its own boot attempt before reading preferences or sending a startup load. Privileged clients may retain direct private-record/PID checks. The native guard is authoritative across clients; saved preferences confer no ownership. Service faults/restarts never qualify automatic replay. The packaged service/assets are required together.
The provisional 95°C full-speed / 98°C firmware-handoff policy is unchanged and still requires thermal acceptance. Firmware handoff is not evidence of better cooling. Another writer using the same driver mode cannot be detected; stop competing fan daemons. Failed/blocked firmware writes can prevent timely cleanup.
Validation and gates
Exact native candidate:
fffb4b93979dd2622cd93cfa097baacc27b7a2e6, Rust 1.88.0. Local checks pass 52 locked Rust tests, formatting, strict Clippy with only the existinguninlined_format_args/nonminimal_boolallowances, 77 real-systemd/private-D-Bus/fake-sysfs lifecycle assertions, and locked x86_64/aarch64 builds. Both architectures' archive/RPM checks pass exact nine-file payload, bytes, permissions, root ownership and ELF/RPM architecture. Cross-generated ARM runtime-library dependency/install qualification remains outstanding.The software fixture covers preserved first identity through clean/crash/watchdog restarts, corrupt evidence rejection, failed creation without promoting a later invocation, read-only startup status and stale startup-load rejection. The actual Godot client also passes 32 assertions against this native implementation using private D-Bus/fake sysfs: saved JSON and file restoration, owner-pinned explicit loads, bounded input and Automatic cleanup. All counts are local software results; new-head GitHub reads report no test check contexts.
Physical results below belong to earlier native heads. No device deployment or physical validation of this startup-contract follow-up is claimed. The uninterrupted five-minute, 9 W, 352-sample / 64°C result remains at 255e680 (API v1), separately from the later API v0.2 evidence.
The final
1b7330eprivate runtime is physically checked on an AYANEO 3 8840U, kernel7.2.9-ogc3.1.fc44.x86_64, with its running binary hash verified. Automatic, Quiet, Manual 60% and default Custom pass attended operation. Decky restart preserves a live profile; native restart/SIGKILL return to Automatic without replay. A service-namespace sensor fixture verifies invalidation, rejected custom control, latched-fault recovery through explicit Automatic and preserved preferences. No real sensor value is modified. Lost-reply handling issues one request. A real normal boot restores saved Manual 60% once; a separate real boot with native crash before the first Decky startup preserves the original invocation and refuses saved Quiet. Actual suspend/resume preserves the live profile.A five-minute, 16-thread CPU-only Quiet observation at the existing 9 W power setting records 342 samples and a 65.5°C peak, with unchanged power properties and native Automatic cleanup. Exact rollback verifies 72 original files/ownership/modes and the original API v1 binary before the final runtime is restored. Full operating-range/95°C/98°C thermal policy, native hardware curves, revised-code device acceptance and clean official testing/stable packages remain release gates. HX 370 compatibility remains assumed; only 8840U physical testing is claimed. The bounded private result does not qualify those conditions. No qualified release assets are published here.
The existing push-only workflow trigger is retained.
serde_jsonpromotes the already-locked version; dependency versions do not change. PR stays open/non-draft; existing review-thread states are preserved. GitHub checks will be recorded separately from local results.Unqualified local candidate SHA256 hashes
16351bc09a835ed9e8c40827a9f8a05887a5982959b619058cd091c546a8d80184ba31496b4e1d059e693523bb792ed0cb872d8f057b15272beedce21196874d077de26938e4bed86a33de62d6eb365b5ab3dabadf1bbba0b31b40a0251148d6e7f9134d7da16bf0f6b93025eccc6b1321ec7378b3c1c3b0fdfd840c7439d00fAssisted-by: Codex