Skip to content

hal: Migrate homecomp is_module pin declaration to bool - #4329

Merged
BsAtHome merged 1 commit into
LinuxCNC:masterfrom
grandixximo:homecomp-is-module-bool
Aug 3, 2026
Merged

hal: Migrate homecomp is_module pin declaration to bool#4329
BsAtHome merged 1 commit into
LinuxCNC:masterfrom
grandixximo:homecomp-is-module-bool

Conversation

@grandixximo

Copy link
Copy Markdown
Contributor

One leftover from the getter/setter migration: homecomp.comp still declares its dummy is_module pin with the legacy bit type, while the rest of the component was migrated in f107cd3 (#4296).

Found with a tree-wide halcompupdate --check sweep (from #4256) against current master; this was the only genuine declaration leftover among all in-tree .comp files not already covered by open migration PRs.

Change: pin out bit is_module=1 => pin out bool is_module=1. Builds clean.

Leftover from f107cd3: the is_module dummy pin was still declared
with the legacy bit type while the rest of homecomp uses the new
getter/setter API.
@BsAtHome

BsAtHome commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

The build is filtered out in the Submakefile together with tpcomp.comp. These are supposed to be examples of plug-able homing and TP. Neither works in practice or has never been tested to be actually functional afaik. Both modules need to hook into a lot of internals and that is quite contrary the modularization idea.

But sure, we still need to update it, even if it doesn't fix things ;-)

@grandixximo

grandixximo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

This is all that came up, while re-reviewing looking for sets before create, that's a good sign...

@BsAtHome

BsAtHome commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This is all that came up, while re-reviewing looking for sets before create, that's a good sign...

Yes it is!

And, have you noticed, no one complained about breakage :-)
Maybe because it is holiday season.

@BsAtHome
BsAtHome merged commit d30c9e4 into LinuxCNC:master Aug 3, 2026
31 of 32 checks passed
@Sigma1912

Copy link
Copy Markdown
Contributor

These are supposed to be examples of plug-able homing and TP. Neither works in practice or has never been tested to be actually functional afaik.

Note that 'homecomp.comp' has definitely been used and has been proven to work.
As an example:
https://forum.linuxcnc.org/ethercat/52968-help-needed-cracking-the-code-on-linuxcnc-servo-homing-setup?start=30#303183

'tpcomp.comp' on the other hand has apparently proven less useful.

@BsAtHome

BsAtHome commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Interesting...
However, I'm sure it can no longer compile today on master. The homecomp.comp tries to include a lot of internal headers that are no longer exposed. All of that needs to be untangled before it can work again. It starts with including motion.h and homing.h, but these include more and more of non-public stuff.

WRT tpcomp.comp, this should be removed IMO. The first refactor step is to get TP moved out of RT and make a formal TP input/output specification that can be exposed. Then we can have proper plug-able TPs. However, moving it out of RT is a big undertaking and a lot of work.

@Sigma1912

Sigma1912 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

However, I'm sure it can no longer compile today on master.

If it is to be removed then we'll want to also remove it from the documentation:

* `HOMEMOD =` _alternate_homing_module_ [home_parms=value]
The HOMEMOD variable is optional. If specified, use a specified (user-built) module instead of the default (homemod).
Module parameters (home_parms) may be included if supported by the named module.
The setting may be overridden from the command line using the -m option ($ linuxcnc -h).

@BsAtHome

BsAtHome commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

We could also fix it :-)

But that will take some time. Besides, we'd need a real example to add, not a mostly empty shell.

@Sigma1912

Copy link
Copy Markdown
Contributor

We could also fix it :-)

That would be very much my preference. :)

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.

3 participants