Skip to content

Commit 27ffcdf

Browse files
Copilotdorkmo
andcommitted
Address code review feedback: add bounds validation and fix documentation
Co-authored-by: dorkmo <[email protected]>
1 parent 157e347 commit 27ffcdf

File tree

3 files changed

+9
-6
lines changed

3 files changed

+9
-6
lines changed

TankAlarm-112025-Client-BluesOpta/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ Used for sensors like the Dwyer 626-06-CB-P1-E5-S1 (0-5 PSI) mounted near the bo
110110
- Configuration:
111111
- `currentLoopType`: "pressure"
112112
- `sensorMountHeight`: 2.0
113-
- `maxValue`: 138.0 (or actual tank height if tank is smaller)
113+
- `maxValue`: 118.0 (tank height minus mount height: 120 - 2 = 118 inches)
114+
115+
> **Note:** For pressure sensors, set `maxValue` to the tank's usable height minus the sensor mount height. The implementation adds `sensorMountHeight` to the measured value, so `maxValue` should represent the height of liquid *above* the sensor, not the total tank height.
114116
115117
#### Ultrasonic Sensor (Top-Mounted)
116118
Used for sensors like the Siemens Sitrans LU240 mounted on top of the tank looking down.

TankAlarm-112025-Client-BluesOpta/TankAlarm-112025-Client-BluesOpta.ino

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -789,8 +789,8 @@ static bool loadConfigFromFlash(ClientConfig &cfg) {
789789
} else {
790790
cfg.tanks[i].currentLoopType = CURRENT_LOOP_PRESSURE; // Default: pressure sensor
791791
}
792-
// Load sensor mount height (for calibration)
793-
cfg.tanks[i].sensorMountHeight = t["sensorMountHeight"].is<float>() ? t["sensorMountHeight"].as<float>() : 0.0f;
792+
// Load sensor mount height (for calibration) - validate non-negative
793+
cfg.tanks[i].sensorMountHeight = t["sensorMountHeight"].is<float>() ? fmaxf(0.0f, t["sensorMountHeight"].as<float>()) : 0.0f;
794794
}
795795

796796
return true;
@@ -1332,9 +1332,9 @@ static void applyConfigUpdate(const JsonDocument &doc) {
13321332
gConfig.tanks[i].currentLoopType = CURRENT_LOOP_PRESSURE;
13331333
}
13341334
}
1335-
// Handle sensor mount height (for calibration)
1335+
// Handle sensor mount height (for calibration) - validate non-negative
13361336
if (t.containsKey("sensorMountHeight")) {
1337-
gConfig.tanks[i].sensorMountHeight = t["sensorMountHeight"].as<float>();
1337+
gConfig.tanks[i].sensorMountHeight = fmaxf(0.0f, t["sensorMountHeight"].as<float>());
13381338
}
13391339
}
13401340
}
@@ -1563,6 +1563,7 @@ static float readTankSensor(uint8_t idx) {
15631563
levelInches = rawLevel + cfg.sensorMountHeight;
15641564
// Clamp to valid range (sensorMountHeight to maxValue + sensorMountHeight)
15651565
if (levelInches < cfg.sensorMountHeight) levelInches = cfg.sensorMountHeight;
1566+
if (levelInches > cfg.maxValue + cfg.sensorMountHeight) levelInches = cfg.maxValue + cfg.sensorMountHeight;
15661567
}
15671568
return levelInches;
15681569
}

TankAlarm-112025-Server-BluesOpta/TankAlarm-112025-Server-BluesOpta.ino

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1056,7 +1056,7 @@ static const char CONFIG_GENERATOR_HTML[] PROGMEM = R"HTML(
10561056
} else if (type === 2) { // Current Loop (4-20mA)
10571057
heightField.style.display = 'flex';
10581058
heightLabel.textContent = 'Tank Height (in)';
1059-
heightTooltip.setAttribute('data-tooltip', 'Maximum height of liquid in the tank in inches. For ultrasonic sensors, this is the distance the sensor measures at full tank.');
1059+
heightTooltip.setAttribute('data-tooltip', 'Maximum height of liquid in the tank in inches. Meaning varies based on sensor subtype.');
10601060
digitalInfoBox.style.display = 'none';
10611061
currentLoopInfoBox.style.display = 'block';
10621062
switchModeField.style.display = 'none';

0 commit comments

Comments
 (0)