feat(fancontrol): Implement fan control system#1954
Open
ruaan-deysel wants to merge 5 commits intounraid:mainfrom
Open
feat(fancontrol): Implement fan control system#1954ruaan-deysel wants to merge 5 commits intounraid:mainfrom
ruaan-deysel wants to merge 5 commits intounraid:mainfrom
Conversation
7 tasks
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a fan control subsystem: hwmon and IPMI controller providers, fan curve interpolation and periodic control loop, safety management and restoration, GraphQL models/resolver/mutations/subscription, persistent configuration, tests, and a dev config version bump with removed plugin entry. Changes
Sequence Diagram(s)sequenceDiagram
participant FanCurve as FanCurveService
participant Temp as TemperatureService
participant Controller as Hwmon/IPMI
participant Safety as FanSafetyService
rect rgba(200,200,255,0.5)
FanCurve->>Temp: getMetrics()
Temp-->>FanCurve: temperatures
end
rect rgba(200,255,200,0.5)
FanCurve->>Controller: readAll()
Controller-->>FanCurve: RawFanReadings
end
rect rgba(255,230,200,0.5)
FanCurve->>FanCurve: interpolateSpeed(curve, temp) -> target %
FanCurve->>Safety: validatePwmValue / captureState
Safety-->>FanCurve: validated PWM / state ack
FanCurve->>Controller: setMode / setPwm
Controller-->>FanCurve: ack
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 538bc27632
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
| } | ||
|
|
||
| this.configService.replaceConfig(updated); |
There was a problem hiding this comment.
This mutation only persists the config and returns success, but nothing in this commit ever invokes FanCurveService.start()/stop(). As a result, enabling fan control or changing fan-control settings has no runtime effect on automatic curves, so users can save settings that never get applied to hardware.
Useful? React with 👍 / 👎.
Comment on lines
+81
to
+84
| const readings = await this.hwmonService.readAll(); | ||
| for (const reading of readings) { | ||
| if (reading.hasPwmControl) { | ||
| try { |
There was a problem hiding this comment.
In emergency mode, the code immediately forces all PWM fans to manual/full speed without recording their previous state. restoreAllFans() only restores entries in originalStates, so fans that were not previously captured via normal control paths cannot be restored after an emergency and can remain stuck in forced manual mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new fan control/monitoring subsystem to the Unraid API Metrics GraphQL surface, including polling + subscription publishing, sysfs hwmon reading, and initial mutation endpoints for PWM/mode changes.
Changes:
- Introduces
FanControlModulewith hwmon + IPMI providers, config persistence, safety helpers, and curve interpolation scaffolding. - Extends
Metrics/MetricsResolverto exposefanControlmetrics and asystemMetricsFanControlsubscription (via newFAN_METRICSpubsub channel). - Updates unit/integration tests to wire the new services into existing metrics/temperature test harnesses.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/unraid-shared/src/pubsub/graphql.pubsub.ts | Adds FAN_METRICS pubsub channel enum value for GraphQL subscriptions. |
| api/src/unraid-api/graph/resolvers/metrics/temperature/temperature.resolver.integration.spec.ts | Updates test DI graph to include fan control service/config providers. |
| api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts | Registers fan polling + publishes systemMetricsFanControl; adds fanControl field + subscription. |
| api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts | Updates unit tests for new resolver dependencies and topic count. |
| api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts | Updates integration test wiring for new resolver dependencies. |
| api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts | Imports FanControlModule into MetricsModule. |
| api/src/unraid-api/graph/resolvers/metrics/metrics.model.ts | Adds fanControl field to the Metrics GraphQL type. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.module.ts | Registers fan control services (config, safety, curve, controllers, resolver). |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts | Aggregates fan readings from available providers and builds summary metrics (with caching). |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.ts | Unit tests for provider selection, PWM percentage math, detection flags, caching, and mapping helpers. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts | Adds mutations for setFanSpeed, setFanMode, restoreAllFans, and config update. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.model.ts | Defines GraphQL models/enums for fan metrics, fans, profiles, and curve inputs. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts | Defines mutation input types for speed/mode/profile assignment. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.service.ts | Adds persistent config service for fancontrol.json with defaults. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts | Defines config object/input types (safety/profile/zone types). |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts | Adds safety clamping, state capture/restore, and emergency full-speed helper. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.spec.ts | Unit tests for safety clamping, emergency mode behavior, capture/restore. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts | Adds curve interpolation and interval-based curve application scaffolding. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.ts | Unit tests for curve interpolation behavior. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/controller.interface.ts | Defines provider interface + helper mappings for pwm enable/mode. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.ts | Implements sysfs hwmon device discovery, reading, and sysfs PWM writes. |
| api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts | Implements IPMI sensor reading (and stubs for control via raw commands). |
| api/dev/configs/api.json | Updates dev config version and plugin list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new Error(`Fan ${input.fanId} does not support PWM control`); | ||
| } | ||
|
|
||
| const currentMode = pwmEnableToControlMode(fan.pwmEnable); |
There was a problem hiding this comment.
SetFanModeInput uses the FanControlMode enum (which includes OFF), but FanSafetyService.validateModeTransition() always rejects OFF, making this an API option that can never succeed. Consider removing OFF from the mutation input (separate enum) or allowing a safe OFF transition (and documenting/implementing the expected hwmon behavior).
Suggested change
| const currentMode = pwmEnableToControlMode(fan.pwmEnable); | |
| const currentMode = pwmEnableToControlMode(fan.pwmEnable); | |
| if (input.mode === FanControlMode.OFF) { | |
| throw new Error( | |
| 'Turning fans fully off is not supported by the safety system. ' + | |
| 'Please use MANUAL, AUTOMATIC, or FIXED modes instead.' | |
| ); | |
| } |
| @@ -0,0 +1,46 @@ | |||
| import { Field, Float, InputType, Int } from '@nestjs/graphql'; | |||
There was a problem hiding this comment.
Unused import: Float is imported from @nestjs/graphql but not referenced in this file. This will typically fail linting (no-unused-vars) and should be removed.
Suggested change
| import { Field, Float, InputType, Int } from '@nestjs/graphql'; | |
| import { Field, InputType, Int } from '@nestjs/graphql'; |
Comment on lines
+13
to
+15
| @Field(() => Int, { description: 'PWM value (0-255)' }) | ||
| @IsNumber() | ||
| pwmValue!: number; |
There was a problem hiding this comment.
pwmValue is documented as 0-255, but the input type doesn’t enforce that range (or integer-ness). Since this value directly drives hardware writes (after safety clamping), it would be better to validate at the API boundary (e.g., @Min(0), @Max(255), @IsInt) so callers get a clear validation error instead of silent clamping.
| .map((f) => f.name); | ||
|
|
||
| return { | ||
| totalFans: fans.length, |
There was a problem hiding this comment.
FanControlSummary.totalFans is described in the GraphQL schema as “Total number of fans detected”, but here it’s set to fans.length (which includes detected=false / disconnected fans). Either change this to detectedFans.length or update the schema/field description to reflect the actual meaning.
Suggested change
| totalFans: fans.length, | |
| totalFans: detectedFans.length, |
Comment on lines
+45
to
+57
| validatePwmValue(value: number): number { | ||
| const config = this.configService.getConfig(); | ||
| const minPercent = config.safety?.min_speed_percent ?? 20; | ||
| const minPwm = Math.round((minPercent / 100) * 255); | ||
| return Math.max(minPwm, Math.min(255, Math.round(value))); | ||
| } | ||
|
|
||
| validateCpuFanPwm(value: number): number { | ||
| const config = this.configService.getConfig(); | ||
| const minPercent = config.safety?.cpu_min_speed_percent ?? 30; | ||
| const minPwm = Math.round((minPercent / 100) * 255); | ||
| return Math.max(minPwm, Math.min(255, Math.round(value))); | ||
| } |
There was a problem hiding this comment.
The config includes safety settings like max_temp_before_full and fan_failure_threshold, but this service never reads/uses them, so they currently have no effect. Given the PR description calls out critical-temp overrides and fan-failure detection, consider either implementing those checks (e.g., trigger emergencyFullSpeed() / mark failing fans when RPM drops to 0 with PWM > 0) or removing the unused config options to avoid a false sense of safety.
Comment on lines
+14
to
+23
| @Injectable() | ||
| export class FanSafetyService { | ||
| private readonly logger = new Logger(FanSafetyService.name); | ||
| private originalStates = new Map<string, OriginalState>(); | ||
| private isEmergencyMode = false; | ||
|
|
||
| constructor( | ||
| private readonly hwmonService: HwmonService, | ||
| private readonly configService: FanControlConfigService | ||
| ) {} |
There was a problem hiding this comment.
FanSafetyService captures original PWM/mode state, but nothing calls restoreAllFans() automatically on shutdown/crash (no OnModuleDestroy hook or similar). This means a manual PWM change could persist after the API stops, which conflicts with the “fallback to hardware defaults on service failure” behavior described in the PR. Consider restoring captured states in an onModuleDestroy/shutdown handler (best-effort) to reduce operational risk.
| if (this.isApplyingCurves) { | ||
| return; | ||
| } | ||
| await this.applyCurves(); |
There was a problem hiding this comment.
The setInterval callback is async and awaits applyCurves() without handling rejections. If applyCurves() throws, this will create an unhandled promise rejection (and can crash the process depending on Node/unhandledRejection settings). Wrap the call in a try/catch (or .catch(...)) and log the error so the interval keeps running safely.
Suggested change
| await this.applyCurves(); | |
| try { | |
| await this.applyCurves(); | |
| } catch (error) { | |
| this.logger.error( | |
| 'Error applying fan curves in interval loop', | |
| error | |
| ); | |
| } |
Comment on lines
+106
to
+111
| const metrics = Object.assign(new FanControlMetrics(), { | ||
| id: 'fanControl', | ||
| fans, | ||
| profiles: [], | ||
| summary, | ||
| }); |
There was a problem hiding this comment.
profiles is always returned as an empty list, and there’s no wiring that applies/activates profiles or starts FanCurveService. This doesn’t match the PR description (profiles + automatic curves + related mutations), and it makes the profiles/activeProfile fields misleading for API consumers. Either implement the profile/curve plumbing or trim the schema/description to what’s actually supported in this PR.
|
|
||
| @ObjectType({ implements: () => Node }) | ||
| export class FanControlMetrics extends Node { | ||
| @Field(() => [Fan], { description: 'All detected fans' }) |
There was a problem hiding this comment.
FanControlMetrics.fans is described as “All detected fans”, but FanControlService includes entries with detected=false (RPM 0) in the list. Either filter out undetected fans before returning, or adjust this field description so clients don’t assume every returned fan is physically present.
Suggested change
| @Field(() => [Fan], { description: 'All detected fans' }) | |
| @Field(() => [Fan], { | |
| description: | |
| 'All fans reported by the fan control service, including entries that may be undetected or have zero RPM', | |
| }) |
Comment on lines
+40
to
+95
| @ObjectType() | ||
| export class FanProfileConfig { | ||
| @Field(() => String, { nullable: true }) | ||
| @IsString() | ||
| @IsOptional() | ||
| description?: string; | ||
|
|
||
| @Field(() => [FanCurvePointConfig]) | ||
| @ValidateNested({ each: true }) | ||
| @Type(() => FanCurvePointConfig) | ||
| curve!: FanCurvePointConfig[]; | ||
| } | ||
|
|
||
| @ObjectType() | ||
| export class FanZoneConfig { | ||
| @Field(() => [String]) | ||
| @IsString({ each: true }) | ||
| fans!: string[]; | ||
|
|
||
| @Field(() => String) | ||
| @IsString() | ||
| sensor!: string; | ||
|
|
||
| @Field(() => String) | ||
| @IsString() | ||
| profile!: string; | ||
| } | ||
|
|
||
| @ObjectType() | ||
| export class FanControlConfig { | ||
| @Field({ nullable: true }) | ||
| @IsBoolean() | ||
| @IsOptional() | ||
| enabled?: boolean; | ||
|
|
||
| @Field({ nullable: true }) | ||
| @IsBoolean() | ||
| @IsOptional() | ||
| control_enabled?: boolean; | ||
|
|
||
| @Field(() => Int, { nullable: true }) | ||
| @IsNumber() | ||
| @IsOptional() | ||
| polling_interval?: number; | ||
|
|
||
| @Field(() => String, { nullable: true }) | ||
| @IsString() | ||
| @IsOptional() | ||
| control_method?: string; | ||
|
|
||
| @Field(() => FanControlSafetyConfig, { nullable: true }) | ||
| @ValidateNested() | ||
| @Type(() => FanControlSafetyConfig) | ||
| @IsOptional() | ||
| safety?: FanControlSafetyConfig; | ||
| } |
There was a problem hiding this comment.
FanProfileConfig / FanZoneConfig types exist (and FanCurveService expects zones), but FanControlConfig doesn’t include any fields to persist zones/profiles. As written, there’s no way to configure automatic curves via config, which conflicts with the PR description’s “persistent fan profiles” / “temperature-based curves”. Consider adding these fields to FanControlConfig (and the update input) or removing the unused types until they’re supported.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.ts (1)
6-6: Consider instantiating the service instead of extracting prototype method.Extracting
FanCurveService.prototype.interpolateSpeeddirectly works because the method is currently a pure function that doesn't accessthis. However, this pattern is fragile—if the method ever needs instance state (e.g., for logging or accessing injected dependencies), these tests will silently break.♻️ Safer alternative using service instantiation
-const interpolateSpeed = FanCurveService.prototype.interpolateSpeed; +const service = new FanCurveService(); describe('Fan Curve Interpolation', () => { // ... it('should return minimum speed below lowest temp', () => { - expect(interpolateSpeed(balancedCurve, 20)).toBe(30); + expect(service.interpolateSpeed(balancedCurve, 20)).toBe(30); }); // ... update other calls similarly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.ts` at line 6, The test currently pulls the method off the prototype via FanCurveService.prototype.interpolateSpeed which is fragile; instead instantiate the service and call the instance method so it stays bound to potential future instance state. Replace usages of FanCurveService.prototype.interpolateSpeed with an instance like new FanCurveService() and invoke its interpolateSpeed (or assign const interpolateSpeed = new FanCurveService().interpolateSpeed.bind(instance) if you need a standalone function) so tests use the service instance and will continue to work if interpolateSpeed later references this or injected dependencies.api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts (2)
1-1: Remove unusedFloatimport.The
Floatscalar is imported but not used in this file.🧹 Remove unused import
-import { Field, Float, InputType, Int } from '@nestjs/graphql'; +import { Field, InputType, Int } from '@nestjs/graphql';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts` at line 1, The import list in fancontrol.input.ts includes an unused symbol Float; update the import declaration that currently imports { Field, Float, InputType, Int } to remove Float so it only imports the used symbols (Field, InputType, Int) and then run type-check/lint to confirm no unused-import warnings remain.
13-15: Consider adding range validation for PWM value.The description states "PWM value (0-255)" but there's no
@Min(0)/@Max(255)validation. While the safety service may clamp values, validating at the input layer provides earlier feedback and clearer error messages.✨ Add range validators
+import { IsEnum, IsNumber, IsOptional, IsString, Max, Min } from 'class-validator'; ... `@Field`(() => Int, { description: 'PWM value (0-255)' }) `@IsNumber`() + `@Min`(0) + `@Max`(255) pwmValue!: number;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts` around lines 13 - 15, Add explicit range validation to the pwmValue input field: annotate the pwmValue property (the field decorated with `@Field`(() => Int, { description: 'PWM value (0-255)' }) and `@IsNumber`()) with `@Min`(0) and `@Max`(255) from class-validator so requests outside 0–255 are rejected with a clear validation error rather than relying only on downstream clamping.api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts (1)
141-156: Consider documenting the fan type inference heuristic.The assumption that
fanNumber === 1indicates a CPU fan may not hold true for all hardware configurations. This is a reasonable default, but users should be aware this is heuristic-based.Consider adding a brief comment or allowing user override via configuration in a future iteration:
private inferFanType(name: string, fanNumber: number): FanType { const lower = name.toLowerCase(); + // Heuristic: Fan 1 is typically the CPU fan on most motherboards if (lower.includes('cpu') || fanNumber === 1) { return FanType.CPU; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts` around lines 141 - 156, Add a short doc comment above the inferFanType function explaining that the mapping is heuristic-based (e.g., lowercased name checks and the special-case fanNumber === 1 assumed as CPU) and may not be accurate for all hardware; mention that FanType.CUSTOM is the fallback and add a TODO note to support a user override/configuration in the future so callers can supply explicit mappings if needed. Ensure the comment references the function name inferFanType and the FanType enum so readers can locate the logic and future work.api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.ts (2)
117-125: Non-null assertions on regex match results could throw.Lines 119 and 124 use
!non-null assertions on regex match results. While the filter ensures the regex matches, TypeScript can't verify this, and future refactoring could break the assumption.♻️ Safer pattern with optional chaining
const fans = files .filter((f) => /^fan\d+_input$/.test(f)) - .map((f) => parseInt(f.match(/^fan(\d+)_input$/)![1], 10)) + .map((f) => { + const match = f.match(/^fan(\d+)_input$/); + return match ? parseInt(match[1], 10) : 0; + }) + .filter((n) => n > 0) .sort((a, b) => a - b); const pwms = files .filter((f) => /^pwm\d+$/.test(f)) - .map((f) => parseInt(f.match(/^pwm(\d+)$/)![1], 10)) + .map((f) => { + const match = f.match(/^pwm(\d+)$/); + return match ? parseInt(match[1], 10) : 0; + }) + .filter((n) => n > 0) .sort((a, b) => a - b);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.ts` around lines 117 - 125, The code for computing fans and pwms uses non-null assertions on regex match results (in the fans and pwms mapping) which can throw; change the pattern to safely handle potential nulls by capturing the match first and only mapping when match is truthy (e.g., use files.filter(...).map(f => { const m = f.match(...); return m ? parseInt(m[1], 10) : undefined }).filter(Boolean) or use flatMap with a conditional match) so that parseInt is only called with a valid match; update the fans and pwms computations (the variables named fans and pwms) to perform a null-check on the match instead of using `!` and keep the final .sort((a,b)=>a-b).
151-158: Silent error handling in readSysfsInt may mask hardware issues.Returning 0 on any error (line 156) doesn't distinguish between "file doesn't exist" and "permission denied" or "hardware error". For fans, RPM=0 could be misinterpreted as a stopped fan rather than a read failure. Consider logging at debug level for unexpected errors.
💡 Add debug logging for read failures
private async readSysfsInt(devicePath: string, filename: string): Promise<number> { try { const content = await readFile(join(devicePath, filename), 'utf-8'); return parseInt(content.trim(), 10); - } catch { + } catch (err) { + // ENOENT is expected for optional files; log other errors + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + this.logger.debug(`Failed to read ${filename} from ${devicePath}: ${err}`); + } return 0; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.ts` around lines 151 - 158, The readSysfsInt method currently swallows all errors and returns 0, which can hide hardware or permission issues; update the catch to capture the thrown error and emit a debug-level log that includes devicePath, filename and the error details (message/stack) using the file's existing logger (or console.debug if no logger exists) before returning 0 so RPM=0 can be distinguished from a read failure for troubleshooting.api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts (1)
156-169: TheOFFcase in modeToEnable is unreachable due to upstream validation.
validateModeTransition()(per Context snippet 3 atfan-safety.service.ts:104-113) always returnsfalseforFanControlMode.OFF, blocking transitions beforemodeToEnableis ever called with that value. Consider removing theOFFcase or adding a defensive comment explaining it exists for completeness.💡 Optional: Add comment clarifying dead code path
private modeToEnable(mode: FanControlMode): number { switch (mode) { case FanControlMode.MANUAL: return 1; case FanControlMode.AUTOMATIC: return 2; case FanControlMode.FIXED: return 1; case FanControlMode.OFF: + // Unreachable: validateModeTransition blocks OFF transitions return 0; default: return 2; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts` around lines 156 - 169, modeToEnable contains an unreachable FanControlMode.OFF branch because validateModeTransition (see fan-safety.service.ts validateModeTransition) blocks OFF transitions earlier; remove the OFF case from modeToEnable or keep it but add a one-line defensive comment above the switch noting OFF is unreachable due to upstream validation (reference function modeToEnable and enum FanControlMode) so future maintainers understand it’s intentionally redundant.api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts (1)
38-57: Redundant clamping after validation.Line 39-41 validates the value is in 0-255 range, then line 42 clamps again with
Math.max(0, Math.min(255, value)). The clamping is redundant since validation already ensures bounds.♻️ Simplify by removing redundant clamping
async setPwm(devicePath: string, pwmNumber: number, value: number): Promise<void> { if (!Number.isFinite(value) || value < 0 || value > 255) { throw new Error(`Invalid PWM value: ${value}. Must be a number between 0 and 255.`); } - const percent = Math.round((Math.max(0, Math.min(255, value)) / 255) * 100); + const percent = Math.round((value / 255) * 100); try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts` around lines 38 - 57, In setPwm, remove the redundant clamping and use the already-validated value when computing percent: after the existing Number.isFinite/value-range check in setPwm, replace Math.max(0, Math.min(255, value)) with value so percent is calculated as Math.round((value / 255) * 100); keep the validation/throw as-is and preserve the execa call, timeout handling, logger.debug and error handling in the existing setPwm method.api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts (1)
162-167: Silent skip when fan not found in zone configuration.When
readings.find((r) => r.id === fanId)returns undefined (line 164), the code silently continues. This could mask configuration errors where zone.fans contains invalid fan IDs. Consider logging at debug level when a configured fan isn't found.💡 Add debug logging for missing fans
const fan = readings.find((r) => r.id === fanId); - if (!fan || !fan.hasPwmControl) { + if (!fan) { + this.logger.debug(`Fan ${fanId} not found in readings, skipping`); + continue; + } + if (!fan.hasPwmControl) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts` around lines 162 - 167, The loop over zone.fans in fan-curve.service.ts silently skips when readings.find((r) => r.id === fanId) returns undefined; add a debug log to surface misconfigured fan IDs: inside the for (const fanId of zone.fans) block, when fan is falsy before the continue, call the module/process logger at debug (or trace) level with a clear message including the zone.id (or zone identifier), the missing fanId and context (e.g., "configured in zone but no reading found"), then continue; keep existing behavior for fan.hasPwmControl but also consider logging when hasPwmControl is false for completeness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`:
- Around line 106-111: The setInterval async callback can produce unhandled
promise rejections when this.applyCurves() throws; update the callback used to
set this.curveInterval so the await is wrapped in a try/catch (inside the
existing if (this.isApplyingCurves) guard) and any error is handled/logged
(e.g., via this.logger.error or console.error) so rejections are not left
unhandled; reference the existing symbols: this.curveInterval, setInterval(...),
this.isApplyingCurves, and this.applyCurves().
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts`:
- Around line 59-75: restoreAllFans currently clears originalStates and sets
isEmergencyMode = false even if some hwmonService.restoreAutomatic calls fail;
update restoreAllFans to track per-fan failures (use a local failedIds or
failedStates collection), only clear this.originalStates and set
this.isEmergencyMode = false when all restores succeed, otherwise keep failed
entries in this.originalStates for retry and leave isEmergencyMode true; ensure
failures from hwmonService.restoreAutomatic are logged via this.logger.error (as
already done) and consider returning a boolean or throwing an error to surface
partial failure to callers.
- Around line 25-43: captureState currently calls hwmonService.readAll()
internally which can race with the caller's own snapshot; change
captureState(fanId: string, devicePath: string, pwmNumber: number) to accept an
optional reading parameter (e.g., reading?: HwmonReading) and use that reading
to populate originalStates when provided, falling back to hwmonService.readAll()
only if no reading is passed. Update callers such as
FanCurveService.applyCurves() to pass the reading/snapshot they already obtained
instead of letting captureState re-read, and keep the originalStates map
handling and logger.debug behavior unchanged.
---
Nitpick comments:
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.ts`:
- Around line 117-125: The code for computing fans and pwms uses non-null
assertions on regex match results (in the fans and pwms mapping) which can
throw; change the pattern to safely handle potential nulls by capturing the
match first and only mapping when match is truthy (e.g., use
files.filter(...).map(f => { const m = f.match(...); return m ? parseInt(m[1],
10) : undefined }).filter(Boolean) or use flatMap with a conditional match) so
that parseInt is only called with a valid match; update the fans and pwms
computations (the variables named fans and pwms) to perform a null-check on the
match instead of using `!` and keep the final .sort((a,b)=>a-b).
- Around line 151-158: The readSysfsInt method currently swallows all errors and
returns 0, which can hide hardware or permission issues; update the catch to
capture the thrown error and emit a debug-level log that includes devicePath,
filename and the error details (message/stack) using the file's existing logger
(or console.debug if no logger exists) before returning 0 so RPM=0 can be
distinguished from a read failure for troubleshooting.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts`:
- Around line 38-57: In setPwm, remove the redundant clamping and use the
already-validated value when computing percent: after the existing
Number.isFinite/value-range check in setPwm, replace Math.max(0, Math.min(255,
value)) with value so percent is calculated as Math.round((value / 255) * 100);
keep the validation/throw as-is and preserve the execa call, timeout handling,
logger.debug and error handling in the existing setPwm method.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.ts`:
- Line 6: The test currently pulls the method off the prototype via
FanCurveService.prototype.interpolateSpeed which is fragile; instead instantiate
the service and call the instance method so it stays bound to potential future
instance state. Replace usages of FanCurveService.prototype.interpolateSpeed
with an instance like new FanCurveService() and invoke its interpolateSpeed (or
assign const interpolateSpeed = new
FanCurveService().interpolateSpeed.bind(instance) if you need a standalone
function) so tests use the service instance and will continue to work if
interpolateSpeed later references this or injected dependencies.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`:
- Around line 162-167: The loop over zone.fans in fan-curve.service.ts silently
skips when readings.find((r) => r.id === fanId) returns undefined; add a debug
log to surface misconfigured fan IDs: inside the for (const fanId of zone.fans)
block, when fan is falsy before the continue, call the module/process logger at
debug (or trace) level with a clear message including the zone.id (or zone
identifier), the missing fanId and context (e.g., "configured in zone but no
reading found"), then continue; keep existing behavior for fan.hasPwmControl but
also consider logging when hasPwmControl is false for completeness.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts`:
- Line 1: The import list in fancontrol.input.ts includes an unused symbol
Float; update the import declaration that currently imports { Field, Float,
InputType, Int } to remove Float so it only imports the used symbols (Field,
InputType, Int) and then run type-check/lint to confirm no unused-import
warnings remain.
- Around line 13-15: Add explicit range validation to the pwmValue input field:
annotate the pwmValue property (the field decorated with `@Field`(() => Int, {
description: 'PWM value (0-255)' }) and `@IsNumber`()) with `@Min`(0) and `@Max`(255)
from class-validator so requests outside 0–255 are rejected with a clear
validation error rather than relying only on downstream clamping.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`:
- Around line 156-169: modeToEnable contains an unreachable FanControlMode.OFF
branch because validateModeTransition (see fan-safety.service.ts
validateModeTransition) blocks OFF transitions earlier; remove the OFF case from
modeToEnable or keep it but add a one-line defensive comment above the switch
noting OFF is unreachable due to upstream validation (reference function
modeToEnable and enum FanControlMode) so future maintainers understand it’s
intentionally redundant.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts`:
- Around line 141-156: Add a short doc comment above the inferFanType function
explaining that the mapping is heuristic-based (e.g., lowercased name checks and
the special-case fanNumber === 1 assumed as CPU) and may not be accurate for all
hardware; mention that FanType.CUSTOM is the fallback and add a TODO note to
support a user override/configuration in the future so callers can supply
explicit mappings if needed. Ensure the comment references the function name
inferFanType and the FanType enum so readers can locate the logic and future
work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6bf90734-d7a1-4a75-ad9f-d0602828b9f9
📒 Files selected for processing (23)
api/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/controller.interface.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.module.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.model.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/temperature/temperature.resolver.integration.spec.tspackages/unraid-shared/src/pubsub/graphql.pubsub.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1954 +/- ##
==========================================
+ Coverage 52.08% 52.19% +0.10%
==========================================
Files 1031 1043 +12
Lines 71564 72920 +1356
Branches 8090 8326 +236
==========================================
+ Hits 37275 38061 +786
- Misses 34164 34734 +570
Partials 125 125 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.ts (1)
157-160: Avoid order-coupled fan assertions.These cases assert
metrics.fans[0]andmetrics.fans[2], so a harmless merge/sort change inFanControlServicewould fail the suite even if the returned payload is still correct. Resolve the expected fan byidornamebefore asserting its fields.Also applies to: 166-167, 174-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.ts` around lines 157 - 160, The test is order-coupled by asserting metrics.fans[0] and metrics.fans[2]; change the assertions to first locate the expected fan object by a stable key (e.g., id or name) from the metrics.fans array (use Array.find with the known id or name) and then assert its fields (current.rpm, name, etc.) for the found object; apply the same change to the other similar assertions referenced (the ones currently using metrics.fans[0], metrics.fans[2] and the assertions at the other mentioned locations) so tests no longer rely on array ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`:
- Around line 87-93: The interpolation loop in interpolateSpeed (iterating
sorted, using lower/upper) doesn't guard against zero-width segments where
upper.temp === lower.temp, causing division by zero and NaN/Infinity to flow
into setPwm; fix by adding a check inside the loop: if upper.temp === lower.temp
then return a sensible deterministic speed (e.g., lower.speed or upper.speed) or
skip the segment and continue, otherwise compute the ratio normally, ensuring
any degenerate adjacent points from user profiles cannot produce NaN.
- Around line 152-159: The temperature safety check is currently run against
only zoneSensors (built from this.activeZones), so checkTemperatureSafety only
sees zoned sensors and misses any overheating unmapped sensors; change the call
in the FanCurveService method to pass tempMetrics.sensors (all reported sensors)
into this.safetyService.checkTemperatureSafety while retaining the
zoneSensorIds/zoneSensors filtering for subsequent curve selection logic (i.e.,
use tempMetrics.sensors for safety check and still use zoneSensors for selecting
curves based on this.activeZones and zoneSensorIds).
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts`:
- Around line 47-53: captureState() saves both pwmEnable and pwmValue into
originalStates, but restoreAllFans() only calls
HwmonService.restoreAutomatic(...) which only restores pwm*_enable and thus
loses any saved duty cycle; update the restore path to replay the saved pwmValue
for fans that were in manual mode: when iterating originalStates in
restoreAllFans(), check each saved state (pwmEnable and pwmValue) and if
pwmEnable indicates manual mode, call the appropriate HwmonService method to set
the pwm value (or call a new method e.g., HwmonService.restoreManual(devicePath,
pwmNumber, pwmValue)) and then restore the enable flag, otherwise call
HwmonService.restoreAutomatic as before; ensure the symbol names involved are
originalStates, captureState(), restoreAllFans(), pwmEnable, pwmValue and
HwmonService.restoreAutomatic/restoreManual so the manual duty cycle is
reapplied on restore.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts`:
- Around line 4-14: Add range validation decorators to the fan control config
and its mutation input so safety percentages and intervals cannot produce
invalid PWM or timers: in fancontrol-config model class (e.g., the persisted
config model) add `@Min`(0) `@Max`(100) to min_speed_percent and max_speed_percent
and add `@Min`(1) `@Max`(600000) (or appropriate bounds) to polling_interval, and
mirror those same `@Min/`@Max constraints on the mutation/input DTO used by your
GraphQL mutation; ensure imports for Min and Max from 'class-validator' are
added and that these fields are validated before
FanSafetyService.validatePwmValue() and FanCurveService.start() are called.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`:
- Around line 254-266: modeToEnable maps both FanControlMode.FIXED and .MANUAL
to pwm_enable=1 causing pwmEnableToControlMode to always decode 1 as MANUAL and
lose FIXED; change modeToEnable so FIXED maps to a distinct pwm_enable value
(choose a value the hardware/protocol supports, e.g., 3) and update
pwmEnableToControlMode to decode that value back to FanControlMode.FIXED; ensure
both functions (modeToEnable and pwmEnableToControlMode) reference
FanControlMode so FIXED round-trips correctly.
- Around line 161-167: The fan-curve engine is being started/fire-and-forgot
without awaiting startup and an empty zones array isn't stopping the engine;
update updateFanControlConfig to await this.fanCurveService.start(updated.zones)
so you only log/consider it started after applyCurves completes, and add an
explicit stop call when updated.zones is present but empty (i.e., treat
zones.length === 0 as a stop), ensuring you call this.fanCurveService.stop() in
that case; apply the same await-and-empty-zones-stop fix for the other
occurrence around the start lines referenced (the block using
this.fanCurveService.start/stop at the second location).
- Around line 229-245: The createFanProfile resolver (createFanProfile)
currently only persists description and curve into config.profiles, so
CreateFanProfileInput fields temperatureSensorId, minSpeed and maxSpeed are
dropped; update the persistence to include those fields into the stored
FanProfileConfig object (e.g., add temperatureSensorId, minSpeed, maxSpeed
alongside description and curve derived from input.curvePoints) and ensure the
read path that returns FanProfile (and FanProfile.minSpeed/maxSpeed) maps those
stored config fields back; alternatively remove those fields from
CreateFanProfileInput if you prefer not to implement persistence yet.
- Around line 192-210: The mutation currently only checks existence of the fan;
instead, validate that the matched fan (from this.hwmonService.readAll()) is
PWM-capable and reject any new zone assignment without a temperatureSensorId to
avoid persisting inert zones that FanCurveService.applyCurves() will ignore. In
the resolver handling (around readings, fan, zones, existingZoneIdx and
config.zones), after finding the fan confirm its PWM capability (e.g., a
property like fan.isPwm or fan.pwmSupported) and throw an error if not PWM; when
creating a new zone (the else branch that pushes into zones) require
input.temperatureSensorId to be present and throw if missing; for existing zones
allow updating profile but also validate/clear sensor assignment consistently.
Ensure error messages reference the fan id and missing sensor to aid debugging.
---
Nitpick comments:
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.ts`:
- Around line 157-160: The test is order-coupled by asserting metrics.fans[0]
and metrics.fans[2]; change the assertions to first locate the expected fan
object by a stable key (e.g., id or name) from the metrics.fans array (use
Array.find with the known id or name) and then assert its fields (current.rpm,
name, etc.) for the found object; apply the same change to the other similar
assertions referenced (the ones currently using metrics.fans[0], metrics.fans[2]
and the assertions at the other mentioned locations) so tests no longer rely on
array ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe9e8dc4-095f-4620-91f6-bf76a7031129
📒 Files selected for processing (15)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/metrics/temperature/temperature.resolver.integration.spec.ts
✅ Files skipped from review due to trivial changes (1)
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- api/src/unraid-api/graph/resolvers/metrics/temperature/temperature.resolver.integration.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.service.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.ts
Comment on lines
+254
to
+266
| private modeToEnable(mode: FanControlMode): number { | ||
| switch (mode) { | ||
| case FanControlMode.MANUAL: | ||
| return 1; | ||
| case FanControlMode.AUTOMATIC: | ||
| return 2; | ||
| case FanControlMode.FIXED: | ||
| return 1; | ||
| case FanControlMode.OFF: | ||
| return 0; | ||
| default: | ||
| return 2; | ||
| } |
Contributor
There was a problem hiding this comment.
FIXED cannot round-trip through the hardware mapping.
Both FIXED and MANUAL map to pwm_enable = 1, but pwmEnableToControlMode() only decodes 1 back to MANUAL. After setFanMode(FIXED), the next read will report MANUAL, so the API currently exposes a mode the backend cannot faithfully represent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`
around lines 254 - 266, modeToEnable maps both FanControlMode.FIXED and .MANUAL
to pwm_enable=1 causing pwmEnableToControlMode to always decode 1 as MANUAL and
lose FIXED; change modeToEnable so FIXED maps to a distinct pwm_enable value
(choose a value the hardware/protocol supports, e.g., 3) and update
pwmEnableToControlMode to decode that value back to FanControlMode.FIXED; ensure
both functions (modeToEnable and pwmEnableToControlMode) reference
FanControlMode so FIXED round-trips correctly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts (2)
201-213:⚠️ Potential issue | 🟠 MajorRequire
temperatureSensorIdwhen creating a new zone.This still persists
sensor: ''for first-time assignments.FanCurveService.applyCurves()resolves sensors by id/name, so the new zone can never drive that fan until some later config repair.🛠️ Suggested change
if (existingZoneIdx >= 0) { zones[existingZoneIdx].profile = input.profileName; if (input.temperatureSensorId) { zones[existingZoneIdx].sensor = input.temperatureSensorId; } } else { + if (!input.temperatureSensorId) { + throw new Error( + `temperatureSensorId is required when assigning fan ${input.fanId} to a new zone` + ); + } zones.push({ fans: [input.fanId], - sensor: input.temperatureSensorId ?? '', + sensor: input.temperatureSensorId, profile: input.profileName, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts` around lines 201 - 213, When creating a new zone in the zones.push block, do not persist an empty sensor string; require input.temperatureSensorId and fail fast if it's missing so FanCurveService.applyCurves can resolve the sensor. Update the logic around zones, existingZoneIdx and input.fanId to: if existing zone -> keep the existing behavior; if creating a new zone -> validate input.temperatureSensorId is present (throw or return a validation error) and set sensor to input.temperatureSensorId (no fallback to ''). Ensure the validation prevents pushing a zone with sensor: '' so FanCurveService.applyCurves can find the sensor by id/name.
259-268:⚠️ Potential issue | 🟠 Major
FIXEDstill cannot round-trip.This mapping intentionally collapses
FIXEDonto the same raw value asMANUAL, so the next read can only come back asMANUAL. The API is still advertising a state it cannot faithfully persist or report.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts` around lines 259 - 268, The resolver currently maps FanControlMode.FIXED to the same raw value as MANUAL in modeToEnable, which means FIXED cannot round-trip; change modeToEnable to reject/throw when callers try to set FanControlMode.FIXED (or return a clear error) so callers can't persist an unrepresentable state, and update the reverse mapping (e.g., modeFromEnable / readAll) to never emit FIXED (always emit MANUAL for the raw value 1); reference the FanControlMode enum, the modeToEnable method, and the code paths that convert raw pwm_enable values back to FanControlMode (readAll/modeFromEnable) to implement these changes.api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts (1)
138-141:⚠️ Potential issue | 🟠 Major
polling_intervalstill lacks integer/range validation.The persisted model and the mutation DTO both accept zero, negative, and fractional values here. One bad config can turn the polling loop into an effectively unbounded timer.
🛠️ Suggested change
import { IsBoolean, + IsInt, IsNumber, IsOptional, IsString, @@ `@Field`(() => Int, { nullable: true }) - `@IsNumber`() + `@IsInt`() `@IsOptional`() + `@Min`(1) + `@Max`(600000) polling_interval?: number; @@ `@Field`(() => Int, { nullable: true }) - `@IsNumber`() + `@IsInt`() `@IsOptional`() + `@Min`(1) + `@Max`(600000) polling_interval?: number;Also applies to: 226-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts` around lines 138 - 141, The polling_interval field allows zero, negative, and fractional values; add integer and range validation by decorating polling_interval with `@IsInt`() and `@Min`(1) (imported from class-validator) so only positive integers are accepted, and apply the same change to the other polling_interval occurrence in this file/mutation DTO to keep model and DTO consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`:
- Around line 61-63: Stopping the curve engine currently only clears timers
(stop()/onModuleDestroy()) but doesn't restore fans from manual PWM mode; update
stop() and onModuleDestroy() (and the other shutdown path around lines 128-134)
to iterate active fans and restore hardware control by calling the existing
fan-mode reset API (e.g., call setFanManual(fanId, false) or the equivalent
method that re-enables hardware/fancontrol for each fan last touched by
applyCurves()), ensuring fans are not left pinned at the last duty cycle when
the engine is disabled or the module is destroyed.
- Around line 113-125: The polling interval is started before the initial
startup pass completes so a failing initial applyCurves() can leave a background
loop running; change the order so you await this.applyCurves() first and only
call setInterval(...) to assign this.curveInterval (and log via this.logger.log)
after that call succeeds; keep the existing isApplyingCurves guard and the
try/catch inside the interval callback to preserve runtime error handling.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts`:
- Around line 154-166: The zones array currently allows duplicate fan ids so
later zones override earlier ones at runtime; add a class-level uniqueness
validation for FanControlConfig (or the model that declares zones) to reject
configs where the same fan id appears in more than one FanZoneConfig. Implement
a custom validator (e.g., ValidateUniqueFanIdsAcrossZones) that iterates
FanControlConfig.zones (each FanZoneConfig) and ensures all fan ids are unique,
attach it with `@Validate` at the class level (or on the zones property) so
updateFanControlConfig() and validation pipelines fail when duplicates are
present; reference FanZoneConfig, FanControlConfig (the model with zones), and
FanCurveService.applyCurves()/updateFanControlConfig() to locate where this
uniqueness must be enforced.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`:
- Around line 238-250: The overwrite check currently calls
this.fanCurveService.getProfiles() which returns the runtime merged view; change
it to use the built-in-only set (e.g. call
this.fanCurveService.getBuiltinProfiles() or the service method that returns
only built-in profiles) and check that built-in list for input.name before
throwing; update the check around profiles[input.name] so it uses
builtInProfiles[input.name] (and keep the rest of the creation logic unchanged).
---
Duplicate comments:
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts`:
- Around line 138-141: The polling_interval field allows zero, negative, and
fractional values; add integer and range validation by decorating
polling_interval with `@IsInt`() and `@Min`(1) (imported from class-validator) so
only positive integers are accepted, and apply the same change to the other
polling_interval occurrence in this file/mutation DTO to keep model and DTO
consistent.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`:
- Around line 201-213: When creating a new zone in the zones.push block, do not
persist an empty sensor string; require input.temperatureSensorId and fail fast
if it's missing so FanCurveService.applyCurves can resolve the sensor. Update
the logic around zones, existingZoneIdx and input.fanId to: if existing zone ->
keep the existing behavior; if creating a new zone -> validate
input.temperatureSensorId is present (throw or return a validation error) and
set sensor to input.temperatureSensorId (no fallback to ''). Ensure the
validation prevents pushing a zone with sensor: '' so
FanCurveService.applyCurves can find the sensor by id/name.
- Around line 259-268: The resolver currently maps FanControlMode.FIXED to the
same raw value as MANUAL in modeToEnable, which means FIXED cannot round-trip;
change modeToEnable to reject/throw when callers try to set FanControlMode.FIXED
(or return a clear error) so callers can't persist an unrepresentable state, and
update the reverse mapping (e.g., modeFromEnable / readAll) to never emit FIXED
(always emit MANUAL for the raw value 1); reference the FanControlMode enum, the
modeToEnable method, and the code paths that convert raw pwm_enable values back
to FanControlMode (readAll/modeFromEnable) to implement these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 352e9640-54f1-41fb-b3da-6f6c83238979
📒 Files selected for processing (4)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts (1)
232-262:⚠️ Potential issue | 🟡 MinorExisting custom profiles are silently overwritten.
The mutation checks for built-in profile name collisions (line 244) but doesn't prevent overwriting an existing custom profile. Calling
createFanProfilewith the same name twice will silently replace the previous definition.💡 Suggested improvement
const builtInProfiles = this.fanCurveService.getDefaultProfiles(); if (builtInProfiles[input.name]) { throw new Error( `Cannot overwrite built-in profile "${input.name}". Choose a different name.` ); } + if (profiles[input.name]) { + throw new Error( + `Profile "${input.name}" already exists. Use updateFanProfile to modify it.` + ); + } profiles[input.name] = {Alternatively, if overwriting is intentional, rename the mutation to
upsertFanProfileor document the behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts` around lines 232 - 262, The createFanProfile mutation currently allows a new custom profile to silently overwrite an existing custom profile because it only checks builtInProfiles; update createFanProfile to check profiles[input.name] (the existing custom profiles object retrieved from configService.getConfig()) and throw an error if a custom profile with that name already exists, or alternatively implement explicit upsert behavior and rename to upsertFanProfile; ensure the check occurs before assigning to profiles[input.name] and before calling configService.replaceConfig so existing definitions are not replaced unintentionally.
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts (1)
50-59: Consider adding range constraints to curve point values.The
speedfield inFanCurvePointConfiglacks@Min(0)and@Max(100)constraints that are present onFanProfileConfig.minSpeed/maxSpeed. Invalid speed values (e.g., negative or >100) in curve points could produce unexpected PWM outputs during interpolation.💡 Suggested improvement
`@ObjectType`() export class FanCurvePointConfig { `@Field`(() => Float) `@IsNumber`() + `@Min`(0) temp!: number; `@Field`(() => Float) `@IsNumber`() + `@Min`(0) + `@Max`(100) speed!: number; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts` around lines 50 - 59, The FanCurvePointConfig.speed field currently has no range validation; add `@Min`(0) and `@Max`(100) decorators (matching FanProfileConfig.minSpeed/maxSpeed usage) to FanCurvePointConfig.speed and ensure the class-validator imports include Min and Max so curve points cannot be negative or exceed 100, preventing invalid PWM interpolation values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`:
- Around line 232-262: The createFanProfile mutation currently allows a new
custom profile to silently overwrite an existing custom profile because it only
checks builtInProfiles; update createFanProfile to check profiles[input.name]
(the existing custom profiles object retrieved from configService.getConfig())
and throw an error if a custom profile with that name already exists, or
alternatively implement explicit upsert behavior and rename to upsertFanProfile;
ensure the check occurs before assigning to profiles[input.name] and before
calling configService.replaceConfig so existing definitions are not replaced
unintentionally.
---
Nitpick comments:
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts`:
- Around line 50-59: The FanCurvePointConfig.speed field currently has no range
validation; add `@Min`(0) and `@Max`(100) decorators (matching
FanProfileConfig.minSpeed/maxSpeed usage) to FanCurvePointConfig.speed and
ensure the class-validator imports include Min and Max so curve points cannot be
negative or exceed 100, preventing invalid PWM interpolation values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 535b8e67-d81d-4e29-8ce8-dca01f566f7d
📒 Files selected for processing (3)
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts
Member
|
Thank you so much! This is a large change and we'll need a bit to review this and get it added. We really appreciate your hard work on building this out, and I'm excited to test this. |
| @@ -0,0 +1,48 @@ | |||
| import { Field, InputType, Int } from '@nestjs/graphql'; | |||
|
|
|||
| import { IsEnum, IsInt, IsNumber, IsOptional, IsString, Max, Min } from 'class-validator'; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
Author
No worries! Take your time to review and let me know if you need anything else |
…gement - Added FanSafetyService to manage fan states and emergency protocols. - Introduced FanControlConfigService for handling fan control configurations. - Created models for fan control configuration, including safety parameters. - Developed resolver and input types for fan control operations. - Implemented fan control metrics and summary calculations. - Added unit tests for fan control service and related functionalities.
…profiles - Improved HwmonService and IpmiFanService to handle fan input parsing more robustly. - Added temperature safety checks in FanSafetyService to trigger emergency modes based on sensor readings. - Implemented fan failure detection logic in FanSafetyService to monitor and respond to fan failures. - Enhanced FanCurveService to support custom fan profiles and automatic curve control based on temperature zones. - Updated FanControlConfig to include zones and profiles for better configuration management. - Added integration tests for new features in MetricsResolver and FanControlService. - Refactored fan control resolver to support assigning profiles and creating custom fan profiles.
…onfiguration limits
…for improved safety and error handling
…ate profile names
elibosley
force-pushed
the
feat/fan-control-system
branch
from
4806160 to
66f0d92
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts`:
- Around line 45-47: The IPMI raw command is using pwmNumber where a Supermicro
fan zone ID is required; update the code around the execa call in
ipmi_fan.service.ts so that you map the incoming pwmNumber to the correct zone
ID (e.g., 0 for CPU, 1 for peripheral) before invoking execa, and pass that
zoneId as the fifth argument instead of pwmNumber; adjust the function signature
or add a helper (e.g., mapPwmToZone or computeZoneId) and use zoneId in the
execa call along with the existing percent parameter.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`:
- Around line 155-205: The curve loop currently only reads from hwmon
(this.hwmonService.readAll()) and always calls hwmon write methods, so
IPMI-exposed fans are ignored; fix by also fetching IPMI fan readings (e.g.
await this.ipmiFanService.readAll()) and merging them into the readings array
before the zone loop, and when writing, branch on the fan source (e.g.
fan.source === 'ipmi' or presence of an ipmi-specific id) to call
this.ipmiFanService.setMode/setPwm for IPMI fans and
this.hwmonService.setMode/setPwm for hwmon fans; keep safetyService.captureState
and PWM validation as-is but ensure you route writes to the correct service
based on the fan object.
- Around line 106-111: In start(), always reset this.profiles based on the
current config so stale custom profiles are not retained: call
this.configService.getConfig(), then set this.profiles = { ...DEFAULT_PROFILES,
...(config.profiles ?? {}) } (or set to DEFAULT_PROFILES when config.profiles is
falsy) instead of only assigning when config.profiles is truthy; update the
start() method in fan-curve.service.ts to perform this unconditional assignment
before proceeding.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts`:
- Around line 159-170: The emergency full-speed loop only handles hwmon fans and
skips IPMI-controlled fans; update the emergency path in fan-safety.service.ts
to also query this.ipmiFanService.readAll() (or equivalent IPMI enumerator) and
perform the same captureState/override steps for those entries: call
captureState with the IPMI fan identifier, then use the IPMI service methods to
switch control into manual/full-speed (e.g., this.ipmiFanService.setMode or
this.ipmiFanService.setFullSpeed / setPwm depending on API) inside the existing
try/catch and log failures the same way as for hwmon fans. Ensure you reference
and handle the IPMI fan fields analogous to reading.id, reading.devicePath, and
reading.pwmNumber so IPMI-controlled fans are included in the
critical-temperature override.
- Around line 131-138: The code currently calls this.hwmonService.setPwm(...)
when state.pwmEnable === 1 and then always calls
this.hwmonService.restoreAutomatic(...), but restoreAutomatic coerces a value of
1 to 2 so a fan that was originally in manual mode never gets restored
correctly; change the logic in the block around state.pwmEnable to skip calling
this.hwmonService.restoreAutomatic(state.devicePath, state.pwmNumber,
state.pwmEnable) when state.pwmEnable === 1 (i.e., only call restoreAutomatic
for non-manual modes) or alternatively ensure restoreAutomatic receives the
original mode without coercion—locate the handling in fan-safety.service.ts
referencing state.pwmEnable, this.hwmonService.setPwm, and
this.hwmonService.restoreAutomatic and update the conditional so manual-mode
(pwmEnable === 1) is preserved.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts`:
- Around line 91-102: The FanZoneConfig model allows empty zones; add validation
to prevent empty arrays/strings by decorating fans with `@ArrayMinSize`(1) (in
addition to `@IsString`({ each: true })) and decorating sensor and profile with
`@IsNotEmpty`() (or `@MinLength`(1)) so they cannot be empty strings; also apply the
same decorators to the corresponding input DTO class used for updates (the
fan-zone input DTO used by updateFanControlConfig) and import the validators
from class-validator so persisted configs and incoming DTOs cannot be { fans:
[], sensor: '', profile: '' }.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`:
- Around line 43-84: setFanSpeed currently writes PWM/mode directly via
hwmonService (setMode/setPwm) after captureState but ignores whether the fan
belongs to an active zone, so the curve engine can immediately overwrite the
manual change; update setFanSpeed (and the analogous code around lines 92-123)
to first check zone membership and active control: use the zone management API
(e.g., ZoneService.isFanInActiveZone(fan.id) or similar) and either reject the
mutation with a clear error when the fan is in an active automatic zone, or
programmatically remove/detach the fan from that zone (call
ZoneService.detachFanFromZone(fan.id) or ZoneService.updateZone to exclude it)
before changing mode/PWM, then proceed with safetyService.captureState and
hwmonService.setMode/setPwm; ensure you reference pwmEnableToControlMode and
FanControlMode.MANUAL logic after handling the zone state so the manual override
is durable.
- Around line 201-218: The current logic in the resolver uses existingZoneIdx
(found by zones.findIndex(z => z.fans.includes(input.fanId))) and then mutates
zones[existingZoneIdx].profile and .sensor, which unintentionally changes all
fans in a multi-fan zone; update the handler in the function that applies
setFanProfile so that if the located zone's fans array has length > 1 you either
(A) split the single fan out: remove input.fanId from
zones[existingZoneIdx].fans and push a new zone object { fans: [input.fanId],
sensor: input.temperatureSensorId, profile: input.profileName } (ensuring
temperatureSensorId exists), or (B) reject the operation by throwing an explicit
error when trying to set a per-fan profile on a shared zone; implement one of
these behaviors around the zones / existingZoneIdx branch so you no longer
overwrite the shared zone profile/sensor for sibling fans.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts`:
- Around line 131-141: The mapper currently hard-codes minSpeed/maxSpeed
(20/100) and drops temperatureSensorId, so stored custom profiles won't
round-trip; update the mapping in the Object.entries(allProfiles).map callback
that constructs new FanProfile() to copy profileConfig.minSpeed,
profileConfig.maxSpeed, and profileConfig.temperatureSensorId (with sensible
fallbacks like profileConfig.minSpeed ?? 20 and profileConfig.maxSpeed ?? 100)
instead of using fixed literals, keeping the existing curvePoints mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 89a385d6-f453-4df0-be3a-5aaf78adaa59
📒 Files selected for processing (23)
api/dev/configs/api.jsonapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/controller.interface.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/hwmon.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.service.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.model.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.module.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.tsapi/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.model.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.module.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.integration.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.tsapi/src/unraid-api/graph/resolvers/metrics/metrics.resolver.tsapi/src/unraid-api/graph/resolvers/metrics/temperature/temperature.resolver.integration.spec.tspackages/unraid-shared/src/pubsub/graphql.pubsub.ts
✅ Files skipped from review due to trivial changes (7)
- packages/unraid-shared/src/pubsub/graphql.pubsub.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.module.ts
- api/dev/configs/api.json
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.input.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.service.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/controller.interface.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- api/src/unraid-api/graph/resolvers/metrics/metrics.model.ts
- api/src/unraid-api/graph/resolvers/metrics/temperature/temperature.resolver.integration.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.ts
- api/src/unraid-api/graph/resolvers/metrics/metrics.resolver.spec.ts
- api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.spec.ts
Comment on lines
+45
to
+47
| await execa( | ||
| 'ipmitool', | ||
| ['raw', '0x30', '0x70', '0x66', '0x01', String(pwmNumber), String(percent)], |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Supermicro fan control with ipmitool raw 0x30 0x70 0x66 0x01 , what values are valid for ? Does the command target fan zones instead of individual fan headers?
💡 Result:
Valid values for <zone> in the Supermicro IPMI command ipmitool raw 0x30 0x70 0x66 0x01 <zone> <duty> are motherboard-specific integers starting from 0x00 (typically 0 in decimal), such as 0x00 for CPU fans (e.g., FAN1-FAN4) and 0x01 for peripheral/system fans (e.g., FANA-FANH). Higher values like 0x02-0x04 may exist on boards with more zones. The exact mapping depends on the motherboard model; users should test zones incrementally (0,1,2,...) while monitoring fan speeds via ipmitool sdr to identify active zones. Yes, the command targets fan zones (groups of fan headers) rather than individual fan headers. Supermicro IPMI groups fans into zones for unified control (e.g., Zone 0: CPU area fans; Zone 1: peripheral fans). Individual header control is not supported via this command.
Citations:
- 1: https://a.osmarks.net/content/serverfault.com_en_all_2022-11/questions/662526/fan-speeds-on-supermicro-system-via-ipmi
- 2: https://codestrian.com/index.php/2022/06/29/updating-the-fan-speed-for-supermicro/
- 3: https://b3n.org/supermicro-fan-speed-script/
- 4: https://serverfault.com/questions/662526/fan-speeds-on-supermicro-system-via-ipmi
- 5: Super Micro X10DRU-i+ zones petersulyok/smfc#20
- 6: https://forums.servethehome.com/index.php?threads%2Fsupermicro-x9-x10-x11-fan-speed-control.10059%2Fpage-12=
- 7: https://eterio.eu/sterowniki/serwery/Manual/Management/SMCIPMITool_User_Guide.pdf
🏁 Script executed:
cat -n api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts | head -60Repository: unraid/api
Length of output: 2543
Use a zone ID instead of pwmNumber in the Supermicro IPMI zone command.
The command 0x30 0x70 0x66 0x01 is a zone-level Supermicro IPMI command where the fifth parameter must be a fan zone ID (e.g., 0 for CPU fans, 1 for peripheral fans), not a per-fan PWM number. Passing pwmNumber directly will either fail with invalid zone values or control the wrong fan group. The zone mapping should be handled before calling this command.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/controllers/ipmi_fan.service.ts`
around lines 45 - 47, The IPMI raw command is using pwmNumber where a Supermicro
fan zone ID is required; update the code around the execa call in
ipmi_fan.service.ts so that you map the incoming pwmNumber to the correct zone
ID (e.g., 0 for CPU, 1 for peripheral) before invoking execa, and pass that
zoneId as the fifth argument instead of pwmNumber; adjust the function signature
or add a helper (e.g., mapPwmToZone or computeZoneId) and use zoneId in the
execa call along with the existing percent parameter.
Comment on lines
+106
to
+111
| const config = this.configService.getConfig(); | ||
| const interval = config.polling_interval ?? 2000; | ||
|
|
||
| if (config.profiles) { | ||
| this.profiles = { ...DEFAULT_PROFILES, ...config.profiles }; | ||
| } |
Contributor
There was a problem hiding this comment.
Reset this.profiles on every start().
If one run loaded custom profiles and a later config removes them, this service keeps the old entries because the field is only reassigned when config.profiles is truthy. The curve engine can keep applying deleted profiles until process restart.
Suggested fix
const config = this.configService.getConfig();
const interval = config.polling_interval ?? 2000;
-
- if (config.profiles) {
- this.profiles = { ...DEFAULT_PROFILES, ...config.profiles };
- }
+ this.profiles = { ...DEFAULT_PROFILES, ...(config.profiles ?? {}) };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`
around lines 106 - 111, In start(), always reset this.profiles based on the
current config so stale custom profiles are not retained: call
this.configService.getConfig(), then set this.profiles = { ...DEFAULT_PROFILES,
...(config.profiles ?? {}) } (or set to DEFAULT_PROFILES when config.profiles is
falsy) instead of only assigning when config.profiles is truthy; update the
start() method in fan-curve.service.ts to perform this unconditional assignment
before proceeding.
Comment on lines
+155
to
+205
| const readings = await this.hwmonService.readAll(); | ||
|
|
||
| const isOvertemp = await this.safetyService.checkTemperatureSafety(tempMetrics.sensors); | ||
| if (isOvertemp) { | ||
| return; | ||
| } | ||
|
|
||
| const isFanFailure = await this.safetyService.checkFanFailure(readings); | ||
| if (isFanFailure || this.safetyService.isInEmergencyMode()) { | ||
| return; | ||
| } | ||
|
|
||
| for (const zone of this.activeZones) { | ||
| const profile = this.profiles[zone.profile]; | ||
| if (!profile) { | ||
| this.logger.warn(`Profile ${zone.profile} not found, skipping zone`); | ||
| continue; | ||
| } | ||
|
|
||
| const sensor = tempMetrics.sensors.find( | ||
| (s) => s.id === zone.sensor || s.name === zone.sensor | ||
| ); | ||
| if (!sensor) { | ||
| this.logger.debug(`Sensor ${zone.sensor} not found, skipping zone`); | ||
| continue; | ||
| } | ||
|
|
||
| const targetSpeed = this.interpolateSpeed(profile.curve, sensor.current.value); | ||
| const targetPwm = Math.round((targetSpeed / 100) * 255); | ||
|
|
||
| for (const fanId of zone.fans) { | ||
| try { | ||
| const fan = readings.find((r) => r.id === fanId); | ||
| if (!fan || !fan.hasPwmControl) { | ||
| this.logger.debug(`Fan ${fanId} not found or not controllable, skipping`); | ||
| continue; | ||
| } | ||
|
|
||
| await this.safetyService.captureState(fanId, fan.devicePath, fan.pwmNumber, fan); | ||
|
|
||
| const isCpuFan = fan.fanNumber === 1 || fan.name.toLowerCase().includes('cpu'); | ||
| const safePwm = isCpuFan | ||
| ? this.safetyService.validateCpuFanPwm(targetPwm) | ||
| : this.safetyService.validatePwmValue(targetPwm); | ||
|
|
||
| if (fan.pwmEnable !== 1) { | ||
| await this.hwmonService.setMode(fan.devicePath, fan.pwmNumber, 1); | ||
| } | ||
|
|
||
| await this.hwmonService.setPwm(fan.devicePath, fan.pwmNumber, safePwm); | ||
| } catch (err) { |
Contributor
There was a problem hiding this comment.
Automatic fan curves never touch IPMI-backed fans.
This loop reads the snapshot from HwmonService and sends all mode/PWM writes back through HwmonService, so fans exposed by IpmiFanService are visible in metrics but invisible to the curve engine. On IPMI-only systems, automatic mode effectively does nothing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts`
around lines 155 - 205, The curve loop currently only reads from hwmon
(this.hwmonService.readAll()) and always calls hwmon write methods, so
IPMI-exposed fans are ignored; fix by also fetching IPMI fan readings (e.g.
await this.ipmiFanService.readAll()) and merging them into the readings array
before the zone loop, and when writing, branch on the fan source (e.g.
fan.source === 'ipmi' or presence of an ipmi-specific id) to call
this.ipmiFanService.setMode/setPwm for IPMI fans and
this.hwmonService.setMode/setPwm for hwmon fans; keep safetyService.captureState
and PWM validation as-is but ensure you route writes to the correct service
based on the fan object.
Comment on lines
+131
to
+138
| if (state.pwmEnable === 1) { | ||
| await this.hwmonService.setPwm(state.devicePath, state.pwmNumber, state.pwmValue); | ||
| } | ||
| await this.hwmonService.restoreAutomatic( | ||
| state.devicePath, | ||
| state.pwmNumber, | ||
| state.pwmEnable | ||
| ); |
Contributor
There was a problem hiding this comment.
Manual-mode fans are still restored to automatic mode.
When state.pwmEnable === 1, this code reapplies the saved PWM and then calls restoreAutomatic(...). HwmonService.restoreAutomatic() coerces 1 to 2, so a fan that started in manual mode never gets its original mode back.
Suggested fix
try {
if (state.pwmEnable === 1) {
- await this.hwmonService.setPwm(state.devicePath, state.pwmNumber, state.pwmValue);
- }
- await this.hwmonService.restoreAutomatic(
- state.devicePath,
- state.pwmNumber,
- state.pwmEnable
- );
+ await this.hwmonService.setMode(state.devicePath, state.pwmNumber, 1);
+ await this.hwmonService.setPwm(
+ state.devicePath,
+ state.pwmNumber,
+ state.pwmValue
+ );
+ } else {
+ await this.hwmonService.restoreAutomatic(
+ state.devicePath,
+ state.pwmNumber,
+ state.pwmEnable
+ );
+ }
this.logger.log(
`Restored fan ${fanId} to enable=${state.pwmEnable}, pwm=${state.pwmValue}`
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts`
around lines 131 - 138, The code currently calls this.hwmonService.setPwm(...)
when state.pwmEnable === 1 and then always calls
this.hwmonService.restoreAutomatic(...), but restoreAutomatic coerces a value of
1 to 2 so a fan that was originally in manual mode never gets restored
correctly; change the logic in the block around state.pwmEnable to skip calling
this.hwmonService.restoreAutomatic(state.devicePath, state.pwmNumber,
state.pwmEnable) when state.pwmEnable === 1 (i.e., only call restoreAutomatic
for non-manual modes) or alternatively ensure restoreAutomatic receives the
original mode without coercion—locate the handling in fan-safety.service.ts
referencing state.pwmEnable, this.hwmonService.setPwm, and
this.hwmonService.restoreAutomatic and update the conditional so manual-mode
(pwmEnable === 1) is preserved.
Comment on lines
+159
to
+170
| const readings = await this.hwmonService.readAll(); | ||
| for (const reading of readings) { | ||
| if (reading.hasPwmControl) { | ||
| try { | ||
| await this.captureState(reading.id, reading.devicePath, reading.pwmNumber, reading); | ||
| await this.hwmonService.setMode(reading.devicePath, reading.pwmNumber, 1); | ||
| await this.hwmonService.setPwm(reading.devicePath, reading.pwmNumber, 255); | ||
| } catch (err) { | ||
| this.logger.error(`Failed emergency full-speed for ${reading.id}: ${err}`); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Emergency full-speed skips IPMI-controlled fans.
The emergency path only iterates HwmonService.readAll(). If a board exposes controllable fans through IpmiFanService, the critical-temperature override leaves those fans untouched, which breaks the safety guarantee this feature advertises.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts`
around lines 159 - 170, The emergency full-speed loop only handles hwmon fans
and skips IPMI-controlled fans; update the emergency path in
fan-safety.service.ts to also query this.ipmiFanService.readAll() (or equivalent
IPMI enumerator) and perform the same captureState/override steps for those
entries: call captureState with the IPMI fan identifier, then use the IPMI
service methods to switch control into manual/full-speed (e.g.,
this.ipmiFanService.setMode or this.ipmiFanService.setFullSpeed / setPwm
depending on API) inside the existing try/catch and log failures the same way as
for hwmon fans. Ensure you reference and handle the IPMI fan fields analogous to
reading.id, reading.devicePath, and reading.pwmNumber so IPMI-controlled fans
are included in the critical-temperature override.
Comment on lines
+91
to
+102
| export class FanZoneConfig { | ||
| @Field(() => [String]) | ||
| @IsString({ each: true }) | ||
| fans!: string[]; | ||
|
|
||
| @Field(() => String) | ||
| @IsString() | ||
| sensor!: string; | ||
|
|
||
| @Field(() => String) | ||
| @IsString() | ||
| profile!: string; |
Contributor
There was a problem hiding this comment.
Zone validation still allows configs the runtime cannot use.
fans can be empty, and sensor / profile can be empty strings here. That means updateFanControlConfig can persist a zone like { fans: [], sensor: '', profile: '' } and then start the curve engine with a zone it can never drive. Please add non-empty/cardinality checks on both the persisted model and the input DTO (@ArrayMinSize(1) / @IsNotEmpty() or equivalent).
Also applies to: 231-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts`
around lines 91 - 102, The FanZoneConfig model allows empty zones; add
validation to prevent empty arrays/strings by decorating fans with
`@ArrayMinSize`(1) (in addition to `@IsString`({ each: true })) and decorating
sensor and profile with `@IsNotEmpty`() (or `@MinLength`(1)) so they cannot be empty
strings; also apply the same decorators to the corresponding input DTO class
used for updates (the fan-zone input DTO used by updateFanControlConfig) and
import the validators from class-validator so persisted configs and incoming
DTOs cannot be { fans: [], sensor: '', profile: '' }.
Comment on lines
+43
to
+84
| async setFanSpeed( | ||
| @Args('input', { type: () => SetFanSpeedInput }) input: SetFanSpeedInput | ||
| ): Promise<boolean> { | ||
| const config = this.configService.getConfig(); | ||
| if (!config.control_enabled) { | ||
| throw new Error('Fan control is not enabled. Enable it in config first.'); | ||
| } | ||
|
|
||
| if (this.safetyService.isInEmergencyMode()) { | ||
| throw new Error('System is in emergency mode. Fan control is locked.'); | ||
| } | ||
|
|
||
| const readings = await this.hwmonService.readAll(); | ||
| const fan = readings.find((r) => r.id === input.fanId); | ||
| if (!fan) { | ||
| throw new Error(`Fan ${input.fanId} not found`); | ||
| } | ||
| if (!fan.hasPwmControl) { | ||
| throw new Error(`Fan ${input.fanId} does not support PWM control`); | ||
| } | ||
|
|
||
| await this.safetyService.captureState(fan.id, fan.devicePath, fan.pwmNumber); | ||
|
|
||
| const isCpuFan = fan.fanNumber === 1 || fan.name.toLowerCase().includes('cpu'); | ||
| if (isCpuFan) { | ||
| this.logger.debug( | ||
| `Fan ${input.fanId} identified as CPU fan via heuristic (fanNumber=${fan.fanNumber}, name=${fan.name})` | ||
| ); | ||
| } | ||
| const safePwm = isCpuFan | ||
| ? this.safetyService.validateCpuFanPwm(input.pwmValue) | ||
| : this.safetyService.validatePwmValue(input.pwmValue); | ||
|
|
||
| const currentMode = pwmEnableToControlMode(fan.pwmEnable); | ||
| if (currentMode !== FanControlMode.MANUAL) { | ||
| await this.hwmonService.setMode(fan.devicePath, fan.pwmNumber, 1); | ||
| } | ||
|
|
||
| await this.hwmonService.setPwm(fan.devicePath, fan.pwmNumber, safePwm); | ||
| this.logger.log(`Set fan ${input.fanId} PWM to ${safePwm} (requested: ${input.pwmValue})`); | ||
|
|
||
| return true; |
Contributor
There was a problem hiding this comment.
Manual overrides are not durable while curve control is active.
These mutations write directly to hwmon but leave the fan's zone membership unchanged. If the target fan is still part of an active zone, the running curve engine will re-apply its computed PWM on the next tick and can immediately undo the manual speed/mode change. Please either reject manual overrides for zoned fans while automatic control is running, or detach/reconfigure that fan's zone before returning success.
Also applies to: 92-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`
around lines 43 - 84, setFanSpeed currently writes PWM/mode directly via
hwmonService (setMode/setPwm) after captureState but ignores whether the fan
belongs to an active zone, so the curve engine can immediately overwrite the
manual change; update setFanSpeed (and the analogous code around lines 92-123)
to first check zone membership and active control: use the zone management API
(e.g., ZoneService.isFanInActiveZone(fan.id) or similar) and either reject the
mutation with a clear error when the fan is in an active automatic zone, or
programmatically remove/detach the fan from that zone (call
ZoneService.detachFanFromZone(fan.id) or ZoneService.updateZone to exclude it)
before changing mode/PWM, then proceed with safetyService.captureState and
hwmonService.setMode/setPwm; ensure you reference pwmEnableToControlMode and
FanControlMode.MANUAL logic after handling the zone state so the manual override
is durable.
Comment on lines
+201
to
+218
| const zones = config.zones ?? []; | ||
| const existingZoneIdx = zones.findIndex((z) => z.fans.includes(input.fanId)); | ||
| if (existingZoneIdx >= 0) { | ||
| zones[existingZoneIdx].profile = input.profileName; | ||
| if (input.temperatureSensorId) { | ||
| zones[existingZoneIdx].sensor = input.temperatureSensorId; | ||
| } | ||
| } else { | ||
| if (!input.temperatureSensorId) { | ||
| throw new Error( | ||
| 'temperatureSensorId is required when assigning a profile to a fan not already in a zone' | ||
| ); | ||
| } | ||
| zones.push({ | ||
| fans: [input.fanId], | ||
| sensor: input.temperatureSensorId, | ||
| profile: input.profileName, | ||
| }); |
Contributor
There was a problem hiding this comment.
This rewrites the whole zone, not just the addressed fan.
existingZoneIdx is found by fan membership, but the update mutates the zone's shared profile and sensor. For any multi-fan zone, calling setFanProfile for one fan silently changes every sibling fan in that zone, which contradicts the mutation contract. Split the fan into its own zone here, or reject per-fan updates against shared zones.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts`
around lines 201 - 218, The current logic in the resolver uses existingZoneIdx
(found by zones.findIndex(z => z.fans.includes(input.fanId))) and then mutates
zones[existingZoneIdx].profile and .sensor, which unintentionally changes all
fans in a multi-fan zone; update the handler in the function that applies
setFanProfile so that if the located zone's fans array has length > 1 you either
(A) split the single fan out: remove input.fanId from
zones[existingZoneIdx].fans and push a new zone object { fans: [input.fanId],
sensor: input.temperatureSensorId, profile: input.profileName } (ensuring
temperatureSensorId exists), or (B) reject the operation by throwing an explicit
error when trying to set a per-fan profile on a shared zone; implement one of
these behaviors around the zones / existingZoneIdx branch so you no longer
overwrite the shared zone profile/sensor for sibling fans.
Comment on lines
+131
to
+141
| return Object.entries(allProfiles).map(([name, profileConfig]) => | ||
| Object.assign(new FanProfile(), { | ||
| name, | ||
| description: profileConfig.description, | ||
| curvePoints: (profileConfig.curve ?? []).map((point) => ({ | ||
| temperature: point.temp ?? 0, | ||
| speed: point.speed ?? 0, | ||
| })), | ||
| minSpeed: 20, | ||
| maxSpeed: 100, | ||
| }) |
Contributor
There was a problem hiding this comment.
Expose the stored profile fields instead of hard-coding them.
CreateFanProfileInput accepts temperatureSensorId, minSpeed, and maxSpeed, but this mapper drops temperatureSensorId and always returns 20/100. Custom profiles won't round-trip through the API with their actual settings.
Suggested fix
return Object.entries(allProfiles).map(([name, profileConfig]) =>
Object.assign(new FanProfile(), {
name,
description: profileConfig.description,
+ temperatureSensorId: profileConfig.temperatureSensorId,
curvePoints: (profileConfig.curve ?? []).map((point) => ({
temperature: point.temp ?? 0,
speed: point.speed ?? 0,
})),
- minSpeed: 20,
- maxSpeed: 100,
+ minSpeed: profileConfig.minSpeed ?? 20,
+ maxSpeed: profileConfig.maxSpeed ?? 100,
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return Object.entries(allProfiles).map(([name, profileConfig]) => | |
| Object.assign(new FanProfile(), { | |
| name, | |
| description: profileConfig.description, | |
| curvePoints: (profileConfig.curve ?? []).map((point) => ({ | |
| temperature: point.temp ?? 0, | |
| speed: point.speed ?? 0, | |
| })), | |
| minSpeed: 20, | |
| maxSpeed: 100, | |
| }) | |
| return Object.entries(allProfiles).map(([name, profileConfig]) => | |
| Object.assign(new FanProfile(), { | |
| name, | |
| description: profileConfig.description, | |
| temperatureSensorId: profileConfig.temperatureSensorId, | |
| curvePoints: (profileConfig.curve ?? []).map((point) => ({ | |
| temperature: point.temp ?? 0, | |
| speed: point.speed ?? 0, | |
| })), | |
| minSpeed: profileConfig.minSpeed ?? 20, | |
| maxSpeed: profileConfig.maxSpeed ?? 100, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.service.ts`
around lines 131 - 141, The mapper currently hard-codes minSpeed/maxSpeed
(20/100) and drops temperatureSensorId, so stored custom profiles won't
round-trip; update the mapping in the Object.entries(allProfiles).map callback
that constructs new FanProfile() to copy profileConfig.minSpeed,
profileConfig.maxSpeed, and profileConfig.temperatureSensorId (with sensible
fallbacks like profileConfig.minSpeed ?? 20 and profileConfig.maxSpeed ?? 100)
instead of using fixed literals, keeping the existing curvePoints mapping.
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.
Summary
This Implements a fan control system for the Unraid API, enabling real-time fan monitoring and control via Unraid API.
Closes this open feature bounty #1598
What's Included
Fan Monitoring
Fan Control
setFanSpeed,setFanMode,setFanProfile, andcreateFanProfileSafety
Architecture
MetricsResolveralongside CPU, memory, and temperatureHwmonService(sysfs) andIpmiFanService(IPMI)FanControlConfigServicefor persistent configuration viaConfigFilePersisterFanCurveServicefor temperature-linked speed interpolationFanSafetyServicefor safety guards and failure detectionFiles Changed
api/src/unraid-api/graph/resolvers/metrics/fancontrol/MetricsResolver,MetricsModule, andMetricsmodelFAN_METRICSpubsub channelTesting
Summary by CodeRabbit
New Features
Tests
Chores