Conversation
FanProfile.loadAll() reads custom profiles from ~/Library/Application Support/ThermalForge/profiles, but nothing called it. The menu bar picker and `watch` both used FanProfile.builtIn, so a saved custom profile was never offered anywhere. Issue ProducerGuy#28 reported this; the install self-deletion half was fixed, this half wasn't. - AppState publishes availableProfiles, read once at launch. The picker is rebuilt on every status update, so it must not hit the filesystem. - Launch restore resolves the saved id against that list via selectable(id:from:), so a custom profile comes back instead of falling back to Silent. - `watch` accepts any custom profile id, and lists what's available on error. It also now accepts `smart`, which it rejected before because Smart isn't in builtIn -- a small CLI addition rather than a side effect. - loadAll(from:) and save(to:) take a directory. Tests used to write into the real Application Support directory and delete afterwards, which would destroy a user's own balanced.json when running the suite. - Running with elevated privileges makes the effective user root, whose home is /var/root, so the profiles directory resolves through SUDO_UID (as `install` already does) and cross-checks the resolved account. Without it the root CLI looks in the wrong home and finds nothing. Profiles loaded from disk are screened before being offered, the same way CalibrationData is screened on load, since hand-editing the JSON is the only way to make one today and nothing else validated it. Unreadable and undecodable files are logged rather than vanishing. `silent` and `smart` are reserved because the code branches on both by id: a custom smart.json would have most of its curve ignored by Smart's adaptive path, and a custom silent.json would leave the picker showing one profile while the monitor ran another. Ordering: loadAll appended, so a profile quieter than Balanced landed after Max and read as the most aggressive entry. Sorting by maxRPMPercent ascending reproduces the built-in order exactly (0, 0.60, 0.85, 1.0), so anyone with no custom profiles sees the list unchanged -- there's a test asserting that. No change to the curve maths, the tick loop, or the daemon. Curve.targetPercent is untouched, and the daemon speaks only RPM, so the 95C override and the heartbeat watchdog cover custom profiles exactly as they cover the built-ins.
Covers the directory, the JSON shape, what makes a curve valid, and that the daemon safety override applies to custom profiles unchanged. The example overrides the built-in Balanced rather than inventing a new profile name.
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.
What
FanProfile.loadAll()reads custom profiles from~/Library/Application Support/ThermalForge/profiles, but nothing calls it.MenuBarViewandwatchboth useFanProfile.builtIn, so a saved custom profile never appears. Second half of #28; the install self-deletion half is already fixed on main.Behaviour change
None on a machine with no custom profile files, except one thing.
Curve.targetPercentis byte-identical to main. Tick loop, ramp governors and daemon untouched. Ordering sorts bymaxRPMPercentascending, reproducing the built-in order (0, 0.60, 0.85, 1.0); a test assertsorderedByCeiling(builtIn) == builtIn.The exception:
watch --profile smartused to throw aValidationErrorbecause Smart isn't inbuiltIn. It now works.Changes
AppStatepublishesavailableProfiles, read once at launch. The picker rebuilds on every status update, so it can't touch the filesystem.loadAll(from:)andsave(to:)take a directory. The existing tests wrote into the real Application Support directory and deleted afterwards, destroying a user's ownbalanced.json.SUDO_UIDwhen running as root, asinstallalready does, cross-checked against the uid. Without it the root CLI reads/var/rootand finds nothing.Curve.validationError, the same wayCalibrationDatais screened on load. Hand-editing JSON is the only way to make one today and nothing validated it. Unreadable files are logged instead of disappearing.silentandsmartare reserved ids because the code branches on both. A customsmart.jsonwould have most of its curve ignored by Smart's adaptive path; a customsilent.jsonwould show one profile in the picker while the monitor ran another.Safety
The daemon speaks only RPM and knows nothing about profiles, so the 95°C override and heartbeat watchdog cover custom profiles the same as built-ins. Validation rejects curves that engage above the safety threshold or set
alwaysOn, and enforces the 5°C hysteresis rule so a hand-written file can't cycle the fans on sensor noise.Overlap with open PRs
#44 replaces this
PickerwithButtonrows. The two changes are orthogonal: #44 changes the control, this changes what the rows are built from. Whichever lands first, the other is a small rebase, and I'm happy to do that rebase rather than make you resolve it.Two notes
#46 proposes a settings UI covering custom curves, per-profile enable/disable and moving Smart inline, and someone has offered to prototype it. This PR doesn't prejudge that; it surfaces the JSON
loadAll()already reads. If #46 lands, the ordering here is the first thing to redo.The README sells against tools that require manual configuration, and this adds a section on hand-writing JSON.
loadAll(),save()and the profiles directory are already yours; this only makes them reachable. Drop the README commit if you disagree, the code stands without it.Testing
61 tests,
swift buildandswift testclean. Every filesystem test uses a UUID scratch directory undertemporaryDirectory, so the suite no longer touches the real profiles directory.