Gmoccapy: tooltable fixes #3706

Sigma1912 · 2026-01-18T12:01:32Z

Restores the tooltips for [Delete, Add, Reload, Save] buttons
Restores blocking the user from deleting the loaded tool
Makes tooledit widget return a list rather than 'None' when multiple checkboxes are selected (required to check if loaded tool is among multiple selected tools)
Hides current tool number display in the tooledit frame when the touch keyboard is not active

zz912 · 2026-01-18T12:56:57Z

Should be possible add to this PR functionality from #3474 ?

Sigma1912 · 2026-01-18T16:50:41Z

Should be possible add to this PR functionality from #3474 ?

I'll have a look

hansu · 2026-01-18T18:30:57Z

src/emc/usr_intf/gmoccapy/gmoccapy.py

-        lbl_tool_text = "Tool loaded: " + str(toolnumber)
+        lbl_tool_text = ""
+        # When using touch keyboard we display the current tool in the tooltable frame
+        if self.widgets.chk_use_kb_on_tooledit.get_active():
+            lbl_tool_text = "Tool loaded: " + str(toolnumber)


Why not always display the tool number of the current tool?

There was a request to avoid having redundant information:
#3509 (comment)

Ah ok. I disagree 🙃

hansu · 2026-01-18T18:46:29Z

Removing the calls of self.widgets.tooledit1.set_selected_tool(act_tool) improves the behaviour in most cases but for on_reload_clicked() I would like to keep it. So that the tool in the spindle is checked when opening the tool table and on reload.
What do you think?
The "reload tooltable when the widget is shown" has one disadvantage: when you add a new tool, and go back without saving it, your changes got lost. What is the benefit from reload when showing?

Sigma1912 · 2026-01-18T19:01:05Z

I would like to keep it. So that the tool in the spindle is checked when opening the tool table and on reload.
What do you think?

No problem

What is the benefit from reload when showing?

Current behavior is that unsaved changes in the tool table are retained when leaving the tool table screen but the changed values are discarded when a tool change is executed and the saved offsets are used instead.
IMO this is unexpected behavior. When I open the tool table I expect to see the values that will be applied when a tool is changed.
Maybe the user should be presented with a popup message if there are unsaved changes on leaving the page?

zz912 · 2026-01-18T20:09:10Z

I think the reolading is good idea. But I did not test it yet.

hansu · 2026-01-18T20:20:59Z

I would like to keep it. So that the tool in the spindle is checked when opening the tool table and on reload.
What do you think?

No problem

👍

Maybe the user should be presented with a popup message if there are unsaved changes on leaving the page?

That would be the cleanest solution.
Then we might also save the tool table when doing a tool change. Imagine someone changes the offsets of a tool and then initiates a tool change for that tool. Then the newly entered offsets wouldn't be used.
Or add another warning message like "Tool table contains unsaved changes. They will get lost after the Tool change."

Sigma1912 · 2026-01-19T14:10:57Z

Dropped the commit hiding the label for current tool number in the tool table frame
Added message on leaving the page with unsaved changes (either by 'back' button or by change tool buttons)

Sigma1912 · 2026-01-19T14:14:18Z

Then we might also save the tool table when doing a tool change.

I opted for the message popup as I prefer the user to be aware of what is going on.

hansu · 2026-01-19T18:00:27Z

It seems something went wrong. The first two commits doesn't belong in this PR and there is also a merge conflict...

zz912 · 2026-01-19T18:09:20Z

I tested popup. It works.
Is it possible to respond with a popup to a cell that has been filled out when trying to leave?

hansu · 2026-01-19T18:14:49Z

And to be a bit pedantic - I would change the message "Discard changes?" (when changing a tool with unsaved changes) to "Change tool without saving changes/ and discard changes?"
Otherwise it could be read as "save changes and change tool". What do you think?

And the warning pop up could also be added for reload.

Sigma1912 · 2026-01-19T18:15:23Z

Is it possible to respond with a popup to a cell that has been filled out when trying to leave?

I have not found a solution for that yet.

hansu · 2026-01-19T18:53:00Z

Looks good now, thanks. Only the buildbot looks weird.

hansu · 2026-01-19T21:29:41Z

Let me rebase that to master and see if that makes the buildbot/CI happy...

BsAtHome · 2026-01-19T21:43:06Z

Let me rebase that to master and see if that makes the buildbot/CI happy...

CI fails on Debian:Sid telling us that there no longer is a working c++ compiler:

...
checking whether the C++ compiler works... no
configure: error: in '/__w/linuxcnc/linuxcnc/src':
configure: error: C++ compiler cannot create executables
See 'config.log' for more details
...

The rest of CI seems to work fine. Probably someone pushed an update to the CI infrastructure that broke stuff.

Sigma1912 · 2026-01-20T08:09:21Z

Fixes the bug with the Diameter value not updating when changing the active tool diameter in the table:

zz912 · 2026-01-20T08:12:06Z

Thank you.

Is offset updated too?

Sigma1912 · 2026-01-20T08:19:16Z

Is offset updated too?

Not yet. I want to fix the obvious bugs first.
I'm still pondering the 'G43' issue as I don't like it when the GUI silently issues MDI commands when I click on a button like that.

Sigma1912 · 2026-01-20T08:27:20Z

Is offset updated too?

Oh, did you mean the 'offset z' value?
That is NOT updated as this value is taken from 'motion.tooloffset.z' and reflects the actually applied offset. So this is '0.0' with 'G49'.
One might argue that it should just reflect the tool table value but personally I find it useful to have it like this.

zz912 · 2026-01-20T10:47:50Z

I think I've solved your questions in #3474.

If G49 is active, nothing happens.
This PR fixes the issue with mode switching when executing mdi.command. I think this solution is the best the current API offers.

I don't like it when the GUI silently issues MDI commands

First I wanted to write to you to make a popup question whether G43 should be executed when a value in the table is changed. But then I thought about it and I couldn't think of a situation where someone would change the offsets in the tooltable and still want to work with the old offsets.

It would be stupid if the "save button" activated G43 when G49 was active, but my PR doesn't do that.

Furthermore, during testing I found out that it should not be possible to change the tool diameter when G41 or G42 is on. The change should only be possible when G40 is on.
https://linuxcnc.org/docs/devel/html/gcode/g-code.html#gcode:g41-g42
https://linuxcnc.org/docs/devel/html/gcode/tool-compensation.html#sec:cutter-radius-compensation

zz912 · 2026-01-20T11:19:59Z

Perhaps the command self.command.mdi("G43") could be replaced with a direct call. However, I don't know how and whether it is possible in Gmoccapy:

https://linuxcnc.org/docs/devel/html/code/code-notes.html#_g_codes_affecting_tools

24.3.3. G43/G43.1/G49

Apply tool length offset. G43 uses the TLO of the currently loaded tool, or of a specified tool if the H-word is given in the block. G43.1 gets TLO from axis-words in the block. G49 cancels the TLO (it uses 0 for the offset for all axes).

Handled by Interp::convert_tool_length_offset().

It starts by building an EmcPose containing the 9-axis offsets to use. For G43.1, these tool offsets come from axis words in the current block. For G43 these offsets come from the current tool (the tool in pocket 0), or from the tool specified by the H-word in the block. For G49, the offsets are all 0.

The offsets are passed to Canon’s USE_TOOL_LENGTH_OFFSET() function.

    (saicanon) Records the TLO in _tool_offset.

    (emccanon) Builds an EMC_TRAJ_SET_OFFSET message containing the offsets and sends it to Task. Task copies the offsets to emcStatus->task.toolOffset and sends them on to Motion via an EMCMOT_SET_OFFSET command. Motion copies the offsets to emcmotStatus->tool_offset, where it gets used to offset future motions.

Back in interp, the offsets are recorded in settings->tool_offset. The effective pocket is recorded in settings->tool_offset_index, though this value is never used.

Sigma1912 · 2026-01-20T12:33:33Z

But then I thought about it and I couldn't think of a situation where someone would change the offsets in the tooltable and still want to work with the old offsets.

That is the thing I don't like about it. The GUI trying to guess what the user wants.
Say I edit the wrong tool offset and suddenly my active tool offsets are silently changed, that is what I'm trying to avoid.

I'd much prefer a popup when clicking the 'back' button with changes to the current tool offset. I have this mostly working but I'm out of time now.

Furthermore, during testing I found out that it should not be possible to change the tool diameter when G41 or G42 is on. The change should only be possible when G40 is on.

Good point.
Maybe it should not be possible to change the offset values of the active tool with 'G43' on either?

zz912 · 2026-01-20T13:18:34Z

Say I edit the wrong tool offset and suddenly my active tool offsets are silently changed, that is what I'm trying to avoid.

If you adjust the wrong offsets, it will always be a problem.
My PR was created at the request of user LCNC:
https://forum.linuxcnc.org/gmoccapy/56079-change-tool-offsets-after-editing-the-tool-table?start=0

@hansu @andypugh Do you think reactivating G43 after changing offsets in the tooltable is a good idea?

Maybe it should not be possible to change the offset values of the active tool with 'G43' on either?

G43 has different behavior compared to G41 G42. You can change G43 H1 => G43 H2, but you cant change G42 D1 => G42 D2. You must G42 D1 => G40 => G42 D2.

Sigma1912 · 2026-01-20T14:33:32Z

I'm not saying that we cannot apply G43 on leaving the tool table with changed offsets for the tool in the spindle. What I want is for the operator to be notified about what is happening.

zz912 · 2026-01-20T14:45:16Z

I have no problem with the operator notification.

- show message on leaving tool table with unsaved changes - reload tooltable from file when showing the table

…ng on saving tooltable

Sigma1912 · 2026-01-20T19:10:19Z

I pushed a fix to 5635759 as the 'Comments' column was not being checked for unsaved edits.

hansu · 2026-01-20T22:32:09Z

Let me look into the G43 thingy tomorrow...

Another thing I noticed:
In this selection scenario it's a bit unclear to which tool should be changed. There has been a warning before when attempting a tool change with this selection. It would be nice if that can be restored.

… offset changed

Sigma1912 · 2026-01-21T07:44:38Z

Added user confirmation on leaving tooltable with active tool offsets changed:

Sigma1912 · 2026-01-21T08:20:00Z

There has been a warning before when attempting a tool change with this selection. It would be nice if that can be restored.

Ideally the first column should be renamed 'Delete' but I have not found a way to do that yet as 'set_title' method is ignored. So it will have to be another message popup.

zz912 · 2026-01-21T08:47:12Z

The Popup looks good.

I'm taking this from the perspective of a beginner learning to work with a CNC machine. I was in this situation recently. I change the offset, and nothing happens. Then I try to "save" and nothing happens again. As a beginner, it doesn't occur to me to "exit" to update the offsets.

Or I would like to have some other option to trigger the reactivation of G43 in the tooltable environment.

hansu · 2026-01-21T11:13:24Z

@hansu @andypugh Do you think reactivating G43 after changing offsets in the tooltable is a good idea?

The solution with the confirmation dialog is good. But it doesn't appear after the second change or if you apply G43 manually:

tooltable.mp4

zz912 · 2026-01-21T11:37:37Z

To hans:

Note that if you have G43 activated and you change the offset to 0, then the next time you reactivate G43, G49 will be activated. This feature of LCNC confused me a lot when I was learning to work with offsets.
It might be worth making another statement for the situation where the current tool offset is 0 that g49 will be activated.

hansu · 2026-01-21T11:50:16Z

Ah ok I didn't know/noticed that. My machine doesn't allow resp. it doesn't make sense to work with tool offsets.
So it's fine, sorry for the noise David.

hansu · 2026-01-21T12:31:01Z

Removing the calls of self.widgets.tooledit1.set_selected_tool(act_tool) improves the behaviour in most cases but for on_reload_clicked() I would like to keep it. So that the tool in the spindle is checked when opening the tool table and on reload.
What do you think?

No problem

As the selection row is only used for deleting, maybe it would be better to highlight the current tool in another way like having the line bold. But not sure if that is possible.

Sigma1912 · 2026-01-21T13:45:26Z

maybe it would be better to highlight the current tool in another way like having the line bold. But not sure if that is possible.

The idea was for the user to select the tool in the same way we select the offset in the offset page.
The issue with marking the currently loaded tool in the table is that it is not always visible as we scroll up/down. That is why we decided to have the label in the button box.

Sigma1912 · 2026-01-21T14:04:05Z

Or I would like to have some other option to trigger the reactivation of G43 in the tooltable environment.

Generally I'm not a great fan of GUIs that try to provide convenience functions for everything. The automatic reactivation of G43 after a tool change was also such a function until we got rid of it.

IMO allowing the operator to change the tool table entry of the tool in the spindle with 'G43' active is a bit dubious. In the end all of these things could just as well be done by the operator in the MDI page with the added benefit of actually having to think whats happening.

zz912 · 2026-01-21T14:36:24Z

The automatic reactivation of G43 after a tool change was also such a function until we got rid of it.

We didn't get rid of this feature, we just moved it to remap. This was due to the race condition and instability of this feature when it was in the GUI. "reactivation of G43 after a tool change" caused problems mainly with the combination of halui.mdicommands and multiple commands in mdi-commands (for example M6 T4 G4 P1). Reactivation of G43 in the tooltable is a different situation, I don't see any problem here.

Generally I'm not a great fan of GUIs that try to provide convenience functions for everything.

IMO allowing the operator to change the tool table entry of the tool in the spindle with 'G43' active is a bit dubious.

I had similar thoughts. After the first crash of my friend's machine, I reevaluated everything.

I don't know if you have a machine with an automatic tool changer and whether you use tool compensation.

I'm sorry that I keep adding more and more requests, but on the other hand it makes sense. You're making Gmoccapy and tooltable better and better. I'm convinced that it makes sense. Please be patient with me.

Sigma1912 · 2026-01-21T14:39:42Z

@hansu

In this selection scenario it's a bit unclear to which tool should be changed.

How about this:

zz912 · 2026-01-21T14:43:16Z

@hansu

In this selection scenario it's a bit unclear to which tool should be changed.

How about this:

Should be possible change image "button delete"?
The first button image would look like a checkbutton.
The second button image would remain the same cross.

Tooltip should be changed too. First tooltip "Select for delete", second tooltip "delete"

Sigma1912 · 2026-01-21T14:44:55Z

Changing the tooltips is no problem.

Changing icons is also possible but more work.

hansu · 2026-01-21T15:10:20Z

In this selection scenario it's a bit unclear to which tool should be changed.

How about this

This feels a bit weird. What I could imagine is to have the delete button as toggle button to enable delete mode and then to have small delete icons in every row which deletes the tool immediately:

But I am also fine with the current design with the checkbox. Just to get the column's title changed. Can't we change the title in the glade file, or is it only for Gmoccapy a delete-only checkbox?

Sigma1912 · 2026-01-21T16:02:52Z

What I could imagine is to have the delete button as toggle button to enable delete mode and then to have small delete icons in every row which deletes the tool immediately

Yes, why not. Not sure how easy it is to modify the checkbox to an icon button.

Can't we change the title in the glade file, or is it only for Gmoccapy a delete-only checkbox?

Original behavior was for this column to select the rows to delete and the tool to change to. I have not found a way to change the column title other than in the 'toolwidget.glade' file.

Sigma1912 · 2026-01-21T16:46:02Z

According to this it is unfortunately not possible to replace the checkbox with a button:
https://discourse.gnome.org/t/cellrendererbutton-have-a-clickable-button-in-a-treeview-column/2103

zz912 · 2026-01-21T16:59:54Z

I have no problem with double funkcionality first button.
For select, it could look like:

For delete as is:

Sigma1912 mentioned this pull request Jan 18, 2026

Gmoccapy: Move tool table buttons to table frame, add toggle button for calculator use #3509

Merged

Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from 7f22530 to 57da7f7 Compare January 18, 2026 12:17

hansu reviewed Jan 18, 2026

View reviewed changes

Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from 2d5a5b9 to eff3d22 Compare January 19, 2026 14:03

Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from eff3d22 to ffcaa40 Compare January 19, 2026 15:47

Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from ffcaa40 to d27a24e Compare January 19, 2026 18:10

Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from d27a24e to e217db4 Compare January 19, 2026 18:39

Sigma1912 added 2 commits January 19, 2026 22:28

Gmoccapy: Restore tooltips and reconnect handlers for tooltable buttons

ed5442c

Gmoccapy: Fix current tool check when deleting multiple tools

027c75a

hansu force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from e217db4 to d42a12d Compare January 19, 2026 21:29

Sigma1912 added 2 commits January 20, 2026 20:05

Gmoccapy: better handling of unsaved changes in tooltable

5635759

- show message on leaving tool table with unsaved changes - reload tooltable from file when showing the table

Gmoccapy: Fix 'Diameter' value in 'Tool information' frame not updati…

354d91e

…ng on saving tooltable

Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from fd841dc to 354d91e Compare January 20, 2026 19:06

Gmoccapy: Add user confirmation on leaving tooltable with active tool…

4788f77

… offset changed

Gmoccapy: tooltable fixes #3706

Are you sure you want to change the base?

Gmoccapy: tooltable fixes #3706

Conversation

Sigma1912 commented Jan 18, 2026

Uh oh!

zz912 commented Jan 18, 2026

Uh oh!

Sigma1912 commented Jan 18, 2026

Uh oh!

hansu Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Sigma1912 Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

hansu Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

hansu commented Jan 18, 2026

Uh oh!

Sigma1912 commented Jan 18, 2026

Uh oh!

zz912 commented Jan 18, 2026

Uh oh!

hansu commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sigma1912 commented Jan 19, 2026

Uh oh!

Sigma1912 commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hansu commented Jan 19, 2026

Uh oh!

zz912 commented Jan 19, 2026

Uh oh!

hansu commented Jan 19, 2026

Uh oh!

Sigma1912 commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hansu commented Jan 19, 2026

Uh oh!

hansu commented Jan 19, 2026

Uh oh!

BsAtHome commented Jan 19, 2026

Uh oh!

Sigma1912 commented Jan 20, 2026

Uh oh!

zz912 commented Jan 20, 2026

Uh oh!

Sigma1912 commented Jan 20, 2026

Uh oh!

Sigma1912 commented Jan 20, 2026

Uh oh!

zz912 commented Jan 20, 2026

Uh oh!

zz912 commented Jan 20, 2026

Uh oh!

Sigma1912 commented Jan 20, 2026

Uh oh!

zz912 commented Jan 20, 2026

Uh oh!

Sigma1912 commented Jan 20, 2026

Uh oh!

zz912 commented Jan 20, 2026

Uh oh!

Sigma1912 commented Jan 20, 2026

Uh oh!

hansu commented Jan 20, 2026

Uh oh!

Sigma1912 commented Jan 21, 2026

Uh oh!

Sigma1912 commented Jan 21, 2026

Uh oh!

zz912 commented Jan 21, 2026

Uh oh!

hansu commented Jan 21, 2026

Uh oh!

hansu commented Jan 18, 2026 •

edited

Loading

Sigma1912 commented Jan 19, 2026 •

edited

Loading

Sigma1912 commented Jan 19, 2026 •

edited

Loading

zz912 commented Jan 21, 2026 •

edited

Loading

Sigma1912 commented Jan 21, 2026 •

edited

Loading

Sigma1912 commented Jan 21, 2026 •

edited

Loading