Skip to content

Gmoccapy: tooltable fixes #3706

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
Open
Sigma1912 wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Gmoccapy: tooltable fixes #3706

Sigma1912 wants to merge 5 commits into from

Conversation

@Sigma1912
Copy link
Contributor

@Sigma1912 Sigma1912 commented Jan 18, 2026

  • 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

@Sigma1912 Sigma1912 mentioned this pull request Jan 18, 2026
Merged
@Sigma1912 Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from 7f22530 to 57da7f7 Compare January 18, 2026 12:17
@zz912
Copy link
Contributor

zz912 commented Jan 18, 2026

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

@Sigma1912
Copy link
Contributor Author

Sigma1912 commented Jan 18, 2026

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

I'll have a look

hansu
hansu reviewed Jan 18, 2026
View reviewed changes
src/emc/usr_intf/gmoccapy/gmoccapy.py Outdated
Comment on lines 2043 to 2052
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)
Copy link
Member

@hansu hansu Jan 18, 2026

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@Sigma1912 Sigma1912 Jan 18, 2026

Choose a reason for hiding this comment

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

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

Copy link
Member

@hansu hansu Jan 18, 2026

Choose a reason for hiding this comment

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

Ah ok. I disagree 🙃

@hansu
Copy link
Member

hansu commented Jan 18, 2026

  1. 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?

  2. 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
Copy link
Contributor Author

Sigma1912 commented Jan 18, 2026

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
Copy link
Contributor

zz912 commented Jan 18, 2026

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

@hansu
Copy link
Member

hansu commented Jan 18, 2026 •
edited
Loading

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 Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from 2d5a5b9 to eff3d22 Compare January 19, 2026 14:03
@Sigma1912
Copy link
Contributor Author

Sigma1912 commented Jan 19, 2026

  • 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
Copy link
Contributor Author

Sigma1912 commented Jan 19, 2026 •
edited
Loading

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.

@Sigma1912
Gmoccapy: better handling of unsaved changes in tooltable
ffcaa40 
- show message on leaving tool table with unsaved changes
- reload tooltable from file when showing the table
@Sigma1912 Sigma1912 force-pushed the Gmoccapy-tooltable_Restore_button_tooltips branch from eff3d22 to ffcaa40 Compare January 19, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hansu hansu hansu 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

@Sigma1912 @zz912 @hansu