Skip to content

Refactor WebServer settings update logic and improve SSE client handling#106

Merged
harpua555 merged 1 commit intomainfrom
dev
Mar 1, 2026
Merged

Refactor WebServer settings update logic and improve SSE client handling#106
harpua555 merged 1 commit intomainfrom
dev

Conversation

@harpua555
Copy link
Owner

@harpua555 harpua555 commented Mar 1, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved settings update request handling to prevent conflicts during concurrent operations.
    • Enhanced detection ratio threshold configuration to accept flexible input formats with automatic normalization.
    • Optimized server connection limit handling with improved error logging.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026

Walkthrough

The PR adds a test output ignore rule, modifies the /update_settings handler to prevent overwriting pending settings, improves SSE client rejection with explicit closure, and refactors detection_ratio_threshold handling to support both 0-1 and 0-100 value ranges.

Changes

Cohort / File(s) Summary
Build and Test Configuration
.gitignore
Added ignore rule for /test/tsan_output.txt in test output section.
Settings and Client Management
src/WebServer.cpp
Modified /update_settings handler to prevent overwriting pending settings during concurrent updates. Enhanced SSE client rejection with explicit logging and connection closure. Refactored detection_ratio_threshold handling to accept both 0-1 and 0-100 ranges, normalizing to internal integer representation.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: refactoring settings update logic in WebServer and improving SSE client handling, which are the primary modifications shown in src/WebServer.cpp.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/WebServer.cpp (1)

529-537: Clamp normalized detection_ratio_threshold to a valid range before save.

After normalization, values outside 0..100 are still possible. Clamping here makes the endpoint robust regardless of downstream setter behavior.

♻️ Suggested patch
         if (jsonObj.containsKey("detection_ratio_threshold"))
         {
             float threshold = jsonObj["detection_ratio_threshold"].as<float>();
             // Accept legacy 0.0-1.0 ratio payloads as well as 0-100 percent.
             if (threshold > 0.0f && threshold <= 1.0f)
             {
                 threshold *= 100.0f;
             }
+            if (threshold < 0.0f)
+            {
+                threshold = 0.0f;
+            }
+            else if (threshold > 100.0f)
+            {
+                threshold = 100.0f;
+            }
             settingsManager.setDetectionRatioThreshold(static_cast<int>(threshold + 0.5f));
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/WebServer.cpp` around lines 529 - 537, The code that reads
"detection_ratio_threshold" normalizes values in WebServer.cpp but doesn't clamp
them, so values outside 0..100 can be saved; after the normalization step for
threshold (the local variable named threshold derived from
jsonObj["detection_ratio_threshold"]) clamp threshold to the inclusive range
0.0f..100.0f before calling
settingsManager.setDetectionRatioThreshold(static_cast<int>(threshold + 0.5f));
to ensure the setter always receives a valid percentage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/WebServer.cpp`:
- Around line 529-537: The code that reads "detection_ratio_threshold"
normalizes values in WebServer.cpp but doesn't clamp them, so values outside
0..100 can be saved; after the normalization step for threshold (the local
variable named threshold derived from jsonObj["detection_ratio_threshold"])
clamp threshold to the inclusive range 0.0f..100.0f before calling
settingsManager.setDetectionRatioThreshold(static_cast<int>(threshold + 0.5f));
to ensure the setter always receives a valid percentage.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2df4d7e and 705f1c7.

📒 Files selected for processing (2)
  • .gitignore
  • src/WebServer.cpp

@harpua555 harpua555 merged commit 705f1c7 into main Mar 1, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant