refactor: use JSON serialization for profiles by beenycool · Pull Request #29 · beenycool/AsusFanControl

beenycool · 2026-04-12T09:41:15Z

Summary

Add JsonPropertyName attributes to FanCurvePoint, FanProfile for compact JSON output
Add ToJson/FromJson methods to FanCurve and FanProfile classes
Update FanCurveEditor to use ToJson() for saving curve files
Migrate ProfileManager to JSON-based serialization for profiles

- Add JsonPropertyName attributes to FanCurvePoint, FanProfile for compact JSON - Add ToJson/FromJson methods to FanCurve and FanProfile - Update FanCurveEditor to use ToJson() for saving - Migrate ProfileManager to JSON-based serialization

coderabbitai · 2026-04-12T09:41:21Z

Warning

Rate limit exceeded

@beenycool has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 49 seconds before requesting another review.

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 49 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a8ba226d-78f3-4ad8-850d-fc607d3ccf93

📥 Commits

Reviewing files that changed from the base of the PR and between a469e19 and 77c47d2.

📒 Files selected for processing (3)

AsusFanControl.Core/FanCurve.cs
AsusFanControl.Core/ProfileManager.cs
AsusFanControlGUI/FanCurveEditor.cs

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch pr3-json-serialization

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

chatgpt-codex-connector · 2026-04-12T09:41:22Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

gemini-code-assist

Code Review

This pull request migrates the serialization of fan curves and profiles from custom string-based formats to JSON. While this modernizes the data handling, the review highlights critical concerns regarding backward compatibility, as existing user profiles and exported curves will fail to load without a migration path or fallback logic. Additionally, the Points property in FanCurve should remain an IReadOnlyList to correctly signal that the getter returns a snapshot, and error handling during deserialization should be enhanced to provide better diagnostic information instead of silently swallowing exceptions.

gemini-code-assist · 2026-04-12T09:42:39Z

+                try
+                {
+                    var data = JsonSerializer.Deserialize<List<FanProfile>>(serialized);
+                    if (data != null)
+                        _profiles.AddRange(data);
+                }
+                catch (JsonException)
                {
-                    var profile = FanProfile.FromString(entry);
-                    if (profile != null)
-                        _profiles.Add(profile);
+                    Debug.WriteLine("Settings are not valid JSON.");
                }


This refactor breaks existing profile storage. The LoadProfiles method will fail to parse the legacy pipe-delimited format (name|curve|processes), causing all existing user profiles to be lost upon upgrading to this version. A migration path is necessary to convert the old string-based settings into the new JSON format to ensure users do not lose their data.

gemini-code-assist · 2026-04-12T09:42:39Z

-            curve.SetPoints(points);
-            return curve;
-        }
+public static FanCurve FromString(string data) => FromJson(data);


This change breaks backward compatibility for existing fan curve files. The previous format was temperature:speed pairs separated by commas, while the new implementation expects JSON. Existing user-exported curve files will fail to load. You should implement a fallback to the legacy parsing logic if JSON deserialization fails. Additionally, this line is missing proper indentation.

gemini-code-assist · 2026-04-12T09:42:39Z


-        public IReadOnlyList<FanCurvePoint> Points
+    [JsonPropertyName("p")]
+    public List<FanCurvePoint> Points


The Points property returns a new list instance (ToList()) in its getter, but the property type is now List<FanCurvePoint>. This is misleading because callers might expect that modifying the returned list (e.g., via .Add()) would update the FanCurve state, which it won't due to the cloning logic. Since you've added a setter for JSON support, consider changing the public type back to IReadOnlyList<FanCurvePoint> to correctly convey that the getter provides a snapshot.

public IReadOnlyList<FanCurvePoint> Points

gemini-code-assist · 2026-04-12T09:42:39Z

+            catch
+            {
+                return new FanCurve();
            }


Swallowing all exceptions during JSON deserialization and returning an empty FanCurve makes it difficult to diagnose corrupted configuration files. Consider logging the exception or allowing it to propagate if the input is non-empty but invalid.

kilo-code-bot · 2026-04-12T09:43:42Z

 using System.Linq;
+using System.Text.Json.Serialization;
+using System.Text.Json;



Testing line 6

kilo-code-bot · 2026-04-12T09:44:35Z

+public class FanCurve
+{
+    private readonly object _lock = new object();
+    private List<FanCurvePoint> _points = new List<FanCurvePoint>();


WARNING: Points property exposes List<FanCurvePoint> instead of IReadOnlyList<FanCurvePoint>. This could allow external modification without proper locking, leading to thread-safety issues. Consider using IReadOnlyList<FanCurvePoint> for the property type and keeping the setter private or protected.

kilo-code-bot · 2026-04-12T09:44:35Z

@@ -85,15 +94,20 @@ public int GetTargetSpeed(int currentTemp)
            return sortedPoints[count - 1].Speed;
        }



WARNING: FromJson returns a new empty FanCurve on any deserialization error, which can hide data corruption. Consider returning null or throwing an exception, and let the caller handle the error appropriately.

kilo-code-bot · 2026-04-12T09:44:35Z

+            catch
+            {
+                return new FanCurve();
            }


SUGGESTION: The FromString method is now just a wrapper for FromJson. Consider removing it entirely or keeping it for backward compatibility with a migration path (e.g., try parsing old format first).

kilo-code-bot · 2026-04-12T09:44:35Z

@@ -60,12 +60,15 @@ public void LoadProfiles(string serialized)
                _activeProfileName = null;


WARNING: LoadProfiles only catches JsonException. If other exceptions occur (e.g., ArgumentNullException), they will slip through. Consider catching Exception or at least documenting why only JsonException is caught.

kilo-code-bot · 2026-04-12T09:44:36Z

@@ -20,26 +27,19 @@ public FanProfile(string name, FanCurve curve, IEnumerable<string> triggerProces
            TriggerProcesses = triggerProcesses.ToList();
        }



SUGGESTION: FanProfile.FromJson returns null on error, which is better than returning an empty object. However, consider logging the error or including exception details for debugging.

kilo-code-bot · 2026-04-12T09:44:45Z

Code Review Summary

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	4
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
`AsusFanControl.Core/FanCurve.cs`	28	`Points` property exposes `List<FanCurvePoint>` instead of `IReadOnlyList<FanCurvePoint>`. This could allow external modification without proper locking, leading to thread-safety issues.
`AsusFanControl.Core/FanCurve.cs`	96	`FromJson` returns a new empty `FanCurve` on any deserialization error, which can hide data corruption.
`AsusFanControl.Core/ProfileManager.cs`	60	`LoadProfiles` only catches `JsonException`. If other exceptions occur, they will slip through.
`AsusFanControl.Core/FanCurve.cs`	110	`FromString` is now just a wrapper for `FromJson`. Consider removing it or adding backward compatibility.

SUGGESTION

File	Line	Issue
`AsusFanControl.Core/ProfileManager.cs`	29	`FanProfile.FromJson` returns `null` on error. Consider logging the error for debugging.

Other Observations (not in diff)

This refactoring completely replaces the old string serialization format with JSON, which is a breaking change. Existing user profiles and settings saved in the old format will no longer work. Consider adding a migration path (e.g., try parsing old format first) or maintaining backward compatibility.

Additionally, in Form1.cs (not part of this PR):

Line 81 still uses FanCurve.FromString() which expects the old format.
Line 118 still uses currentFanCurve.ToString() instead of ToJson().
These will cause runtime errors when loading saved settings.

Files Reviewed (5 files)

AsusFanControl.Core/FanCurve.cs
AsusFanControl.Core/ProfileManager.cs
AsusFanControl.GUI/FanCurveEditor.cs
AsusFanControl.GUI/Form1.cs (not in diff)

_{Reviewed by trinity-large-thinking · 393,636 tokens}

refactor: use JSON serialization for profiles

77c47d2

- Add JsonPropertyName attributes to FanCurvePoint, FanProfile for compact JSON - Add ToJson/FromJson methods to FanCurve and FanProfile - Update FanCurveEditor to use ToJson() for saving - Migrate ProfileManager to JSON-based serialization

gemini-code-assist Bot reviewed Apr 12, 2026

View reviewed changes

kilo-code-bot Bot reviewed Apr 12, 2026

View reviewed changes

		@@ -85,15 +94,20 @@ public int GetTargetSpeed(int currentTemp)
		return sortedPoints[count - 1].Speed;
		}

		@@ -60,12 +60,15 @@ public void LoadProfiles(string serialized)
		_activeProfileName = null;

		@@ -20,26 +27,19 @@ public FanProfile(string name, FanCurve curve, IEnumerable<string> triggerProces
		TriggerProcesses = triggerProcesses.ToList();
		}

Conversation

beenycool commented Apr 12, 2026

Summary

Uh oh!

coderabbitai Bot commented Apr 12, 2026

Rate limit exceeded

Uh oh!

chatgpt-codex-connector Bot commented Apr 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Overview

WARNING

SUGGESTION

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kilo-code-bot Bot commented Apr 12, 2026 •

edited

Loading