LucianoDev
c617854c09
Config system fixes: - Implement SavePump() — full XML serialization with insert/update by pump ID - Add CanBusParameter.ToPumpXml() for legacy P1-P6 pump param format - Fix LastRotationDirection never loaded in LoadSettings() - Add SaveAlarms() to ConfigurationService and IConfigurationService - Remove dead fields AppSettings.Clients and AppSettings.PumpIds PDF report redesign: - Professional layout with charts, verdict badges, and tolerance bands - Add ReportChartRenderer (SVG) and ReportTheme styling constants - Embed default_logo.png as fallback report logo Documentation: - Add gap analysis docs (config validation, ford unlock, missing features) - Update CLAUDE.md architecture, known gaps, and debt tracking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
77 lines
4.1 KiB
Markdown
77 lines
4.1 KiB
Markdown
# Gap: Configuration Validation & Migration
|
||
|
||
## Problem
|
||
The configuration system loads 7 XML files with no schema validation, no bounds checking, and silent error swallowing. Malformed files produce partial/default state with no user indication. There is no migration path from the old 32-CFG-file format.
|
||
|
||
## Critical Issues
|
||
|
||
### 1. SavePump() Stub
|
||
**File:** `Services/Impl/ConfigurationService.cs:191-194`
|
||
**Impact:** Pump definition edits (test phase enable/disable, parameter changes, new pumps) are lost on restart.
|
||
**Fix:** Implement full XML serialization mirroring the `LoadPump`/`ParsePumpElement` logic. Use `PumpDefinition.ToXml()`, `TestDefinition.ToXml()`, `PhaseDefinition.ToXml()`. Write to `pumps.xml` at the correct XPath location. Handle both insert (new pump) and update (existing pump) cases.
|
||
|
||
### 2. LastRotationDirection Never Loaded
|
||
**File:** `Services/Impl/ConfigurationService.cs` — `SaveSettings()` writes `<LastRotationDir>` but `LoadSettings()` never reads it.
|
||
**Fix:** Add `TryInt(r, "LastRotationDir", v => settings.LastRotationDirection = (short)v);` in `LoadSettings()`.
|
||
|
||
### 3. No SaveAlarms()
|
||
**File:** `Services/Impl/ConfigurationService.cs`
|
||
**Impact:** Alarm configuration changes cannot be persisted.
|
||
**Fix:** Add `SaveAlarms()` using `Alarm.ToXml()` for each alarm, write to `alarms.xml`.
|
||
|
||
### 4. SaveSettings() Side Effects
|
||
**File:** `Services/Impl/ConfigurationService.cs:101-102`
|
||
**Impact:** Calling `SaveSettings()` always also writes sensors.xml and clients.xml.
|
||
**Fix:** Split into `SaveSettings()`, `SaveSensors()`, `SaveClients()` called independently. Or document the coupling clearly.
|
||
|
||
## Bounds Validation Rules
|
||
|
||
Add these checks in the respective `FromXml`/`Load*` methods:
|
||
|
||
| Field | Location | Valid Range | Guard |
|
||
|-------|----------|-------------|-------|
|
||
| `byteh` / `bytel` | `CanBusParameter.FromXml`, `ParseParamElement` | 0–7 | Clamp or reject + log |
|
||
| `RefreshCanBusReadMs` | `LoadSettings` | >= 1 | Floor at 1 |
|
||
| `RefreshPumpParamsMs` | `LoadSettings` | >= 1 | Floor at 1 |
|
||
| `SecurityRpmLimit` | `LoadSettings` | 100–5000 | Clamp |
|
||
| `MaxPressureBar` | `LoadSettings` | 1–100 | Clamp |
|
||
| `PidP, PidI, PidD` | `LoadSettings` | >= 0 | Floor at 0 |
|
||
| `Relay.Bit` | `ParseRelayElement` | 0–63 | Reject + log |
|
||
| `SensorConfiguration.MaxVolt - MinVolt` | `GetValueFromRaw` | != 0 | Return 0 + log |
|
||
| P1–P6 denominator | `GetTransformResult` | != 0 | Return 0 + log |
|
||
|
||
## Locale-Safe Parsing
|
||
All `double.Parse` / `int.Parse` calls in XML loading must use `CultureInfo.InvariantCulture`. Known violations:
|
||
- `SensorConfiguration.FromXml` — `double.Parse(v)` with no culture (line ~78-83)
|
||
- `CanBusParameter.ParseDecimal` — already uses `InvariantCulture` (correct)
|
||
- `ConfigurationService.TryDouble` — uses `double.TryParse` with `InvariantCulture` (correct)
|
||
|
||
## Config Versioning Approach
|
||
Add a `version` attribute to each XML root element:
|
||
```xml
|
||
<Config version="2">...</Config>
|
||
<Pumps version="1">...</Pumps>
|
||
```
|
||
On load, check version. If missing or old, run migration logic. Bump version after migration. This enables safe format evolution.
|
||
|
||
## Error Reporting Strategy
|
||
Replace all `catch { }` and `catch (Exception) { }` blocks with:
|
||
```csharp
|
||
catch (Exception ex)
|
||
{
|
||
_log.Warning(LogId, $"Skipped malformed element '{elementName}': {ex.Message}");
|
||
}
|
||
```
|
||
Files affected:
|
||
- `ConfigurationService.cs` — `TryInt`, `TryDouble`, `TryBool`, `TryString` helpers (lines 621-646)
|
||
- `SensorConfiguration.cs` — `TryParse` wrapper (line 100-103)
|
||
|
||
## Dead Fields
|
||
`AppSettings.Clients` and `AppSettings.PumpIds` are never populated by `LoadSettings()` — they are dead fields. Either remove them or wire up the loading. Current live data paths: `ConfigurationService._clients` and `GetPumpIds()`.
|
||
|
||
## Old CFG → XML Migration Strategy
|
||
See `gap-pump-data-migration.md` for the full field mapping. In summary:
|
||
1. Parse each CFG file using the pipe-delimited format documented in `old_source/HerlicScripts/Program.cs`
|
||
2. Map CAN IDs, P1-P6 formulas, status bit definitions, and EEPROM passwords to the pumps.xml and status.xml schemas
|
||
3. Run once as a migration tool, not as runtime logic
|