xhc-hb04: rename dashed ini identifiers, migrate old configs in update_ini - #4601
grandixximo wants to merge 2 commits into
Conversation
The new ini parser restricts section and tag identifiers to letters, digits and underscore, not starting with a digit. The historic parser had no identifier character restrictions, so the xhc-hb04 pendant files used dashes freely and no longer parse: - the [XHC_HB04_BUTTONS] keys in the sample configs (start-pause, goto-zero, ...) - the [XHC-HB04] section of the pendant's own layout cfg files, which the component parses with the same IniFile class, making it exit at startup Rename the identifiers to use underscores: the INI button keys in the sample configs, the [XHC_HB04] section in the shipped layout cfg files and in the component's hardcoded section name, plus documentation. The HAL pin names are unchanged and keep their dashes (xhc-hb04.button-start-pause). The xhc-hb04.tcl script maps the underscored INI button names back to the dashed pin names, so existing user HAL files that reference the pins keep working.
| if ! inivalue "$INIFILE" > /dev/null; then | ||
| # Older configs may use dashed identifiers (e.g. the xhc-hb04 pendant | ||
| # button names), which the ini parser no longer accepts. Try to rewrite | ||
| # them with underscores and test again. | ||
| update_ini --fix-identifiers "$INIFILE" | ||
| inivalue "$INIFILE" > /dev/null || { echo "E: The INI-file contains errors that need to be fixed."; exit 1; } | ||
| fi |
There was a problem hiding this comment.
I'm not entirely sure if it is a good idea to do this here. We already have a check for VERSION that invokes the updater. However, the update_ini tool is not ready. See #3704 (still WIP).
There was a problem hiding this comment.
Thanks for having a look. I see the overlap with #3704, but I think the VERSION mechanism can not cover this case. The launcher runs inivalue as a parse check before the [EMC]VERSION check, and a config with dashed identifiers fails that check, so the script exits before update_ini is ever invoked. The same applies to #3704's rewritten update_ini: it also starts by parsing the file with linuxcnc.ini(), which is exactly what fails on these files. A fix for dashed identifiers has to happen as a text pass before parsing, and something has to invoke it on the parse-failure path. The parser-side APIs like lineof() can not help here, precisely because the file does not parse.
On the collision with #3704: my update_ini change is additive and self-contained (two new functions, one CLI flag, six lines at the call site), so it should rebase cleanly in either direction. Happy to rebase onto #3704 if that lands first, or to split this PR so the rename half (which fixes the in-tree configs that are broken on master today) goes ahead and the migration half is coordinated with #3704. How should I proceed?
There was a problem hiding this comment.
Does your addition handle #INCLUDE?
There was a problem hiding this comment.
Yes. The pass follows #INCLUDE recursively with the same rules as the parser (paths relative to the including file, tilde expansion, recursion and depth guards) and rewrites included files too, each keeping its own .bak. The [XHC_HB04_CONFIG]layout scan for a custom pendant cfg also covers included files, since that section can live in an .inc (ja_tests/xyzx_mpg does). The new dash-identifiers test exercises exactly this path.
3742410 to
0ac0834
Compare
Configs using dashed identifiers (old xhc-hb04 pendant setups) can not be parsed at all any more, so the version-triggered update path never gets a chance to run: the launcher exits on the inivalue parse check before reaching the [EMC]VERSION check that invokes update_ini. Add a text pass to update_ini that runs before parsing and independent of [EMC]VERSION: section and tag identifiers containing dashes (but otherwise valid) are rewritten with underscores in the ini file, in its #INCLUDE'd files and in a custom xhc-hb04 layout cfg found via a loose scan of [XHC_HB04_CONFIG]layout. Every modified file keeps a .bak copy and every rename is listed. The pass is available standalone as update_ini --fix-identifiers. The launcher runs this mode when the inivalue parse check fails and retries parsing afterwards, so old pendant configs migrate themselves on first boot. Identifiers that are invalid even without the dash (leading dash or digit) are left alone and still fail with the original error. Add a tests/update_ini/dash-identifiers test covering the ini file, an included file, a custom layout cfg, the .bak copies and the invalid-identifier cases, and extend the inivalue test with plain dash-rejection cases.
0ac0834 to
b70f9ee
Compare
2 participants
The new ini parser restricts identifiers to letters, digits and underscore. The historic parser had no character restrictions, so the xhc-hb04 pendant files used dashes and no longer parse: the
[XHC_HB04_BUTTONS]keys in the sample configs (start-pause,goto-zero, ...) and the[XHC-HB04]section of the pendant's own layout cfg files, which the component parses with the same IniFile class.Per the 2026-09-27 dev meeting (conclusion in #4566), the strict identifier rule stays and the in-tree files and code are updated instead. This replaces the accept-the-dash approach of #4573.
First commit renames the dashed identifiers to underscores:
[XHC-HB04]=>[XHC_HB04]in the component's hardcoded section name, the shipped layout cfg files and the docsstart-pause=>start_pause,goto-zero=>goto_zero, ...)xhc-hb04.tclmaps the underscored INI button names back to the dashed HAL pin names (xhc-hb04.button-start-pauseetc.), which are unchanged, so existing user HAL files referencing the pins keep workingSecond commit adds the migration for existing user configs. A dashed file can not be parsed at all, and the launcher exits on the inivalue parse check before the
[EMC]VERSIONcheck that invokes update_ini, so the version-triggered path never fires. Instead update_ini gets a content-triggered text pass that runs before parsing: dashed (but otherwise valid) section and tag identifiers are rewritten with underscores in the ini file, its#INCLUDE'd files and a custom layout cfg found via a loose scan of[XHC_HB04_CONFIG]layout. Every modified file keeps a.bakcopy and every rename is listed. The launcher runs this asupdate_ini --fix-identifierswhen the parse check fails and retries afterwards, so old pendant configs migrate themselves on first boot. Identifiers that are invalid even without the dash (leading dash or digit) are left alone and fail with the original error. NoTHIS_VERSIONbump.One limitation: HAL files referencing
[XHC_HB04_BUTTONS]start-pausethrough halcmd ini substitution are not rewritten. The xhc-hb04.tcl mechanism does not use such references, so this only affects hand-written HAL files.Behavior note: the migration rewrites the user's config files on first boot without asking for confirmation. The renames are listed on stdout and a
.bakcopy is kept for every modified file, but a GUI user starting from a menu may never see that output. The version-based conversion path, in contrast, asks with a confirmation dialog and refuses to run without an X display. I kept the identifier fix automatic because the config does not start at all without it, the change is small and reversible, and the meeting asked for an automated update. @BsAtHome: do you want a confirmation dialog here when a display is available, like the version-based conversion has?Testing: new
tests/update_ini/dash-identifiers(ini file + included file + custom cfg + .bak + invalid identifiers), the inivalue test gains plain dash-rejection cases, full inifile suite passes 11/11. All four xhc-hb04 sample configs boot with the pendant pins and button nets present (including thestd_start_pausepath), and a config reverted to the old dashed form boots through the launcher and migrates itself.