Skip to content

Commit af2dcd6

Browse files
dkulpclaude
andcommitted
fix(settings): store fan trip temperatures with a decimal so F round-trips
Fan temperatures are stored in C because that is what the kernel wants, but a whole degree C cannot name every whole degree F: 85F rounds to 29C, which reads back as 84F. 60 of the 136 F values in the settable range drift a degree that way, so a value entered in F frequently -- but not always -- came back changed after a page reload. The saved value now carries one decimal place. A tenth of a degree C is 0.18F, so the worst-case storage error is 0.09F and every whole degree F survives the round trip; a value entered in C is still a whole number. Two consumers assumed an integer and had to be fixed with it: - applyThermalSettings() read the setting with getRawSettingInt(), whose hand-rolled digit loop does not stop at a decimal point and turns "29.4" into 2884. The kernel accepts the resulting 2884000-millidegree trip write, so the fan would simply have stopped turning on after the next boot. It now reads the raw string and parses a float. - SetGPIOFanProperties() built millidegrees for the gpio-fan overlay by appending "000" to the setting; it now multiplies. Displaying in C also sets step="any" on the input, since a value holding a tenth is invalid against the default step of 1. Existing whole-degree values stay valid and stable -- they just aren't exact until re-entered. Fixes #2840 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5c0620d commit af2dcd6

4 files changed

Lines changed: 48 additions & 17 deletions

File tree

src/boot/FPPINIT_Config.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <chrono>
1414
#include "fpp-json.h"
1515
#include <cctype>
16+
#include <cmath>
1617
#include <cstdio>
1718
#include <cstdlib>
1819
#include <cstring>
@@ -410,10 +411,18 @@ void applyThermalSettings(bool captureDefaults) {
410411
defaultsChanged = true;
411412
}
412413
}
413-
int temp = getRawSettingInt("FanTrip_" + key, -1);
414-
if (temp > 0) {
415-
printf("FPP - Setting fan trip point %s to %dC\n", key.c_str(), temp);
416-
PutFileContents(tripFile, std::to_string(temp * 1000));
414+
// Read as a string and parse a float: the setting holds degrees C to one
415+
// decimal place so that whole degrees F survive the conversion both ways
416+
// (see PutSetting() in www/api/controllers/settings.php). getRawSettingInt()
417+
// must not be used here - its hand-rolled digit loop does not stop at the
418+
// decimal point and would turn "29.4" into 2884.
419+
std::string tempStr;
420+
if (getRawSetting("FanTrip_" + key, tempStr)) {
421+
double temp = strtod(tempStr.c_str(), nullptr);
422+
if (temp > 0) {
423+
printf("FPP - Setting fan trip point %s to %.1fC\n", key.c_str(), temp);
424+
PutFileContents(tripFile, std::to_string((int) std::lround(temp * 1000.0)));
425+
}
417426
}
418427
});
419428
if (defaultsChanged) {

www/api/controllers/settings.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,15 @@ function PutSetting()
121121
$setting = params('SettingName');
122122

123123
if (($setting == 'GPIOFanTemperature' || str_starts_with($setting, 'FanTrip_')) && isset($settings['temperatureInF']) && $settings['temperatureInF'] == 1) {
124-
$value = round(($value - 32) * 5 / 9);
124+
// Store one decimal place, not a whole degree C. A whole degree C is too
125+
// coarse to name every whole degree F: 85F rounds to 29C, which reads back
126+
// as 84F, and 60 of the 136 values in range drift like that. A tenth of a
127+
// degree C is 0.18F, so the worst case error is 0.09F and every whole
128+
// degree F survives the round trip. Consumers of the stored value must
129+
// therefore parse a float - see applyThermalSettings() in
130+
// src/boot/FPPINIT_Config.cpp and SetGPIOFanProperties() in
131+
// www/common/settings.php. A value entered in C stays a whole number.
132+
$value = round(($value - 32) * 5 / 9, 1);
125133
}
126134

127135
// A failed write must not fall through to the apply/restart steps below:

www/common.php

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,7 @@ function PrintFanThermalSettings()
821821
'description' => $desc,
822822
'tip' => $tip,
823823
'type' => 'number',
824-
'default' => (int) round($currentTemp),
824+
'default' => round($currentTemp, 1),
825825
'min' => 20,
826826
'max' => 95,
827827
'step' => 1,
@@ -1096,13 +1096,25 @@ function PrintSetting($setting, $callback = '', $options = array(), $plugin = ''
10961096
$step = isset($s['step']) ? $s['step'] : 1;
10971097
$default = isset($s['default']) ? $s['default'] : "0";
10981098

1099-
if (($setting == 'GPIOFanTemperature' || str_starts_with($setting, 'FanTrip_')) && isset($settings['temperatureInF']) && $settings['temperatureInF'] == 1) {
1100-
$origVal = isset($settings[$setting]) ? $settings[$setting] : $default;
1101-
$settings[$setting] = round(($origVal * 9 / 5) + 32);
1102-
$min = round(($min * 9 / 5) + 32);
1103-
$max = round(($max * 9 / 5) + 32);
1104-
$default = round(($default * 9 / 5) + 32);
1105-
$suffix = ' F';
1099+
// Temperature settings are always stored in C because that is what the
1100+
// kernel wants, but may be entered and displayed in F. A whole number
1101+
// of C cannot represent every whole number of F, so the stored value is
1102+
// allowed a decimal (see PutSetting()); without it 60 of the 136 usable
1103+
// F values come back a degree lower after a save/reload round trip.
1104+
if ($setting == 'GPIOFanTemperature' || str_starts_with($setting, 'FanTrip_')) {
1105+
if (isset($settings['temperatureInF']) && $settings['temperatureInF'] == 1) {
1106+
$origVal = isset($settings[$setting]) ? $settings[$setting] : $default;
1107+
$settings[$setting] = round(($origVal * 9 / 5) + 32);
1108+
$min = round(($min * 9 / 5) + 32);
1109+
$max = round(($max * 9 / 5) + 32);
1110+
$default = round(($default * 9 / 5) + 32);
1111+
$suffix = ' F';
1112+
} else {
1113+
// Displaying C: the value may carry the tenth left behind by an
1114+
// earlier entry in F, which a step of 1 would flag as invalid.
1115+
// "any" still steps the spinner arrows by 1.
1116+
$s['step'] = 'any';
1117+
}
11061118
}
11071119

11081120
PrintSettingTextSaved($setting, $restart, $reboot, $max, $min, $plugin, $default, $callback, '', 'number', $s);

www/common/settings.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,14 @@ function SetGPIOFanProperties()
291291
{
292292
global $settings;
293293
$fanOn = ReadSettingFromFile('GPIOFan');
294-
$fanTemp = ReadSettingFromFile('GPIOFanTemperature') . "000";
294+
// The overlay wants millidegrees. Multiply rather than appending "000":
295+
// the setting is degrees C to one decimal place so that whole degrees F
296+
// survive the conversion both ways (see PutSetting() in
297+
// www/api/controllers/settings.php), and "29.4" . "000" is not a number.
298+
$fanTempSetting = ReadSettingFromFile('GPIOFanTemperature');
299+
$fanTemp = is_numeric($fanTempSetting) ? (string) ((int) round(floatval($fanTempSetting) * 1000)) : '70000';
295300
$pfx = "";
296301

297-
if ($fanTemp == '000') {
298-
$fanTemp = '70000';
299-
}
300302
if ($fanOn == '0') {
301303
$pfx = "#";
302304
}

0 commit comments

Comments
 (0)