Skip to content

Commit 01ea56e

Browse files
Copilotdorkmo
andcommitted
Address code review feedback: improve validation and fix HTML
- Fix nested label in Relay Outputs (now using div.field) - Fix report time parsing to correctly handle hour 0 (midnight) - Add NaN validation to alarm value parsing - Enhance relay trigger validation to also check if alarm value is set - Enhance SMS trigger validation to also check if alarm value is set Co-authored-by: dorkmo <[email protected]>
1 parent f4b6159 commit 01ea56e

File tree

1 file changed

+20
-14
lines changed

1 file changed

+20
-14
lines changed

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -776,14 +776,14 @@ static const char CONFIG_GENERATOR_HTML[] PROGMEM = R"HTML(
776776
<option value="manual_reset">Stay On Until Manual Server Reset</option>
777777
</select>
778778
</label>
779-
<label class="field"><span>Relay Outputs</span>
779+
<div class="field"><span>Relay Outputs</span>
780780
<div style="display: flex; gap: 12px; padding: 8px 0;">
781781
<label style="display: flex; align-items: center; gap: 4px;"><input type="checkbox" class="relay-1" value="1"> R1</label>
782782
<label style="display: flex; align-items: center; gap: 4px;"><input type="checkbox" class="relay-2" value="2"> R2</label>
783783
<label style="display: flex; align-items: center; gap: 4px;"><input type="checkbox" class="relay-3" value="4"> R3</label>
784784
<label style="display: flex; align-items: center; gap: 4px;"><input type="checkbox" class="relay-4" value="8"> R4</label>
785785
</div>
786-
</label>
786+
</div>
787787
</div>
788788
</div>
789789
@@ -981,8 +981,8 @@ static const char CONFIG_GENERATOR_HTML[] PROGMEM = R"HTML(
981981
const reportTimeValue = document.getElementById('reportTime').value || '05:00';
982982
// Validate time format is HH:MM, fallback to 05:00 if not
983983
const timeParts = reportTimeValue.split(':');
984-
const reportHour = timeParts.length === 2 ? (parseInt(timeParts[0], 10) || 5) : 5;
985-
const reportMinute = timeParts.length === 2 ? (parseInt(timeParts[1], 10) || 0) : 0;
984+
const reportHour = timeParts.length === 2 ? (isNaN(parseInt(timeParts[0], 10)) ? 5 : parseInt(timeParts[0], 10)) : 5;
985+
const reportMinute = timeParts.length === 2 ? (isNaN(parseInt(timeParts[1], 10)) ? 0 : parseInt(timeParts[1], 10)) : 0;
986986
987987
const config = {
988988
site: document.getElementById('siteName').value.trim(),
@@ -1048,10 +1048,16 @@ static const char CONFIG_GENERATOR_HTML[] PROGMEM = R"HTML(
10481048
// Only include alarm values if alarm section is visible and individual alarms are enabled
10491049
if (alarmSectionVisible && (highAlarmEnabled || lowAlarmEnabled)) {
10501050
if (highAlarmEnabled && highAlarmValue !== '') {
1051-
tank.highAlarm = parseFloat(highAlarmValue);
1051+
const highAlarmFloat = parseFloat(highAlarmValue);
1052+
if (!isNaN(highAlarmFloat)) {
1053+
tank.highAlarm = highAlarmFloat;
1054+
}
10521055
}
10531056
if (lowAlarmEnabled && lowAlarmValue !== '') {
1054-
tank.lowAlarm = parseFloat(lowAlarmValue);
1057+
const lowAlarmFloat = parseFloat(lowAlarmValue);
1058+
if (!isNaN(lowAlarmFloat)) {
1059+
tank.lowAlarm = lowAlarmFloat;
1060+
}
10551061
}
10561062
tank.alarmSms = true;
10571063
} else {
@@ -1087,10 +1093,10 @@ static const char CONFIG_GENERATOR_HTML[] PROGMEM = R"HTML(
10871093
if (relayMask === 0) {
10881094
alert("You have set a relay target but have not selected any relay outputs for " + name + ". The configuration will be incomplete.");
10891095
}
1090-
// Validation: warn if relay trigger is set to an alarm type that is not enabled
1091-
if ((relayTrigger === 'high' && !highAlarmEnabled) ||
1092-
(relayTrigger === 'low' && !lowAlarmEnabled)) {
1093-
alert(`Warning: Relay for ${name} is set to trigger on "${relayTrigger}" alarm, but that alarm type is not enabled.`);
1096+
// Validation: warn if relay trigger is set to an alarm type that is not enabled or value is missing
1097+
if ((relayTrigger === 'high' && (!highAlarmEnabled || highAlarmValue === '')) ||
1098+
(relayTrigger === 'low' && (!lowAlarmEnabled || lowAlarmValue === ''))) {
1099+
alert(`Warning: Relay for ${name} is set to trigger on "${relayTrigger}" alarm, but that alarm type is not fully configured (either not enabled or value is missing).`);
10941100
}
10951101
}
10961102
}
@@ -1111,10 +1117,10 @@ static const char CONFIG_GENERATOR_HTML[] PROGMEM = R"HTML(
11111117
trigger: smsTrigger, // 'any', 'high', or 'low'
11121118
message: smsMessage || 'Tank alarm triggered'
11131119
};
1114-
// Validation: warn if SMS trigger is set to an alarm type that is not enabled
1115-
if ((smsTrigger === 'high' && !highAlarmEnabled) ||
1116-
(smsTrigger === 'low' && !lowAlarmEnabled)) {
1117-
alert(`Warning: SMS Alert for ${name} is set to trigger on "${smsTrigger}" alarm, but that alarm type is not enabled.`);
1120+
// Validation: warn if SMS trigger is set to an alarm type that is not enabled or value is missing
1121+
if ((smsTrigger === 'high' && (!highAlarmEnabled || highAlarmValue === '')) ||
1122+
(smsTrigger === 'low' && (!lowAlarmEnabled || lowAlarmValue === ''))) {
1123+
alert(`Warning: SMS Alert for ${name} is set to trigger on "${smsTrigger}" alarm, but that alarm type is not fully configured (either not enabled or value is missing).`);
11181124
}
11191125
}
11201126
}

0 commit comments

Comments
 (0)