Skip to content

Commit a84b684

Browse files
authored
fix: daemon install, TOML integer config, float write verification (#13)
Root-cause fixes for four issues found on real machines, each with tests. - #9: accept TOML integers for Double config fields (incl. fan-curve weights); surface config parse errors in `daemon status` instead of silently resetting the whole config to defaults. - #11: tolerate SMC float quantization in F0Tg write verification (no more per-cycle false-positive Error spam); non-float types stay byte-exact. - #8/#12: resolve smctld via _NSGetExecutablePath instead of argv[0], keeping the brew-upgrade-safe symlink path. Fixes #8 Fixes #9 Fixes #11 Fixes #12
1 parent e6f5da5 commit a84b684

8 files changed

Lines changed: 349 additions & 21 deletions

File tree

Package.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ let package = Package(
7171
.testTarget(
7272
name: "SMCtlDaemonCoreTests",
7373
dependencies: ["SMCtlDaemonCore"]
74+
),
75+
.testTarget(
76+
name: "smctlTests",
77+
dependencies: ["smctl"]
7478
)
7579
]
7680
)

Sources/SMCCore/SMCBackend.swift

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public extension SMCWriteBackend {
8282
for verifyAttempt in 1...retryPolicy.verifyReads {
8383
let readBack = try readValue(key)
8484
actual = readBack.bytes
85-
if Array(actual.prefix(bytes.count)) == bytes {
85+
if smcWriteVerified(expected: bytes, actual: actual, dataType: info.dataType) {
8686
return
8787
}
8888
if verifyAttempt < retryPolicy.verifyReads {
@@ -103,3 +103,31 @@ public extension SMCWriteBackend {
103103
throw lastError ?? SMCError.writeVerificationFailed(key: key, expected: bytes, actual: [])
104104
}
105105
}
106+
107+
/// Whether a written value read back as the value we wrote.
108+
///
109+
/// SMC float (`flt`) writes are quantized: the firmware clears low mantissa bits,
110+
/// so a read-back of an interpolated fan target differs from the written bytes by
111+
/// a fraction of an RPM. An exact byte compare therefore reports a write failure
112+
/// on every policy cycle (issue #11), flooding the log and masking real failures.
113+
/// For `flt` keys we compare the decoded floats within a tolerance; every other
114+
/// type must still match exactly so a genuinely failed write is never hidden.
115+
func smcWriteVerified(expected: [UInt8], actual: [UInt8], dataType: UInt32) -> Bool {
116+
let actualPrefix = Array(actual.prefix(expected.count))
117+
if actualPrefix == expected {
118+
return true
119+
}
120+
let types = FourCharCode.normalizedStrings(dataType)
121+
guard types.contains("flt ") || types.contains("flt") else {
122+
return false
123+
}
124+
guard
125+
let written = (try? SMCDataDecoder.decode(key: "", bytes: expected, dataType: dataType))?.doubleValue,
126+
let readBack = (try? SMCDataDecoder.decode(key: "", bytes: actualPrefix, dataType: dataType))?.doubleValue
127+
else {
128+
return false
129+
}
130+
// Quantization is well under 1 RPM; the relative term keeps headroom for large
131+
// values without ever approaching the gap a genuinely failed write would show.
132+
return abs(written - readBack) <= max(1.0, abs(written) * 0.001)
133+
}

Sources/SMCtlDaemonCore/Alerting.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ struct AlertConfig: Codable, Equatable, Sendable {
5757
name = try container.decodeIfPresent(String.self, forKey: .name) ?? ""
5858
on = try container.decodeIfPresent(String.self, forKey: .on) ?? ""
5959
sensor = try container.decodeIfPresent(String.self, forKey: .sensor)
60-
above = try container.decodeIfPresent(Double.self, forKey: .above)
61-
forSeconds = try container.decodeIfPresent(Double.self, forKey: .forSeconds)
62-
cooldown = try container.decodeIfPresent(Double.self, forKey: .cooldown)
60+
above = try container.decodeLenientDoubleIfPresent(forKey: .above)
61+
forSeconds = try container.decodeLenientDoubleIfPresent(forKey: .forSeconds)
62+
cooldown = try container.decodeLenientDoubleIfPresent(forKey: .cooldown)
6363
resolve = try container.decodeIfPresent(Bool.self, forKey: .resolve)
6464
action = try container.decodeIfPresent(String.self, forKey: .action)
6565
url = try container.decodeIfPresent(String.self, forKey: .url)

Sources/SMCtlDaemonCore/Daemon.swift

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ struct SentryConfig: Codable, Equatable, Sendable {
9292
dsn = try container.decodeIfPresent(String.self, forKey: .dsn) ?? ""
9393
environment = try container.decodeIfPresent(String.self, forKey: .environment) ?? "production"
9494
debug = try container.decodeIfPresent(Bool.self, forKey: .debug) ?? false
95-
let sampleRate = try container.decodeIfPresent(Double.self, forKey: .traces_sample_rate) ?? 0
95+
let sampleRate = try container.decodeLenientDoubleIfPresent(forKey: .traces_sample_rate) ?? 0
9696
traces_sample_rate = min(max(sampleRate, 0), 1)
9797
}
9898
}
@@ -166,6 +166,22 @@ struct FanCurveConfig: Codable, Equatable, Sendable {
166166
self.slew_rate = slew_rate
167167
self.weights = weights
168168
}
169+
170+
enum CodingKeys: String, CodingKey {
171+
case name, sensors, points, hysteresis, slew_rate, weights
172+
}
173+
174+
// Custom decoding so `hysteresis`/`slew_rate` are optional (default rather than
175+
// keyNotFound) and accept TOML integers — see decodeLenientDoubleIfPresent (#9).
176+
init(from decoder: Decoder) throws {
177+
let container = try decoder.container(keyedBy: CodingKeys.self)
178+
name = try container.decode(String.self, forKey: .name)
179+
sensors = try container.decodeIfPresent([String].self, forKey: .sensors) ?? []
180+
points = try container.decode([[FanPointValue]].self, forKey: .points)
181+
hysteresis = try container.decodeLenientDoubleIfPresent(forKey: .hysteresis) ?? 0
182+
slew_rate = try container.decodeLenientDoubleIfPresent(forKey: .slew_rate)
183+
weights = try container.decodeLenientDoubleDictionaryIfPresent(forKey: .weights)
184+
}
169185
}
170186

171187
enum FanPointValue: Codable, Equatable, Sendable {
@@ -178,6 +194,13 @@ enum FanPointValue: Codable, Equatable, Sendable {
178194
self = .number(number)
179195
return
180196
}
197+
// TOML keeps integers and floats as distinct types, so `[50, "max"]` decodes
198+
// the 50 as Int, not Double. Accept it as a number instead of falling through
199+
// to the String branch and rejecting the whole curve.
200+
if let integer = try? container.decode(Int.self) {
201+
self = .number(Double(integer))
202+
return
203+
}
181204
let string = try container.decode(String.self).trimmingCharacters(in: .whitespacesAndNewlines).lowercased()
182205
if string == "max" {
183206
self = .maximum
@@ -197,6 +220,42 @@ enum FanPointValue: Codable, Equatable, Sendable {
197220
}
198221
}
199222

223+
/// TOML keeps integers and floats as distinct types. A hand-written `100` for a
224+
/// Double config field would otherwise throw a type mismatch and — via loadConfig's
225+
/// catch — silently reset the entire config to defaults (issue #9). Accept integers
226+
/// as Doubles; a genuinely wrong type still surfaces as a decoding error.
227+
extension KeyedDecodingContainer {
228+
func decodeLenientDoubleIfPresent(forKey key: Key) throws -> Double? {
229+
guard contains(key) else { return nil }
230+
if let value = try? decode(Double.self, forKey: key) { return value }
231+
if let integer = try? decode(Int.self, forKey: key) { return Double(integer) }
232+
return try decode(Double.self, forKey: key)
233+
}
234+
235+
/// Same integer leniency for a `[String: Double]` field (fan curve weights):
236+
/// `weights = { cpu = 2 }` decodes the values as Int and would otherwise throw,
237+
/// discarding the whole config.
238+
func decodeLenientDoubleDictionaryIfPresent(forKey key: Key) throws -> [String: Double]? {
239+
guard contains(key) else { return nil }
240+
if let value = try? decode([String: Double].self, forKey: key) { return value }
241+
if let integers = try? decode([String: Int].self, forKey: key) { return integers.mapValues(Double.init) }
242+
// Mixed integer/float entries: decode value-by-value (a wrong type still throws).
243+
let nested = try nestedContainer(keyedBy: LenientDictKey.self, forKey: key)
244+
var result: [String: Double] = [:]
245+
for entry in nested.allKeys {
246+
result[entry.stringValue] = try nested.decodeLenientDoubleIfPresent(forKey: entry)
247+
}
248+
return result
249+
}
250+
}
251+
252+
private struct LenientDictKey: CodingKey {
253+
var stringValue: String
254+
var intValue: Int?
255+
init?(stringValue: String) { self.stringValue = stringValue }
256+
init?(intValue: Int) { self.stringValue = String(intValue); self.intValue = intValue }
257+
}
258+
200259
struct SafetyConfig: Codable, Equatable, Sendable {
201260
var temp_ceiling: Double
202261
/// Advanced opt-in: allows `fan set --force` to target below the fan's reported
@@ -211,7 +270,7 @@ struct SafetyConfig: Codable, Equatable, Sendable {
211270
// Defensive decoding — see BatteryConfig.
212271
init(from decoder: Decoder) throws {
213272
let container = try decoder.container(keyedBy: CodingKeys.self)
214-
temp_ceiling = try container.decodeIfPresent(Double.self, forKey: .temp_ceiling)
273+
temp_ceiling = try container.decodeLenientDoubleIfPresent(forKey: .temp_ceiling)
215274
?? FanSafetyGuard.defaultCeilingCelsius
216275
allow_below_minimum = try container.decodeIfPresent(Bool.self, forKey: .allow_below_minimum) ?? false
217276
}
@@ -256,6 +315,10 @@ public final class SmctlDaemon: @unchecked Sendable {
256315
private static let alertHistoryLimit = 50
257316
private var lastEvaluation: Date?
258317
private var lastError: String?
318+
/// Set when the on-disk config fails to parse. The daemon keeps running on
319+
/// defaults, so without this the failure was invisible — `daemon status` showed
320+
/// `last error: -` while silently ignoring the user's config (issue #9).
321+
private var configError: String?
259322
private var lastWriteError: String?
260323
private var timer: DispatchSourceTimer?
261324
private var fanTimer: DispatchSourceTimer?
@@ -376,7 +439,9 @@ public final class SmctlDaemon: @unchecked Sendable {
376439
periodSeconds: SmctlDaemon.period,
377440
configPath: configPath,
378441
lastEvaluation: lastEvaluation,
379-
lastError: lastError
442+
// A config parse failure is more actionable than a transient runtime
443+
// error and persists until the user fixes the file, so it wins.
444+
lastError: configError ?? lastError
380445
)
381446
}
382447
}
@@ -411,7 +476,9 @@ public final class SmctlDaemon: @unchecked Sendable {
411476
func reloadConfig() {
412477
queue.sync {
413478
let wasForcing = config.battery.force_discharge
414-
config = Self.loadConfig(path: configPath)
479+
let loaded = Self.loadConfig(path: configPath)
480+
config = loaded.config
481+
configError = loaded.error
415482
SentryReporter.startIfConfigured(config: config.sentry)
416483
let policy = (try? SleepPolicy.parse(config.battery.sleep_policy)) ?? .strict
417484
sleepMachine.setPolicy(policy)
@@ -1112,16 +1179,16 @@ public final class SmctlDaemon: @unchecked Sendable {
11121179
return "unknown"
11131180
}
11141181

1115-
private static func loadConfig(path: String) -> DaemonConfig {
1182+
private static func loadConfig(path: String) -> (config: DaemonConfig, error: String?) {
11161183
guard FileManager.default.fileExists(atPath: path) else {
1117-
return DaemonConfig()
1184+
return (DaemonConfig(), nil)
11181185
}
11191186
do {
11201187
let text = try String(contentsOfFile: path, encoding: .utf8)
1121-
return try TOMLDecoder().decode(DaemonConfig.self, from: text)
1188+
return (try TOMLDecoder().decode(DaemonConfig.self, from: text), nil)
11221189
} catch {
11231190
logger.error("Unable to parse \(path, privacy: .public): \(String(describing: error), privacy: .public)")
1124-
return DaemonConfig()
1191+
return (DaemonConfig(), "config at \(path) failed to parse, running on defaults: \(String(describing: error))")
11251192
}
11261193
}
11271194

Sources/smctl/main.swift

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,29 @@ private enum CLIFormatters {
1818
static let iso8601 = LockedISO8601Formatter()
1919
}
2020

21+
// MARK: - Daemon install path resolution (#8)
22+
23+
/// Real path of the running executable, independent of how it was launched. Under a
24+
/// PATH lookup argv[0] is just "smctl", so the old argv[0]-based resolution fell back
25+
/// to the working directory and `daemon install` could not find smctld (#8).
26+
func currentExecutablePath() -> String? {
27+
var size: UInt32 = 0
28+
_ = _NSGetExecutablePath(nil, &size) // first call reports the required buffer size
29+
guard size > 0 else { return nil }
30+
var buffer = [CChar](repeating: 0, count: Int(size))
31+
guard _NSGetExecutablePath(&buffer, &size) == 0 else { return nil }
32+
return String(cString: buffer)
33+
}
34+
35+
/// smctld sits next to smctl. Pure and testable. The path is deliberately left
36+
/// un-resolved — the stable /opt/homebrew/bin symlink, not the versioned Cellar
37+
/// target — so the LaunchDaemon keeps pointing at a valid binary across `brew upgrade`.
38+
func smctldPath(besideExecutable executablePath: String, fileExists: (String) -> Bool) -> String? {
39+
let directory = URL(fileURLWithPath: executablePath).deletingLastPathComponent()
40+
let candidate = directory.appendingPathComponent("smctld").standardizedFileURL.path
41+
return fileExists(candidate) ? candidate : nil
42+
}
43+
2144
@main
2245
struct SMCtl: ParsableCommand {
2346
static let configuration = CommandConfiguration(
@@ -727,16 +750,16 @@ struct DaemonInstall: ParsableCommand {
727750
static let plistPath = "/Library/LaunchDaemons/one.leaper.smctl.daemon.plist"
728751

729752
private static func currentSmctldPath() throws -> String {
730-
let rawCommand = CommandLine.arguments[0]
731-
let command = rawCommand.hasPrefix("/")
732-
? URL(fileURLWithPath: rawCommand)
733-
: URL(fileURLWithPath: FileManager.default.currentDirectoryPath).appendingPathComponent(rawCommand)
734-
let directory = command.deletingLastPathComponent()
735-
let candidate = directory.appendingPathComponent("smctld").standardizedFileURL.path
736-
if FileManager.default.isExecutableFile(atPath: candidate) {
737-
return candidate
753+
guard
754+
let executable = currentExecutablePath(),
755+
let path = smctldPath(
756+
besideExecutable: executable,
757+
fileExists: { FileManager.default.isExecutableFile(atPath: $0) }
758+
)
759+
else {
760+
throw ValidationError("Could not find smctld next to the current smctl executable.")
738761
}
739-
throw ValidationError("Could not find smctld next to the current smctl executable.")
762+
return path
740763
}
741764

742765
private static func plist(smctldPath: String) -> String {

Tests/SMCCoreTests/SMCWriteRetryTests.swift

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,80 @@ final class SMCWriteRetryTests: XCTestCase {
8383
}
8484
}
8585
}
86+
87+
// MARK: - Float write verification tolerance (issue #11)
88+
89+
func testFloatWriteVerificationToleratesSMCQuantization() {
90+
// Real sample: wrote 3829.5 RPM, the SMC read back 3829.125 (low mantissa
91+
// bits cleared). Bytes differ; the values are 0.375 RPM apart.
92+
let flt = FourCharCode.unchecked("flt ")
93+
let written: [UInt8] = [0x00, 0x56, 0x6f, 0x45] // 3829.5
94+
let readBack: [UInt8] = [0x00, 0x50, 0x6f, 0x45] // 3829.125
95+
XCTAssertNotEqual(written, readBack)
96+
XCTAssertTrue(smcWriteVerified(expected: written, actual: readBack, dataType: flt))
97+
}
98+
99+
func testFloatWriteVerificationStillRejectsRealMismatch() {
100+
// A write that never landed (read-back still 0) must NOT be hidden by tolerance.
101+
let flt = FourCharCode.unchecked("flt ")
102+
let written = FanController.float32LittleEndianBytes(3000)
103+
let stale = FanController.float32LittleEndianBytes(0)
104+
XCTAssertFalse(smcWriteVerified(expected: written, actual: stale, dataType: flt))
105+
}
106+
107+
func testNonFloatWriteVerificationRequiresExactBytes() {
108+
// Tolerance must apply only to floats; integer/mode keys stay byte-exact.
109+
let ui8 = FourCharCode.unchecked("ui8 ")
110+
XCTAssertTrue(smcWriteVerified(expected: [0x02], actual: [0x02], dataType: ui8))
111+
XCTAssertFalse(smcWriteVerified(expected: [0x02], actual: [0x03], dataType: ui8))
112+
}
113+
114+
func testWriteKeyVerifiesQuantizedFloatWithoutRewriting() throws {
115+
let backend = QuantizingFloatBackend(initialBytes: FanController.float32LittleEndianBytes(0))
116+
try backend.writeKey(
117+
"F0Tg",
118+
bytes: FanController.float32LittleEndianBytes(3829.5),
119+
retryPolicy: SMCWriteRetryPolicy(
120+
maxAttempts: 3,
121+
initialBackoffNanoseconds: 0,
122+
verifyReads: 2,
123+
verifyIntervalNanoseconds: 0
124+
)
125+
)
126+
XCTAssertEqual(backend.rawWriteAttempts, 1, "a quantized read-back must verify on the first write, no rewrite")
127+
}
128+
}
129+
130+
/// Models SMC float quantization: the firmware stores a written float with its low
131+
/// mantissa bits cleared, so the read-back never byte-matches an interpolated fan
132+
/// target (issue #11).
133+
private final class QuantizingFloatBackend: SMCWriteBackend {
134+
private(set) var currentBytes: [UInt8]
135+
private(set) var rawWriteAttempts = 0
136+
137+
init(initialBytes: [UInt8]) {
138+
currentBytes = initialBytes
139+
}
140+
141+
func readKeyInfo(_ key: String) throws -> SMCKeyInfo {
142+
SMCKeyInfo(dataSize: 4, dataType: FourCharCode.unchecked("flt "), dataAttributes: 0xd0)
143+
}
144+
145+
func readValue(_ key: String) throws -> SMCReadValue {
146+
SMCReadValue(key: key, info: try readKeyInfo(key), bytes: currentBytes)
147+
}
148+
149+
func writeRawValue(_ key: String, bytes: [UInt8]) throws {
150+
rawWriteAttempts += 1
151+
var bits = UInt32(bytes[0]) | UInt32(bytes[1]) << 8 | UInt32(bytes[2]) << 16 | UInt32(bytes[3]) << 24
152+
bits &= 0xFFFFF000 // clear the low 12 mantissa bits, as the SMC does
153+
currentBytes = [
154+
UInt8(bits & 0xff),
155+
UInt8((bits >> 8) & 0xff),
156+
UInt8((bits >> 16) & 0xff),
157+
UInt8((bits >> 24) & 0xff)
158+
]
159+
}
86160
}
87161

88162
private final class MockWriteBackend: SMCWriteBackend {

0 commit comments

Comments
 (0)