Skip to content

halcompile: support names= for count_function components - #4313

Open
grandixximo wants to merge 1 commit into
LinuxCNC:masterfrom
grandixximo:issue4303-countfunction-names
Open

halcompile: support names= for count_function components#4313
grandixximo wants to merge 1 commit into
LinuxCNC:masterfrom
grandixximo:issue4303-countfunction-names

Conversation

@grandixximo

@grandixximo grandixximo commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #4303.

Components that use option count_function get their instance count from get_count() instead of a count= module parameter. The code generator also omitted the names= module parameter and parsing for these components, so loadrt ... names=... failed with:

Unknown parameter `names=...'

Affected components: carousel, multiswitch, bldc, serport, pcl720, mesa_7i65.

Change src/hal/utils/halcompile.g so count_function components still declare and parse names=. If names are supplied, use them as instance prefixes; otherwise fall back to get_count() and the usual numeric names.

Tested:

  • loadrt carousel names=car
  • loadrt carousel names=ca,cb pockets=8,12
  • loadrt multiswitch names=ms cfg=4
  • loadrt bldc names=bl cfg=h
  • loadrt serport names=sp io=0x3f8
  • default loadrt carousel, loadrt multiswitch cfg=4, loadrt bldc cfg=h

@BsAtHome

BsAtHome commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

But isn't the problem that with count_function you have a user-supplied count function? You actually do not know what you are supposed to have in names because you are not counting names. The user is counting and you have no control over that.

@grandixximo
grandixximo force-pushed the issue4303-countfunction-names branch from d233823 to 0c619b1 Compare August 1, 2026 08:46
@grandixximo

Copy link
Copy Markdown
Contributor Author

@BsAtHome You are right. With count_function the count is determined by get_count(), so names= should only supply the instance names for that count, not replace the count.

I updated the patch to do exactly that:

  • call get_count() first;
  • parse the names= list and count the entries;
  • if the length matches get_count(), export the instances with the supplied names;
  • otherwise fail with a clear error like names= count (2) does not match carousel count (1);
  • if no names= is given, keep the existing numeric naming.

Tested on carousel, multiswitch, bldc, and serport with matching names, mismatching names, and default loads. The PR branch is force-pushed with the revised commit.

@grandixximo
grandixximo force-pushed the issue4303-countfunction-names branch from 0c619b1 to 051f53a Compare August 1, 2026 09:18
Comment thread src/hal/utils/halcompile.g Outdated
Comment thread src/hal/utils/halcompile.g
@andypugh

andypugh commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

I can definitely see an application for multiple instances of "thermistor" (and possibly hal_input but that isn't a .comp).
I am away from the LinuxCNC machines at the moment so don't actually know how they currently work for multiple instances.

@BsAtHome

BsAtHome commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

But thermistor does a FOR_ALL_INSTS() {} loop. So it should already support multiple counts/names.

@grandixximo

Copy link
Copy Markdown
Contributor Author

@andypugh Before this PR, userspace components without count_function support names= but one load gives one instance. Multiple instances mean multiple processes (loadusr -W thermistor names=bed, loadusr -W thermistor names=hotend, ...). hal_input is a singleton C component. For modbus you would run one process per port.

This PR only affects userspace components that also set option count_function; thermistor and hal_input do not, so they are unchanged. The hypothetical use case is a single userspace process that derives instance count from hardware/config via get_count() and exports named instances (vfd.spindle_main, vfd.spindle_aux, or modbus.port0, modbus.port1) without spawning multiple processes. Whether that is worth supporting is the open question Bertho raised.

@grandixximo

Copy link
Copy Markdown
Contributor Author

@BsAtHome @andypugh You are right, thermistor already supports multiple instances via count= or names= because it does not use count_function. My "one load gives one instance" was wrong.

Corrected framing:

  • Userspace components without count_function already support multiple instances via count= or names=.
  • This PR only affects userspace components that use count_function; for those, names= previously failed with "Unknown parameter".
  • The open question is whether userspace + count_function + names= is a combination worth keeping, or whether it should be a generator error.

@grandixximo

Copy link
Copy Markdown
Contributor Author

@BsAtHome @andypugh Rethinking the userspace + count_function path: it does not cleanly mirror either existing pattern.

Userspace without count_function parses names= into an array and exports one instance per supplied name. RT with count_function parses a comma string and requires the count to match get_count(). The hybrid I added keeps the array parse but adds a get_count() check, which fits neither model.

Options are:

  1. Restructure userspace names= parsing to match RT (risky for existing userspace components).
  2. Let names= override get_count() in userspace (Bertho already flagged this as wrong).
  3. Make userspace + count_function a generator error and keep the RT-only fix.

I lean toward option 3. The actual issue #4303 is about RT components like carousel; the userspace path was speculative. Erroring avoids inconsistency and prevents users from writing ambiguous configs.

@grandixximo
grandixximo force-pushed the issue4303-countfunction-names branch from 051f53a to e104fb3 Compare August 1, 2026 11:36
Components using option count_function declared no names= module param,
so loadrt names=... failed with "Unknown parameter". Declare names and
parse it; use supplied names when given, otherwise fall back to
get_count().

Fixes LinuxCNC#4303.
@grandixximo
grandixximo force-pushed the issue4303-countfunction-names branch from e104fb3 to a1124f6 Compare August 1, 2026 11:44
@grandixximo

grandixximo commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Removed the userspace names= support I added. count_function derives the instance count at load time and it can change across restarts, while HAL names= are fixed.When count_function is set, names= should error out, because it is dangerous to support setting it; users can already load multiple userspace components with names= when count_function is not used.

@BsAtHome was correct all along, I just didn't see it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

carousel: loadrt with names= fails (rtapi_app exits without becoming ready)

3 participants