Fix saving calibration #1028
Fix saving calibration #1028
Conversation
📝 WalkthroughWalkthroughThe changes update the RSSI prediction calculation in the optimization model and introduce several new optimizer classes implementing the IOptimizer interface. Updates include switching from an asynchronous background evaluation to a synchronous update in the optimization runner, refining configuration parameter limits, and enhancing the calibration UI with dynamic node settings retrieval and usage in RSSI calculations. Stability-related functions have been removed from the UI component. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as OptimizationRunner
participant Optimizer as Optimizer (Joint/TwoStage/WeightedJointRx)
participant Config as Configuration
participant Logger as Logger
Runner->>Optimizer: Optimize(OptimizationSnapshot)
Optimizer->>Config: Retrieve configuration values
Optimizer->>Optimizer: Evaluate objective function (Nelder-Mead simplex)
Optimizer->>Logger: Log debug info and errors
Optimizer-->>Runner: Return OptimizationResults
Runner->>Runner: Update best result synchronously
sequenceDiagram
participant Page as Calibration Page
participant API as Node Settings API
participant Cache as nodeSettings Cache
Page->>API: fetchNodeSettings(nodeId)
API-->>Page: Return Node Setting Data
Page->>Cache: Update nodeSettings cache
Page->>Page: Recalculate refRssi using fetched absorption value
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
DTTerastar
temporarily deployed
to
CI - release environment
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/Optimizers/WeightedJointRxAdjAbsorptionOptimizer.cs (2)
19-27: Consider null-checking optimization config more explicitly.
Currently, if_state.Configor_state.Config.Optimizationis unexpectedlynull, the code proceeds with default bounds but doesn't explicitly inform the user. An explicit null check (with a warning log or early return) could improve clarity and prevent silent fallback.
34-36: Reduce log verbosity for repeated per-node debug statements.
Excessively granular logs inside nested loops and iteration steps (e.g., lines 35, 36, 64–65) may generate large log volumes in production, impacting performance. Consider using a higher log level or aggregating data in fewer log entries.Also applies to: 65-65
src/Optimizers/JointRxAdjAbsorptionOptimizer.cs (1)
80-83: Log the exception details for non-convergence.
CatchingMaximumIterationsExceptionis good, but currently the exception (ex) isn’t logged. Includingex.Messageorex.ToString()helps diagnose convergence failures.catch (MaximumIterationsException ex) { Log.Error("Non-convergence for {0}: {1}", g.Key.Id, ex.Message); }src/Optimizers/TwoStageRxAdjAbsorptionOptimizer.cs (2)
41-46: Validate threshold for “close nodes” in Stage 1.
You currently filter nodes bydistance <= 5.0. While this may be a sensible default, consider making that threshold configurable or documented, as different deployments might need different “close node” ranges.
32-110: Explore merging two-stage logic with other optimizers or supporting concurrency.
The two-step approach is conceptually distinct, but still shares substantial overlaps (Nelder-Mead usage, repeated distance calculations, bounds checks, etc.) with the other optimizers. Introducing concurrency for each stage or reusing shared calculation utilities might yield performance benefits and easier maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Models/OptimizationResults.cs(2 hunks)src/Optimizers/JointRxAdjAbsorptionOptimizer.cs(1 hunks)src/Optimizers/OptimizationRunner.cs(2 hunks)src/Optimizers/TwoStageRxAdjAbsorptionOptimizer.cs(1 hunks)src/Optimizers/WeightedJointRxAdjAbsorptionOptimizer.cs(1 hunks)src/config.example.yaml(1 hunks)src/ui/src/routes/calibration/[id]/+page.svelte(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Deploy to Docker add-on with tag
- GitHub Check: Analyze (csharp)
🔇 Additional comments (13)
src/Models/OptimizationResults.cs (2)
2-2: Added Serilog import for improved loggingThe addition of Serilog supports better logging capabilities in the optimization process.
26-26: Improved signal propagation modeling with updated path loss exponentChanging the default path loss exponent from 3 to 2.7 more accurately reflects real-world signal propagation in typical indoor environments, likely improving RSSI prediction accuracy.
src/Optimizers/OptimizationRunner.cs (2)
26-28: Documentation of removed background taskGood practice to document the removal of functionality with explanatory comments.
54-56: Simplified optimization flow with synchronous baseline calculationThe change from an asynchronous background re-evaluation task to a synchronous calculation in the main optimization loop eliminates potential race conditions and ensures evaluations always use current data.
src/config.example.yaml (1)
33-34: Adjusted RSSI adjustment limits to improve calibrationThe changes to RSSI adjustment limits (increasing minimum from -15 to -5, and maximum from 20 to 25) better accommodate the variety of real-world signal environments encountered by users, likely addressing the "Cannot Save Calibration" issue by allowing a more appropriate range of values.
src/ui/src/routes/calibration/[id]/+page.svelte (8)
7-7: Updated type imports to support node settingsAdding the NodeSetting type import enables the new node settings functionality in the calibration UI.
15-17: Added node settings cache for optimizationCreating a cache for node settings avoids redundant API calls and improves performance during calibration.
30-42: Implemented node settings fetch with proper error handlingThe fetchNodeSettings function properly retrieves node-specific settings from the API with appropriate error handling and maintains reactivity through reference updates.
133-146: Dynamic node settings retrieval during calibrationThe enhanced reactive statement now fetches node settings on demand while setting node inclusion defaults, ensuring all required data is available for accurate RSSI calculations.
255-260: Improved RSSI reference calculation using node-specific absorptionThe updated calculation now uses the actual absorption value from each node's settings instead of a hard-coded default, making the calibration more accurate for each specific environment.
460-464: Conditional display of estimated distance based on node-specific absorptionThe UI now correctly uses node-specific absorption values when displaying estimated distances, providing more accurate information to users during calibration.
469-469: Simplified RSSI displayImproved the RSSI display with better formatting for readability.
472-472: Enhanced RSSI@1m calculation with node-specific absorptionThe estimated RSSI@1m calculation now correctly uses each node's specific absorption value, ensuring consistency with the backend calculation model.
|
Fixes #1027 |
Fixes "Cannot Save Calibration" when used in HA AddOn
Summary by CodeRabbit
New Features
Refactor