Update framework dotnet + add better adaptive control - #82
Merged
Conversation
Where calibration judged the EC could hold a commanded speed, the duty demand was converted to an RPM setpoint and handed to the firmware's own loop. That loop clamps the target to its configured maximum for the channel — around the 4900 RPM this repo already records as StandardFirmwareMaximumRpm on Framework 16, where the same fan reaches past 6200 on a direct duty write. So the top quarter of the range could not be asked for. Calibration measured MaximumRpm at 100% duty (correct), then verified speed tracking at only 60% of it — comfortably under the ceiling, so it passed and declared cascade good for the whole range. At runtime full demand mapped to a speed the EC quietly refused: the fan sat ~1600 RPM short and, because the shortfall happened inside the firmware rather than at the controller's own clamp, the anti-windup never fired and the trim wound to +100% against a ceiling it had no way to see. The learner was the worse casualty. It was told CommandedDutyPercent = <the demand> while the EC had substituted its own duty to chase an RPM, so the model was being fitted against duty values that were never applied. Cascade's one advantage — the firmware holding a speed steady as a fan ages — is what the adaptive learner already does, so removing it costs nothing. Speeds are still shown. FanCalibrationSnapshot.EstimateRpm interpolates between the two speeds calibration actually measures, the minimum-spin point and full duty, instead of scaling the maximum by the duty fraction: a fan does not start turning at zero, so the old line read far too low across the whole lower half of the range. On a 20%/1200 -> 100%/6000 fan, 60% duty is 3600 RPM, not 3000. It is named ExpectedRpm because it is an estimate of an outcome, never a demand. The adaptive card's "off setpoint by N RPM" is gone with the setpoint it referred to. It now reports only a fan running SHORT of expectation — which can mean an obstruction — with a tolerance proportional to the speed being predicted, since the estimate's error scales with it. Removed end to end: FanSpeedTrackingMode, TrackingMode on the calibration snapshot and its persisted key, the worker's speed path, and the explainer's cascade/fallback verdict. Proto field 12 is reserved and setpoint_rpm becomes expected_rpm. Calibration no longer probes speed tracking, so a run is one settle shorter. Re-run calibration on each fan to capture the full-speed reading the new display is anchored to. The proto changed, so the service and the app must be rebuilt together. Tests: worker asserts Adaptive writes a duty and never a speed; calibration asserts it issues no speed command even when the simulated fan would honour one; EstimateRpm covered at both anchors, mid-range (explicitly not the duty-fraction answer), below stall, uncalibrated, and for monotonicity. 717 pass.
1 — EC diagnostics Warnings & Issues: new "Embedded controller" card. Throttle itself is behaviour, surfacing through the existing "Throttle escalation" legend row 2 — Sensors Sensor chips + Thermal page take firmware names (via the existing channel name, no new markup); warning band on the thermal chart; Adaptive target slider bounded with a caption naming the bound 3 — USB-C Negotiated contract row on each port card, with a shortfall line when the slot could take more 4 — Battery "Pack health" expander on Power Telemetry: per-cell bars, imbalance verdict, age beside cycles, what the pack is asking for, its own state of health 5 — Firmware "Component firmware" card in Device Capabilities Modules slot cards now carry a Firmware tile — matched by vendor + product id, not slot index. That was my original objection: peripheral descriptors number slots by USB enumeration order, which has no relation to the physical positions that page draws, so an index join would have put a confident wrong version under a picture of a slot. Vendor+product identifies the module type, which is sound. Drive detail falls back to the drive's own NVMe firmware when the OS reports none — an exact join on device path. WMI's value still wins when present, since that's what every other tool shows.
…s promptly Stopping the service took its entire 90-second shutdown budget whenever the app was connected. That is what put "Installer is no longer responding" in front of an uninstall and made it need retrying until the budget ran out — six retries is roughly ninety seconds of MSI patience. All fourteen Watch... calls stream forever by design, and every one waited only on context.CancellationToken. ASP.NET Core fires that token when the client disconnects, or when Kestrel ABORTS the request — and it only aborts once graceful shutdown has already timed out. So on stop the host sat waiting the full budget for streams that were never going to end on their own, then killed them. The 90 seconds was chosen as headroom for restoring fans to Auto on a contended EC; it was being spent every time instead of never. Each stream now runs on a token linked to IHostApplicationLifetime's ApplicationStopping as well, via ServerCallContext.LinkToShutdown, and unwinds the moment shutdown is signalled. The linked source is disposed at every call site — otherwise each completed call would leave a registration on the lifetime token for the life of the process. The OperationCanceledException filters moved to the linked token too. A shutdown-triggered cancellation does not set the request's own token, so the previous `when (context.CancellationToken.IsCancellationRequested)` would have let it through as an unhandled exception. Covers WatchStatus, WatchServiceConfiguration, HoldFanPreview, the nine telemetry streams and both hardware-info streams. Stopping, restarting and upgrading the service benefit as much as uninstalling does.
Three places where raw library values reached the screen unprocessed.
Sensor names. Publishing FirmwareName verbatim put "apu_f75303@4d" and
"gpu_vram_f75303@4d" down the Dashboard's thermal snapshot. Firmware
reports wiring, not labels: that is the APU sensor, on an F75303 chip, at
I2C address 0x4d. The chip and the address are true and useless — they
name the part doing the measuring, not the thing measured — and they ate
the whole label column. FriendlyFirmwareName reduces a name to its
subject: drop everything from '@', drop any segment carrying a digit (no
sensor SUBJECT is spelled with one, so that separates part numbers from
words without a list of chips to maintain), drop a trailing "temp" since
every sensor here measures temperature, then render acronyms upper case.
"ambient_f75303@4d" becomes Ambient, "gpu_temp@40" becomes GPU. A name
that reduces to nothing falls through to the mapped name rather than
rendering blank.
Retimer version. I hand-joined the raw four-byte register payload and got
"17.0.1.0.17.128.4.0.1.0.0.0..." — every byte of a padded buffer in
decimal. The library ships VersionString for exactly this. Using a
formatter it already provides instead of writing my own.
Power-delivery controllers. MainFirmware.ToString() spells out the
bootloader base, the application, and the application's target: "Base:
3.7.0.407, App: 0.0.34 (Notebook)". Four facts in a column with room for
one. Now the application version alone, which is what Framework's own
firmware notes quote.
Their names were the EC's probe order — Right01 / Left23 / Back — which is
worse than terse: that port numbering appears nowhere a user can see it,
and it is off by one against the Power page's own USB-C 1..4 labels. Now
"Right side (USB-C 1 & 2)" and so on, with an unrecognised slot keeping
whatever the firmware called it; a wrong friendly name is worse than a
terse true one. The retimer joins that group rather than getting a heading
of its own, which had put "Retimer" over a single row reading "Retimer".
An unnamed component counts within its group ("USB hub") instead of
echoing "Slot 0" — a USB enumeration index that corresponds to nothing on
the machine.
Pack health reads itself when the section is expanded, and repeat reads
come from pulling it down. Opening the section IS the request, so a button
that repeats what the user just asked for was a second ask for the same
thing. RefreshContainer holds its deferral until the read completes, so
the visualizer spins for the real duration; the content is capped in a
ScrollViewer because RefreshContainer needs something to pull against.
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
1 participant
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.
No description provided.