Skip to content

fix(fans): surface SMC write failures to user instead of silent fail#3190

Draft
marxo126 wants to merge 2 commits intoexelban:masterfrom
marxo126:fix/silent-fan-write-failure
Draft

fix(fans): surface SMC write failures to user instead of silent fail#3190
marxo126 wants to merge 2 commits intoexelban:masterfrom
marxo126:fix/silent-fan-write-failure

Conversation

@marxo126
Copy link
Copy Markdown

@marxo126 marxo126 commented May 7, 2026

Problem

SMC/smc.swift exposes setFanSpeed, setFanMode, and resetFans as Void. On any failure path — IOKit error, missing key on a chip generation, firmware reject — the function print()s and returns silently. The caller in Modules/Sensors/popup.swift has no way to know the SMC write failed, so the slider/mode UI stays in the wrong state and the user gets no feedback.

This is the visible symptom in #1166 ("Unable to set fan speed") and a contributing factor to several other fan reports where users report "fan control didn't take effect".

Change

  • SMC/smc.swiftsetFanSpeed, setFanMode, resetFans now return @discardableResult Bool. Every error path returns false; success returns true. resetFanControl already returns Bool and is unchanged.
  • SMC/Helper/main.swift — the existing (String?) -> Void completion now receives a non-nil error string on failure (was nil for both success and failure, which made errors indistinguishable). Error format: "Failed to set fan <id> speed to <rpm> RPM — SMC rejected write".
  • Kit/helpers.swiftSMCHelper setters gain an optional ((String?) -> Void)? completion (default nil preserves all existing call sites). Forwards XPC error string through.
  • Modules/Sensors/popup.swift — added showFanError(_:) private helper on FanView that dispatches an NSAlert ("Fan control failed", .warning style) to the main queue. All 8 interactive call sites — init restore, mode buttons (auto/manual/off/turbo), setSpeed debouncer, wakeListener — pass a completion that calls showFanError on a non-nil error. sleepListener intentionally stays fire-and-forget (the user can't act on a sleep-time error).

Public API note

Returning Bool from previously-Void functions is a public API change inside the SMC layer. Marked @discardableResult to avoid noise at existing call sites that don't care about the result. If you'd prefer a non-API-changing approach (separate tryFanSpeed etc), happy to refactor.

Net diff

4 files changed, +39 lines net

Testing

Logic-reviewed but not yet hardware-tested. Marking Draft. The XPC completion fires on an internal XPC queue per Apple's docs, so the alert dispatches via DispatchQueue.main.async before NSAlert.runModal() — please double-check that pattern matches your house style for the popup layer.

marxo126 added 2 commits May 7, 2026 10:54
setFanSpeed, setFanMode, and resetFans previously returned Void and only
print()ed on failure, leaving the popup UI in an inconsistent state and
giving users no indication when SMC writes were rejected (firmware
reject, missing key on a chip generation, etc).

Return Bool from these SMC entry points, propagate failure through the
SMCHelper completion handlers, and surface a NSAlert in the Sensors
popup so users get actionable feedback instead of a silently broken
slider.

Closes exelban#1166
… surface

The previous SMCHelper wiring passed completion only through the proxy's
reply block. When the XPC connection itself failed (helper not
bootstrapped, helper crashed), the reply was never delivered and the
completion was silently dropped — defeating the user-facing alert.

Per-call helper now sets remoteObjectProxyWithErrorHandler to invoke the
caller's completion with a typed error string, so XPC failures take the
same surfacing path as helper-side rejects.

Verified by booting out the helper service and confirming the alert
displays.
@marxo126
Copy link
Copy Markdown
Author

marxo126 commented May 7, 2026

Update — runtime verified end-to-end on macOS 26.4.1, Apple Silicon (M4 Max).

Test setup:

  1. `launchctl bootout system /Library/LaunchDaemons/eu.exelban.Stats.SMC.Helper.plist` — torn down the helper to force XPC connection failure on the next fan write.
  2. Launched dev build, opened popup, clicked Manual on a fan.

Result: NSAlert pops with the expected "Fan control failed" + propagated error string ("Fan helper unreachable: Couldn't communicate with a helper application"). Confirmed via popup screenshot.

One real bug surfaced during testing — the original wiring passed the completion only through the proxy's reply block, so XPC connection failures (helper bootstrapped-off, helper crashed) silently dropped the completion and the alert never fired. Fix is in commit `2bb88f02`: per-call helper now sets `remoteObjectProxyWithErrorHandler` to invoke the caller's completion with a typed error string, so XPC failures take the same surfacing path as helper-side rejects. The fix is what makes this PR actually deliver its promised behaviour.

Caveat unchanged: tested on a (necessarily) unsigned dev build with a matching unsigned helper to bypass the SMAuthorizedClients certificate check. Behaviour in a Developer ID-signed production build is identical — the signature check is independent of the failure-surfacing path.

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