Gmoccapy: Fixes in calculator and tooledit_widget

[Sigma1912]

• Calculator no longer hands back float strings when asking for integers
• Tooledit_widget now truncates float input values to positive integer values for the tool- and pocket number

Fixes #4435

[hansu]

That was quick, thanks.

I haven't dug deep into this but I wonder if we should fix it better in tooledit (or additionally).
It fails in line int(new_text) with new text = "5.000" for example.

linuxcnc/lib/python/gladevcp/tooledit_widget.py

Lines 513 to 523 in bae5676

	if col in(1,2):
	try:
	self.model[path][col] = int(new_text)
	except:
	pass
	# validate input for float columns
	elif col in range(3,15):
	try:
	self.model[path][col] = f"{float(new_text.replace(',', '.')):10.4f}"
	except:
	pass

This would also fix it in this case. But it might be bad if this function gets accidentally a float value which is cut then.

    if col in(1,2):
        try:
            import re
            self.model[path][col] = int(re.split(r'[.,]', new_text)[0])

[Sigma1912]

I noticed that the pocket number column currently allows negative integer values, which I don't think is correct?
So Tool and Pocket number columns will now truncate any float to the absolute integer value. I'm not sure we still need the dot,comma replacement but kept it just in case.

[hansu]

I noticed that the pocket number column currently allows negative integer values, which I don't think is correct?

I don't know. Maybe one want to disable a tool by setting pocket to -1?
I think the bigger problem is that duplicated tool numbers and pocket numbers are allowed.

[grandixximo]

I use duplicate pocket numbers, stacked tools

[Sigma1912]

Just checked in Axis gui (if that is anything to go by):

• allows duplicate pocket numbers
• errors out on negative pocket numbers

And yes, there is definitely a use for duplicate pocket number.

[hansu]

Okay, but duplicated tool numbers? You would never know which one will be used.

[Sigma1912]

Okay, but duplicated tool numbers?

Yes, those should indeed not be allowed.

[grandixximo]

I used programs where the tool ID has to be unique, it is tricky to do it without getting in the way when the user is renumbering tools for whatever reason, some thought need be put into that. Although at the final state there should be no duplicates, when user is wanting to do a renumbering, not having any hard blocks (current behavior) allows to have a moment where there are two tools with the same number; the trick is finding a reasonable way to allow renumbering without getting in the way of the renumbering cleanup flow, some kind of message confirmation, same number already in the table, want to swap number? cancel?
Also when adding a tool, it would be nice to have the next available ID be ready for the user.
Also there could be tool tables with duplicates in the wild, a warning that those need be fixed should show up somewhere somehow without nagging the user too much...

[grandixximo]

abs() silently flips the sign instead of rejecting negative input: typing -3 is stored as +3 with no feedback. Since the stated intent is to match Axis, note that Axis rejects negative pocket numbers with an error rather than flipping the sign. A >= 0 check with reject would match that behavior. This widget is shared with axis, gscreen and qtdragon, so the sign-flip applies to direct cell edits there too. Also, hansu's point about pocket = -1 to disable a tool is still open in the discussion; abs() would make that impossible to express.

[hansu]

I can't say much about the use of pockets as I don't use a tool changer. But A valid point is to reject negative values rather than inverting them.

[grandixximo]

This did not get the same float-tolerant treatment: it still uses plain int(new_text). Direct cell entry of 3.0 is now accepted for tool/pocket but still silently rejected for orientation. Same int(float(...)) pattern, keeping the existing range check, would unify it.

[hansu]

What are the lines you are referring to? I think the above lines not.

[grandixximo]

525-531

[grandixximo]

GitHub displays it weird because is not in the diff

[hansu]

I used programs where the tool ID has to be unique, it is tricky to do it without getting in the way when the user is renumbering tools for whatever reason, some thought need be put into that. Although at the final state there should be no duplicates, when user is wanting to do a renumbering, not having any hard blocks (current behavior) allows to have a moment where there are two tools with the same number; the trick is finding a reasonable way to allow renumbering without getting in the way of the renumbering cleanup flow, some kind of message confirmation, same number already in the table, want to swap number? cancel? Also when adding a tool, it would be nice to have the next available ID be ready for the user. Also there could be tool tables with duplicates in the wild, a warning that those need be fixed should show up somewhere somehow without nagging the user too much...

I could imagine to do the check on save. So it's easy to rename the IDs and have duplicate IDs in between.

[Sigma1912]

Generally I cannot really comment since I don't use the tool table much.
Maybe we also shouldn't silently truncate a float entry to integer either.

Check on save seems the most straight forward to me but we would probably also have to check on load since a tool table can easily be edited outside the gui.

This widget is shared with axis, gscreen and qtdragon

I thought this was only shared by gscreen.