feat(fancontrol): Implement fan control system#1954
Open
ruaan-deysel wants to merge 7 commits intounraid:mainfrom
Open
feat(fancontrol): Implement fan control system#1954ruaan-deysel wants to merge 7 commits intounraid:mainfrom
ruaan-deysel wants to merge 7 commits intounraid:mainfrom
Conversation
…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.
7 tasks
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 30 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
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, metrics integration, 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: captureState / validatePwmValue
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 | ✅ 3✅ Passed checks (3 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
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts
Outdated
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -0,0 +1,46 @@ | |||
| import { Field, Float, InputType, Int } from '@nestjs/graphql'; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1954 +/- ##
==========================================
+ Coverage 51.73% 51.86% +0.12%
==========================================
Files 1026 1038 +12
Lines 70718 71795 +1077
Branches 7881 8075 +194
==========================================
+ Hits 36586 37234 +648
- Misses 34009 34438 +429
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…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.
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
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts
Outdated
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-safety.service.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts
Show resolved
Hide resolved
Comment on lines
+192
to
+210
| 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`); | ||
| } | ||
|
|
||
| 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 { | ||
| zones.push({ | ||
| fans: [input.fanId], | ||
| sensor: input.temperatureSensorId ?? '', | ||
| profile: input.profileName, | ||
| }); |
Contributor
There was a problem hiding this comment.
Reject profile assignments that can never drive hardware.
This mutation only checks that the fan exists. A non-PWM fan, or a first-time assignment with no temperatureSensorId, still gets persisted even though FanCurveService.applyCurves() later skips both cases. Fail fast here instead of saving inert zones.
🛠️ Suggested change
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`);
+ }
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 an unzoned fan'
+ );
+ }
zones.push({
fans: [input.fanId],
- sensor: input.temperatureSensorId ?? '',
+ sensor: input.temperatureSensorId,
profile: input.profileName,
});
}📝 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
| 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`); | |
| } | |
| 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 { | |
| zones.push({ | |
| fans: [input.fanId], | |
| sensor: input.temperatureSensorId ?? '', | |
| profile: input.profileName, | |
| }); | |
| 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`); | |
| } | |
| 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 an unzoned fan' | |
| ); | |
| } | |
| zones.push({ | |
| fans: [input.fanId], | |
| 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 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.
Comment on lines
+229
to
+245
| async createFanProfile( | ||
| @Args('input', { type: () => CreateFanProfileInput }) input: CreateFanProfileInput | ||
| ): Promise<boolean> { | ||
| const config = this.configService.getConfig(); | ||
| const profiles = config.profiles ?? {}; | ||
|
|
||
| const defaultProfiles = this.fanCurveService.getProfiles(); | ||
| if (defaultProfiles[input.name]) { | ||
| throw new Error( | ||
| `Cannot overwrite built-in profile "${input.name}". Choose a different name.` | ||
| ); | ||
| } | ||
|
|
||
| profiles[input.name] = { | ||
| description: input.description, | ||
| curve: input.curvePoints.map((p) => ({ temp: p.temperature, speed: p.speed })), | ||
| }; |
Contributor
There was a problem hiding this comment.
The extra profile fields are silently discarded.
CreateFanProfileInput exposes temperatureSensorId, minSpeed, and maxSpeed, but this write path persists only description and curve. Those values can never survive the mutation, and FanProfile.minSpeed / maxSpeed then have no backing source on readback. Either store them in FanProfileConfig and map them through, or remove them from the public schema until they are implemented.
🤖 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 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.
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.
…onfiguration limits
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
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts
Outdated
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fan-curve.service.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol-config.model.ts
Show resolved
Hide resolved
api/src/unraid-api/graph/resolvers/metrics/fancontrol/fancontrol.resolver.ts
Outdated
Show resolved
Hide resolved
…for improved safety and error handling
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
…ate profile names
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
Chores