Repository navigation
Clamp values loaded from /prefs.json, ignore nan and inf - #3797
Open
marcelverdult wants to merge 1 commit into
Open
marcelverdult wants to merge 1 commit into
marcelverdult wants to merge 1 commit into
Conversation
marcelverdult
added a commit
to marcelverdult/MeshCore_Refactor
that referenced
this pull request
Nov 10, 2026
docs: triage of upstream candidates and notes for meshcore-dev#3797
loadPrefs() reads /prefs.json and uses whatever is in it. Only the legacy binary loader (loadPrefsInt) clamps values, so a hand edited or externally written file can put freq, bw, sf, cr, tx_power_dbm, the delay factors or the bridge settings outside the limits the node otherwise enforces. These values go straight into the radio setup. Changes: - Move the clamping out of loadPrefsInt() into CommonCLI::sanitisePrefs() and call it from both loaders. The clamped fields and limits are the ones the legacy loader already used, with the airtime_factor change below. - airtime_factor is clamped to 0..99 instead of 0..9. "set dutycycle 1" stores 100/1 - 1 = 99, so a limit of 9 would reset any duty cycle below 10 percent on the next boot, now also on the /prefs.json path. "set af" still accepts 0..9 only; that is not touched here. - ConfigSerializer no longer stores a float or double that is not finite. constrain() cannot handle NaN (both comparisons are false), so a token like nan, inf or 1e40 now leaves the field at the default set before the file is read. The check uses the bits, not a comparison, because nRF52 builds use -Ofast. For floats it runs after the narrowing, since a double can overflow to infinity as a float. - loadPrefs() logs a parse error from loadSerial(). The file is still applied and now sanitised. The ConfigSerializer change also applies to the companion NodePrefs, which uses the same serializer.
marcelverdult
force-pushed
the
upstream-candidate/prefs-json-range-check
branch
from
November 12, 2026 12:19
31ee36d to
c16115a
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nodes that boot from
/prefs.jsonuse whatever the file contains. Only the legacy binary loader (loadPrefsInt) clamps values. A file that was edited by hand or written by another tool can putfreq,bw,sf,cr,tx_power_dbm, the delay factors or the bridge settings outside the limits the node otherwise enforces, and these values go straight into the radio setup.This moves the clamping into
CommonCLI::sanitisePrefs()and calls it from both loaders. The fields and limits are the ones the legacy loader already used, with one exception, below.airtime_factor. The limit is now 0..99 instead of 0..9.
set dutycycle 1stores100/1 - 1 = 99, so a limit of 9 would reset any duty cycle below 10 percent at the next boot, and with this change that would now also happen on the/prefs.jsonpath.set afstill accepts 0..9 only; I left that alone.nan and inf.
constrain()cannot catch a NaN, since both of its comparisons are false. SoConfigSerializerno longer stores a float or double that is not finite: a token likenan,infor1e40leaves the field at the default the firmware set before reading the file. The check uses the bits and not a comparison, because nRF52 builds use-Ofast. For floats it runs after the narrowing, since a value that is finite as a double can overflow to infinity as a float. This also applies to the companionNodePrefs, which uses the same serializer.loadPrefs()now logs whenloadSerial()reports a parse error. A partially parsed file is still applied, and now sanitised.Not tested on a device with a hand written
/prefs.json.