HC_APTBS/docs/gap-config-validation.md

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:

<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:

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