Skip to content

Orient.comp: Add modes with preceding homing to index - #4343

Open
Sigma1912 wants to merge 2 commits into
LinuxCNC:masterfrom
Sigma1912:Orient_Add-modes-with-index
Open

Orient.comp: Add modes with preceding homing to index#4343
Sigma1912 wants to merge 2 commits into
LinuxCNC:masterfrom
Sigma1912:Orient_Add-modes-with-index

Conversation

@Sigma1912

Copy link
Copy Markdown
Contributor

Some spindle encoder setups cannot track the encoder position when using high spindle speeds and thus cannot use the current 'orient' component due to lost encoder position.
This PR expands the orient component with three additional modes that offer referencing to an index signal prior to orienting the spindle.

Current behavior is unchanged. All modes have been tested on real hardware.

Based on work by @andypugh

For more:
https://forum.linuxcnc.org/10-advanced-configuration/42792-m19-and-spindle-index-enable-issues?start=10

Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
@Sigma1912
Sigma1912 force-pushed the Orient_Add-modes-with-index branch from 4f75431 to 56a499e Compare August 5, 2026 14:41
Comment thread src/hal/components/orient.comp
@Sigma1912
Sigma1912 force-pushed the Orient_Add-modes-with-index branch from 56a499e to ddadd47 Compare August 5, 2026 14:54
@Sigma1912

Copy link
Copy Markdown
Contributor Author

Thank you for reviewing and your pointers! Much appreciated.

@BsAtHome

BsAtHome commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

You are welcome. I'm just looking at this on the structural level. Not looking at what it is actually doing ;-)

There may be one more problem. You have a message printing when the mode value is not handled. If I'm not mistaking, this message would print at every thread-cycle flooding the terminal and GUI.

@grandixximo grandixximo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch. Tested-on-hardware confirmation is much appreciated, and the state machine structure is a clean way to add the index modes. I have a few questions and concerns inline, mostly around behavior changes in the existing modes.

Comment thread src/hal/components/orient.comp
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp Outdated
Comment thread src/hal/components/orient.comp
Comment thread src/emc/rs274ngc/interp_check.cc
@Sigma1912
Sigma1912 force-pushed the Orient_Add-modes-with-index branch from ddadd47 to dd9e02d Compare August 6, 2026 12:47
@Sigma1912

Copy link
Copy Markdown
Contributor Author

Thanks again for investing the time to review!

orient.comp currently does not support using an index signal.
This makes it unusable for setups where the spindle encoder position
cannot be tracked due to the encoder reading being too slow.
@Sigma1912
Sigma1912 force-pushed the Orient_Add-modes-with-index branch from 234e155 to cc32ccf Compare August 6, 2026 12:54
@Sigma1912

Copy link
Copy Markdown
Contributor Author

Changed 'M19' doc:
Screenshot from 2026-08-06 13-59-19

@andypugh

andypugh commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

The way I had anticipated solving this was to add spindle homing pins to the spindle control logic.

It is almost certainly simpler to add it to the orient comp, but I am not sure which way is better.

@grandixximo

Copy link
Copy Markdown
Contributor

The way I had anticipated solving this was to add spindle homing pins to the spindle control logic.

It is almost certainly simpler to add it to the orient comp, but I am not sure which way is better.

Something against having both?

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.

4 participants