Repository navigation
move Halui into task - #4644
move Halui into task#4644rene-dev wants to merge 6 commits into
Conversation
grandixximo
left a comment
grandixximo
left a comment
There was a problem hiding this comment.
This is a good move: halui in task removes a whole process and its NML round trip with no behavior change, and the new tests/halui suite pinning down the quirks is exactly what a port like this needs. I support merging it.
One question on the config sweep: did these get missed, or was leaving them deliberate? They still set HALUI = halui:
- configs/sim/axis/db_demo/base.inc (pulled in via #INCLUDE by db_nonran.ini and db_ran.ini)
- configs/sim/axis/ja_tests/xz/xzbase.inc
- configs/sim/tklinuxcnc/trivkins/xzbase.inc
- configs/sim/touchy/ngcgui/pyngcgui_touchy_moveoff_es.txt
Harmless now that the setting is ignored, but they will confuse anyone grepping for how halui gets started.
The rest is inline.
| * state gating, same serial-number echo, same error reporting. | ||
| * | ||
| * Two producers use it: | ||
| * - ws_server.cc, from the websocket server thread |
There was a problem hiding this comment.
ws_server.cc does not exist anywhere in this tree. Is the websocket server a planned follow-up? As written, "Two producers use it" reads like there is a second producer today, but halui.cc is the only one.
| #include "motion/motion.h" // EMCMOT_ORIENT_* | ||
| #include "ini/inihal.hh" | ||
| #include "halui.hh" // the HAL user interface, in task | ||
| #include "cmd_queue.hh" // commands from ws_server and halui |
There was a problem hiding this comment.
Same question as in cmd_queue.hh: ws_server is not in the tree, so what does "commands from ws_server and halui" refer to?
| $(Q)$(CXX) -o $@ $(ULFLAGS) $^ -lfmt $(LDFLAGS) | ||
| TARGETS += ../bin/linuxcnclcd | ||
|
|
||
| ../bin/halui: $(call TOOBJS, $(HALUISRCS)) ../lib/liblinuxcnc.a ../lib/liblinuxcncini.so.1 ../lib/libnml.so.0 ../lib/liblinuxcnchal.so.0 ../lib/libtooldata.so.0 |
There was a problem hiding this comment.
Removing bin/halui breaks user configs that start it from a HAL file with loadusr halui -ini ..., which is the pattern the docs recommended for StepConf-generated configs, so it will exist in real user setups. The [HAL]HALUI = form degrades gracefully since the pins now always exist, but the loadusr form is a hard startup failure. Would a no-op halui shim be worth keeping for one release? It could print a warning that the pins are built into task now, so those users learn about the change instead of silently succeeding. At minimum I think this needs a migration note in the docs.
| the nml file specified in the INI, usually that file is in the same | ||
| folder as the INI, so it makes sense to run halui from that folder. | ||
| The pins are part of the LinuxCNC task controller and exist in every | ||
| configuration. There is nothing to load and nothing to configure: connect |
There was a problem hiding this comment.
Is "nothing to configure" quite right? [HALUI]MDI_COMMAND, [DISPLAY]MAX_FEED_OVERRIDE and the spindle override limits still configure halui. Maybe just "nothing to load"?
| ---- | ||
| There is nothing to set up: the pins exist in every configuration, created | ||
| by the task controller before the HAL files are read, so a HAL file can | ||
| connect them straight away. Pins nobody connects cost nothing. |
There was a problem hiding this comment.
Does this agree with the halui.cc header comment? There you write that a machine connecting none of the pins still pays for a pin scan every 20 ms. Negligible either way, but which one is it?
2 participants
there is really no reason for halui to be its own process, and go thru nml - it is much better to be integrated in task.
this is the next step to fully remove nml, the only place it is left now is the ui <-> task communication.