Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extend M6 remap to allow return to orig. position #2706

Merged
merged 3 commits into from
Aug 3, 2024
Merged

Extend M6 remap to allow return to orig. position #2706

merged 3 commits into from
Aug 3, 2024

Conversation

VectorHasting
Copy link
Contributor

My use case is that I use FreeCAD to generate g-code and that post-processor does not allow inserting custom code into g-code after the M6 tool change. Yet the default return location in the default QtDragon M6 remap does not return to the point my g-code expects, but rather the same point that GMOCCAPY remap does.

I propose that most people would prefer the changed tool tip to return to the same point in space that old tool tip was at.

There does not seem to be a "cannon" since the AXIS gui proposed default M6 remap returns to the tool change position, and many projects abandoned on git or live in the world have customized remaps that return the tool to the original position.

(See this lengthy forum article, for an example of people's interest.)

What I did:

Added an INI setting in the Versa_toolsetter section that give the option to direct the return point to either:
Option 1 (Also the default if no option specified): Gmoccapy default already released with QtDragon: Only returns to the Change_Position z height, as programmed at: configs > sim > gmoccapy > macros > change.ngc
Option 2: AxisDefault: return to the XYZ tool change position, as programmed at: configs > sim > axis > remap > manual-toolchanges-with-tool-length- switch > nc_subroutines > Manual_change.ngc
Option 3: New QtDragon option: Return the tip of the current changed tool to the XYZ position of the tip of the old tool, moving to Tool Change Z first, then Original XY, then down to new offset Z.

I also moved some error checking earlier.

Changes made to documentation:
All changes are limited to the “Auto Tool Measurement” section. (Currently, #14) except for:

  1. fixing one typo,
  2. adding a [[sub:touch-plate]] tag to the Touch plate section so that the Auto Tool section can redirect readers there.
  3. Increase the Table of Contents Levels depth to 4 (default is 2)

In the “Auto Tool Measurement” Section 14, I made the following changes:

Created a section at the beginning called “Overview” (new 14.1) In this section I explain that this procedure intends to use two probes, the Tool Setter and the spindle probe, also called a Renishaw probe.
It directs people with repeatable tool holders to the Touch Plate section. It directs those with automatic tool changers to axamine the axis remap code rack-toolchange.

It then splits the documentation into “Work Flow Overview” which includes most of the previous documentation from section 14 that was above the QtDragon_hd screen. This becomes section 14.2
In this section, I also moved pre-requisites earlier (ie, instead of warning near the end to have Use Tool sensor selected, include this along with other pre-requisites under the Important heading near the top.
In the “sequence of events” section I added the extra possibilities of this ngc code modification:
The INI setting [RETURN_OPTION] can elicit additional behavior after the return to the [TOOL_CHANGE] Z position:
• If INI’s [RETURN_OPTION] = 1: (or if it is undefined), no further action. This is the default behavior of the GMOCCAPY M6 remap procedure
• If INI’s [RETURN_OPTION] = 2: Continue with a Rapid Move to the X and Y position defined in INI’s [TOOL_CHANGE] X and Y settings. This is the default behavior of the AXIS gui’s M6 remap procedure.
• If INI’s [RETURN_OPTION] = 3: Rapid Move to the original X and Y position at the moment the tool-change started, then Rapid Move down to the now offset Z position at the moment the tool-change started. This effectively moves the tool head back to the position just prior to the tool change. This is new behavior exclusive to QtDragon as of 4th quarter 2023.

I added another section following that one called “Detailed Workflow Example” (new section 14.3)

Renamed the next section to be clear it only applies to the QtDragon_hd interface (was 14.1, now 14.4)

All the following sections automatically increased in numbering.

Added the RETURN_OPTION explanations to what is now section 14.6.3 “The Tool Sensor Section” Corrected “TOOLCHANGE” in section 14.6.3 to be “CHANGE_POSITION”

Thank you for your consideration.

@VectorHasting
Extend M6 remap to allow return to orig. position
b2634c6 
Made a switch in the Versa_toolsetter section that give the option to direct the return point to either:
Option 1 (Default if no option specified): Gmoccapy default already released with QtDragon:
Only returns to the Change_Position z height, as programmed at:
configs > sim > gmoccapy > macros > change.ngc
Option 2: AxisDefault
Return to the XYZ tool change position, as programmed at:
configs > sim > axis > remap > manual-toolchanges-with-tool-length- switch > nc_subroutines > Manual_change.ngc
Option 3: New QtDragon option:
Return the tip of the current changed tool to the XYZ position of the tip of the old tool, moving to Tool Change Z first, then Original XY, then down to new offset Z.
Also moved some error checking earlier.
Changes made to documentation:
All changes are limited to the “Auto Tool Measurement” section. (Currently, LinuxCNC#14) except for
1. fixing one typo,
2 adding a [[sub:touch-plate]] tag to the Touch plate section so that the Auto Tool section can redirect readers there.
3. Increase the Table of Contents Levels depth to 4 (default is 2)
In the “Auto Tool Measurement” Section 14, I made the following changes:
Created a section at the beginning called “Overview” (new 14.1)
In this section I explain that this procedure intends to use two probes.  be done with one or two sensors. The one sensor only method requires the Tool Switch and is described as One Sensor Workflow. The two sensor method makes use of an additional spindle probe, also called a Renishaw probe.
It directs people with repeatable tool holders to the Touch Plate section. It directs those with automatic tool changers to axamine the axis remap code rack-toolchange.
It then splits the documentation into “Work Flow Overview” which includes most of the documentation above the QtDragon_HD screen intact. This becomes section 14.2
In this section, I moved pre-requisites earlier (ie, instead of warning near the end to have Use Tool sensor selected, include this along with other pre-requisites under the Important heading near the top.
In the “sequence of events” section I added the extra possibilities of this ngc code modification:
The INI setting [RETURN_OPTION] can elicit additional behavior after the return to the [TOOL_CHANGE] Z position:
    • If INI’s [RETURN_OPTION] = 1: (or if it is undefined), no further action. This is the default behavior of the GMOCCAPY M6 remap procedure
    • If INI’s [RETURN_OPTION] = 2: Continue with a Rapid Move to the X and Y position defined in INI’s [TOOL_CHANGE] X and Y settings. This is the default behavior of the AXIS gui’s M6 remap procedure.
    • If INI’s [RETURN_OPTION] = 3: Rapid Move to the original X and Y position at the moment the tool-change started, then Rapid Move down to the now offset Z position at the moment the tool-change started. This effectively moves the tool head back to the position just prior to the tool change. This is new behavior exclusive to QtDragon as of 4th quarter 2023.
Added another section following that one called “Workflow Example” (new section 14.2)
Renamed the next section to be clear it only applies to the QtDragon_hd interface (was 14.1, now 14.4)
All the following sections automatically increased in numbering.
Added the RETURN_OPTION explanations to what is now section 14.6.3 “The Tool Sensor Section”
Corrected “TOOLCHANGE” in section 14.6.3 to be “CHANGE_POSITION”
@c-morley
Copy link
Collaborator

Does the sim configuration 'qtdragon_auto_probe' need updating?
This seems like a bigish change for 2.9 (it's been technically released I think)
I can't at the moment test your changes.

@VectorHasting
Copy link
Contributor Author

Should we change the target to a later branch? If so, which is that, and can that be changed here or does this pull need to be closed and re-pulled?

@c-morley
Copy link
Collaborator

Master would be the branch, but we can do that later or maybe 2.9 would be ok anyways - it does default to the same behavior.
I don't want 2.9 users to update and crash their machines so need to be careful.

it can be changed:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request

@VectorHasting
Copy link
Contributor Author

OK, my apologies for being so long in getting back to this (my job search prep took me a lot longer than I thought it would).

Does the sim configuration 'qtdragon_auto_probe' need updating? ... I can't at the moment test your changes.

Out of the box, when I try the qtdragon_auto_probe sim, and I try to home the Z axis, I get "Operator Error: Cannot home while shared home switch is closed j=2."

I figured that was me not knowing something, but based on your question here and the comments on that other pull request #2562 , then perhaps: yes, there may need to be something done for the sim.

But I have an actual machine that I've been testing this remap qt_auto_probe_tool.ngc with, so I'm going to do the test physically.

My plan is to:
a. post this comment
b. make the change to reuse the parameter calculated for printing in the G10 command
c. test the changed G10 command, (this may take me days not minutes to get done, because I'll video it)
d. commit that G10 change to this pull request.
d. look at testing the other pull request #2562, ** but see discussion on that pull request because I have questions **
e. post video evidence of these tests, here and in the thread for the other pull request.

Advise if that's a bad/insufficient course of action.

Thank you for your time.

@VectorHasting VectorHasting mentioned this pull request Nov 13, 2023
Merged
@andypugh
Copy link
Collaborator

Sorry, I am slowly working through the PR backlog.
Is this still live? I would like to re-run the CI tests, but don't seem to be able to (perhaps the PR is now too old?)

@VectorHasting
Copy link
Contributor Author

VectorHasting commented Dec 21, 2023 via email

@andypugh
Copy link
Collaborator

I am still not clear on what this PR is for.
There is built-in behaviour for where the tool goes for a normal, not-remapped toolchange.
But this seems to be meant to address where the tool goes after a remapped tool-change?
It seems to me that the whole point of a remap is to get the behaviour you want, and that it makes more sense to put the custom behaviour in the custom remap, rather than add it to the base code.
Can you explain why you think this belongs as an INI setting rather than as part of the remap?

@VectorHasting
Copy link
Contributor Author

VectorHasting commented Dec 27, 2023 via email

@VectorHasting
Copy link
Contributor Author

I see now there is a conflict with the documentation...

Should I update this pull request by refreshing my qtdragon.adoc, the re-applying my changes, then updating my pull request...

Or are we going to branch to the "change this pull request to suggest a new 'qt_auto_probe_tool_w_return.ngc' (or whatever) file being added to the repository, instead of modifying the old one? Or is this pull request going nowhere?

Thanks!

@andypugh
Copy link
Collaborator

andypugh commented Jan 7, 2024

The "built in" begaviour that I mentioned was basically prior to the tool change. It's a few INI options:
http://linuxcnc.org/docs/stable/html/config/ini-config.html#sub:ini:sec:emcio
TOOL_CHANGE_POSITION , TOOL_CHANGE_QUILL_UP, TOOL_CHANGE_AT_G30

Bear in mind that LinuxCNC was designed before the days of high-speed spindles which only accept collets. Back then there was no need to probe tool length as the common tool holder systems all had repeatable length. (CAT, BT, R8 even)
This is still the case now that we have HSK and Capto (I am really liking Capto, having bought a holder to experiment with)

I imagine that FreeCAD only moves the tool to a safe change position, and assumes that it is still there when the code resumes after tool change. Which ought to be a safe assumption.

I still think it is the job of the toolchanger routine (even if is is a "real" toolchanger with repeatable holders) to put things backj where the G-code left it before continuing the code.

In fact, I don't even think that this needs to be configurable, the remap routine should just do it. If there are "stock" library routines that don't do this then I agree that they probably need to be improved.

Not that there are any "stock" remaps, in that they are really intended for integrator customisation. Even "standard glue" exists in at least three different versions in various locations in the codebase.

@VectorHasting
Copy link
Contributor Author

VectorHasting commented Feb 13, 2024 via email

@andypugh
Copy link
Collaborator

  1. I have no specific objection against modifying the customisation code, it is just that there may not be much point as most systems using it are likely to have modified it further. Use G38.3 in qt_auto_probe_tool.ngc to be able to check probe status. #2562 looks to have actually been approved by the allocated reviewer, but not yet merged. (possibly waiting for you to say if it is incompatible with your proposed changes)
  2. Yes, I think that the stock remaps should leave the machine in the position it was when the remap was called, in most cases. There are obvious exceptions, such as remaps that are meant to move to axes, but things like adding a probe to a toolchange should "minimise surprise"
  3. Adding yet another remap routine might be cleaner in some ways, but is probably unecesary code duplication.

Please don't mistake a lack of activity on an issue for active disapproval, it is just that none of us have as much time to check PRs on actual hardware as we would like. I am typing this on a long train journey, for example, so can answer at length, but can't actually try anything out.

@VectorHasting
Copy link
Contributor Author

I've completed the demonstration video that confirms this code's functionality.

I next need to resolve the documentation conflicts and confirm the version I tested is the same version proposed here. Since 2562 has been merged, both files here are in conflict. So my intention is to 'revert' the code, and then remake my changes in my local git-desktop, then repost them here. That should happen within a day.

All the best.

@VectorHasting
Update per discussions
8caa075 
This update makes the following changes, per comments on 2/17/2024
1. Changes the stock remap to unconditionally return the machine to the position it was in when the remap was called. (This gets rid of a previous iteration that added a 'return_option' variable to the .ini file.)
2. Changes to documentation to reflect this change.

In full disclosure: This change will now also have the effect of largely reversing PR # 2562, because this version goes back to using G38.2. I do that because in practice G38.3 (the main change in PR 2562) does not allow the QtDragon implementation to display probing errors to the user. This this version will restore that functionality. This code does, however, retain the order of additional error-checking that was changed in PR # 2562.

These changes will also hopefully pass error checking because they are based on an updated fetch of 2.9.
@VectorHasting
Copy link
Contributor Author

I made a new commit, but I've forgotten how to initiate the checks. I went to the "Checks" tab and it says "Workflow runs completed with no jobs."

What can I do to initiate the checks on this new commit?

Sorry for being such a github newb.

@VectorHasting
Copy link
Contributor Author

VectorHasting commented Jul 13, 2024 •
edited
Loading

When I did my last commit, I thought that would essentially mean I didn’t need to do the “resolve conflicts” task.

But since it didn’t run the checks, I decided to go ahead and manually do the “resolve conflicts” task. After that, it started doing the error checks, so I think I’m making progress.

During the resolve, I eliminated the following code from qt_auto_probe_tool.ngc, which had been added by PR 2562: namely there was a parameter added that was never used (which means it would involve extra unneeded calculations).
This parameter was apparently copied into the code from the G-Code reference page, but in the reference code example, the parameter is used in the following G10 statement. However, in PR 2562, the parameter was never used: qt_auto_probe_tool.ngc does issue a G10 but with the parameter #<calculated_offset>.
These are the code lines I eliminated that I haven't previously mentioned changing back (ie, this PR rolls back a lot of PR2562, and I'm trying to be transparent about what.)

; calculate Z offset value based on current work offset (#5220) and
; G92/G52 offset if enabled (#5210)
#<zworkoffset> = [#[5203 + #5220 * 20] + #5213 * #5210]

@andypugh andypugh changed the base branch from 2.9 to master August 3, 2024 18:49
@andypugh andypugh merged commit 2286f75 into LinuxCNC:master Aug 3, 2024
6 of 11 checks passed
@andypugh
Copy link
Collaborator

andypugh commented Aug 3, 2024

I have rebased this on Master, as 2.9 is now bugfixes-only.

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

@c-morley c-morley c-morley left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VectorHasting @c-morley @andypugh