Skip to content

core: resetAuto reports rejected SMC writes and never commands a stopped manual fan - #72

Open
iBobbyTS wants to merge 1 commit into
ProducerGuy:mainfrom
iBobbyTS:fix/resetauto-honest
Open

iBobbyTS wants to merge 1 commit into
ProducerGuy:mainfrom
iBobbyTS:fix/resetauto-honest

Conversation

@iBobbyTS

Copy link
Copy Markdown

Problem

FanControl.resetAuto() discarded every writeKey result and logged "Reset to Apple defaults" unconditionally. When a write is rejected — the common case being a non-root caller, where AppleSMC allows reads but the firmware rejects fan-mode writes — the fans stay latched in manual mode while every caller believes the reset succeeded. Two callers are directly affected today: the SIGTERM shutdown path logs "released fans to auto" from this call's result, and the watchdog / .auto paths rely on the throw to tell the client the truth.

This is the same diagnosis as #30 — that PR is the right direction and this builds on it.

Fix

Testing

swift build + full swift test pass (152 tests). The change is in the SMC write path itself, which (like #30) has no hardware-free seam to unit-test; behavior is covered by review of the write sequence.

…ped manual fan

resetAuto() discarded every writeKey result and logged "Reset to Apple
defaults" unconditionally, so a rejected mode write — e.g. the firmware
refusing fan-mode writes — left fans latched in manual while callers
believed the reset succeeded: the SIGTERM shutdown path logs "released
fans to auto" from this very result, and the watchdog / `.auto` paths
rely on the throw to tell the client the truth.

Every fan is still attempted before throwing, so one uncooperative key
cannot strand the remaining fans in manual mode (supersedes the mode/Ftst
part of ProducerGuy#30). Beyond ProducerGuy#30, fans whose AUTO-mode write failed stay in
manual mode, and writing their target to 0 anyway would command a manual
0 RPM — a stopped, unsupervised fan. They keep their last (spinning)
target instead, and the throw makes the caller retry the whole reset.
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.

1 participant