From 538687b45636615d7067c8cd63c7d1449496bd71 Mon Sep 17 00:00:00 2001 From: tzuohann <2057796+tzuohann@users.noreply.github.com> Date: Fri, 19 Jun 2026 21:56:39 -0400 Subject: [PATCH] check_config: assert INI datatypes before comparing limits A user who puts an inline comment on a limit line -- e.g. "MAX_LIMIT = 100 # travel", easy to do without carefully reading the manual -- is currently served a very confusing error. The check reports a phantom limit violation like "[JOINT_0]MAX_LIMIT < [AXIS_X]MAX_LIMIT (100 < {100 # travel})", which appears to claim 100 < 100 and points at the wrong problem entirely. The cause: the [JOINT_n]/[AXIS_L] limit check compares values with Tcl > / <, which silently falls back to *string* comparison when a value isn't a valid number (the stray comment text makes it a string). The result then depends on trailing whitespace rather than the numbers. A duplicated key behaved the same way, since the multi-valued list was compared as a string. Add assert_ini_datatypes(), run once right after parse_ini, before any comparison. For each [JOINT_n]/[AXIS_L] item it validates the value LinuxCNC actually uses -- the first occurrence (IniFile::Find defaults to num=1; the manual, 'The INI File' -> 'Comments', likewise states the first value is used): - numeric items (limits, vels, accels, scales, PID terms, home vels, ...) must pass `string is double` (accepts ints and reals); - enum items must match a fixed set, case-insensitively, mirroring LinuxCNC's own caseless parsers: TYPE -> {LINEAR ANGULAR} (IniFile::mapJointType) bools -> {TRUE YES 1 ON FALSE NO 0 OFF} (IniFile::convertBool) - unknown items are left untouched. A non-numeric value whose text contains '#' or ';' additionally reports that inline comments are not allowed and cites the manual section, so the user is pointed at the real mistake. Duplicate (multi-valued) [JOINT_]/[AXIS_] keys are not fatal -- LinuxCNC uses the first occurrence -- so they emit a warning and the array is collapsed to that first value so the downstream limit comparison uses it too. This subsumes the warn-only warn_for_multiple_ini_values, which is removed. Co-Authored-By: Claude Opus 4.8 --- lib/hallib/check_config.tcl | 88 ++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/lib/hallib/check_config.tcl b/lib/hallib/check_config.tcl index d18b833f8e9..ae893bbe4a8 100644 --- a/lib/hallib/check_config.tcl +++ b/lib/hallib/check_config.tcl @@ -139,6 +139,67 @@ proc validate_identity_kins_limits {} { return $emsg } ;# validate_identity_kins_limits +proc assert_ini_datatypes {} { + # Datatype assertions, run right after parse_ini, before any comparison. + # numeric: must be a real number (covers integers too: string is double) + set numeric { + MIN_LIMIT MAX_LIMIT MAX_VELOCITY MAX_ACCELERATION MAX_JERK + FERROR MIN_FERROR BACKLASH + P I D FF0 FF1 FF2 BIAS DEADBAND MAX_OUTPUT MAX_ERROR + INPUT_SCALE OUTPUT_SCALE SCALE + HOME HOME_OFFSET HOME_SEARCH_VEL HOME_LATCH_VEL HOME_FINAL_VEL + HOME_SEQUENCE STEPGEN_MAXVEL STEPGEN_MAXACCEL + } + # enum: must match a fixed set (case-insensitive, mirroring LinuxCNC's + # caseless maps mapJointType / convertBool) + set bool {TRUE YES 1 ON FALSE NO 0 OFF} + array set enum [list \ + TYPE {LINEAR ANGULAR} \ + HOME_USE_INDEX $bool \ + HOME_IGNORE_LIMITS $bool \ + HOME_IS_SHARED $bool \ + HOME_INDEX_NO_ENCODER_RESET $bool \ + VOLATILE_HOME $bool \ + LOCKING_INDEXER $bool \ + ] + set emsg "" + foreach g [info globals] { + if {![string match JOINT_* $g] && ![string match AXIS_* $g]} continue + foreach item [array names ::$g] { + set val [set ::${g}($item)] + # LinuxCNC reads the first occurrence (IniFile::Find defaults to num=1), + # so a duplicate is not fatal. Validate the first occurrence; if there is + # more than one, warn and collapse the array to it so the downstream limit + # comparison uses the same value LinuxCNC would. + set use [lindex $val 0] + if {[llength $val] > 1} { + lappend ::wmsg "Multiple values for \[$g\]$item: <$val> (first value is used)" + set ::${g}($item) $use + } + if {[lsearch -exact $numeric $item] >= 0} { + if {![string is double -strict $use]} { + set m "\[$g\]$item must be numeric, got: <$use>" + if {[string match {*[#;]*} $use]} { + append m "\n Inline comments are not allowed: '#' and ';' start a comment" + append m "\n only at the start of a line, not after a value. Move the comment" + append m "\n to its own line. See the LinuxCNC manual, \"The INI File\" ->" + append m "\n \"Comments\": https://linuxcnc.org/docs/stable/html/config/ini-config.html#_comments" + } + lappend emsg $m + } + } elseif {[info exists enum($item)]} { + if {[lsearch -nocase $enum($item) $use] < 0} { + lappend emsg "\[$g\]$item must be one of {$enum($item)}, got: <$use>" + } + } + } + } + if {"$emsg" != ""} { + if {[info exists ::wmsg]} {warnings $::wmsg} + err_exit $emsg + } +} ;# assert_ini_datatypes + proc check_extrajoints {} { if ![info exists ::EMCMOT(EMCMOT)] return if {[string first motmod $::EMCMOT(EMCMOT)] <= 0} return @@ -158,30 +219,6 @@ proc check_extrajoints {} { } } ;#check_extrajoints -proc warn_for_multiple_ini_values {} { - set sections [info globals] - set sections_to_check {JOINT_ AXIS_} - - foreach section $sections { - set enforce 0 - foreach csection $sections_to_check { - if {[string first $csection $section"] >= 0} { - set enforce 1 - break - } - } - if !$enforce continue - foreach name [array names ::$section] { - set gsection ::$section - set val [set [set gsection]($name)] - set len [llength $val] - if {$len > 1} { - lappend ::wmsg "Unexpected multiple values \[$section\]$name: $val" - } - } - } -} ;# warn_for_multiple_ini_values - #---------------------------------------------------------------------- # begin package require Linuxcnc ;# parse_ini @@ -196,6 +233,8 @@ if ![file readable $inifile] { } parse_ini $inifile +assert_ini_datatypes + check_mandatory_items set kins_kinematics [lindex $::KINS(KINEMATICS) end] @@ -230,7 +269,6 @@ switch $::kins(module) { } check_extrajoints -warn_for_multiple_ini_values #parray ::kins set emsg [validate_identity_kins_limits] consistent_coords_for_trivkins $::kins(coordinates)