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.

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

Gmoccapy - Patch for Combination M61 + G4 #2489 - Version 2 #2841

Closed
wants to merge 1 commit into from

Conversation

zz912
Copy link
Contributor

@zz912 zz912 commented Jan 8, 2024

This patch ensures that the automatic command G43 is not executed immediately, but when the LCNC is in IDLE mode.

This patch ensures that the automatic command G43 is not executed immediately, but when the LCNC is in IDLE mode.
@zz912 zz912 changed the title Patch for Combination M61 + G4 #2489 - Version 2 Gmoccapy - Patch for Combination M61 + G4 #2489 - Version 2 Jan 8, 2024
@zz912
Copy link
Contributor Author

zz912 commented Jan 8, 2024

I would like to ask @gmoccapy and @hansu to review my patch proposal.

@hansu hansu requested a review from gmoccapy January 9, 2024 21:21
@andypugh
Copy link
Collaborator

The idea as described sounds fine, but I think we need Norbert (@gmoccapy ) to look at the code to be sure it will work without unintended side-effects.

@zz912
Copy link
Contributor Author

zz912 commented Jan 20, 2024

I agree with Andy.
It will be good to wait for Norbert.

@zz912
Copy link
Contributor Author

zz912 commented Mar 3, 2024

I would like to add some more information about what this patch is good for. Currently, Gmoccapy behaves randomly when we want to change the tool with G43 active. Sometimes only a tool change is implemented. Sometimes a tool change is performed and the tool table window is closed.

I made two examples. In both cases I have G43 active and in both cases I change the tool from tool1 to tool10.

set_tool_variant1
set_tool_variant2

@zz912
Copy link
Contributor Author

zz912 commented Mar 23, 2024

@gmoccapy Can we at least add this Pull Request to the master branch? The master branch is still in development and could be tested.

@gmoccapy
Copy link
Collaborator

Please do not add this, I do only know one case where the actual way does not work. I do hope to have time to go on developing the GUI in about one year

@zz912
Copy link
Contributor Author

zz912 commented Mar 31, 2024

@gmoccapy
I was thinking about:
Gmoccapy - Patch for Combination M61 + G4 #2489 - Version 3

Replace MDI commands:

        if "G43" in self.active_gcodes and self.stat.task_mode != linuxcnc.MODE_AUTO:
            self.command.mode(linuxcnc.MODE_MDI)
            self.command.wait_complete()
            self.command.mdi("G43")
            self.command.wait_complete()

By changing the parameters:
'5401-5409' - Tool Offsets for X, Y, Z, A, B, C, U, V & W. Set by G43. Volatile.

There would be no need to deal with problematic MDI mode switching.

Would it be acceptable to you if one of the experienced programmers did it?

I understand that you don't want push PR from an amateur like me. That I'm an amateur is a fact, it's not sarcasm.

@rmu75
Copy link
Contributor

rmu75 commented Aug 6, 2024

can somebody explain what the actual problem is?

@zz912
Copy link
Contributor Author

zz912 commented Aug 6, 2024

It all started here:
#2453
I do not agree that this Issue is closed.

It is race condition.
M61 run:

def _update_toolinfo(self, tool):

In _update_toolinfo is "automatic G43":

if "G43" in self.active_gcodes and self.stat.task_mode != linuxcnc.MODE_AUTO:
self.command.mode(linuxcnc.MODE_MDI)
self.command.wait_complete()
self.command.mdi("G43")
self.command.wait_complete()

Sometimes "mdihistory widget" wins and G4 is executed.
Sometimes "automatic G43" wins and G4 is not executed.

I use
Gmoccapy - Patch for Combination M61 + G4 - Version 4

  1. Remove "automatic G43" from gmoccapy
  2. Added G43 to remap M6 and M61

At this moment I dont recommend merge this patch Version 2, because patch Version 4 is better.

@rmu75
Copy link
Contributor

rmu75 commented Aug 6, 2024

Sorry but I still don't understand. What does M61 have to do with G4? Can you tell what the problem is, in other words, where does gmoccapy something unexpected? What is it that is unexpected and what should happen instead?

@zz912
Copy link
Contributor Author

zz912 commented Aug 7, 2024

Look here: #2489
If you can't play the first video, download it and play it in VLC player. It is not damaged.

You will probably never use M61 + G4 commands in the real world. This combination is only used to simulate bugs.

Expected:
I insert the command "M61 Q5 G4 P10" into the mdihistory widget in Gmoccapy

  1. Tool number 5 is set
  2. There will be a wait of 10 seconds (the wait is indicated by a red circle with a cross on the lower right)

Real:
I insert the command "M61 Q5 G4 P10" into the mdihistory widget

  1. Tool number 5 is set
  2. Sometimes there will be a 10 second wait, sometimes there will not be a 10 second wait

The problem is that this bug is dependent on the PC configuration. Some users are unable to simulate this bug.

What happens to you when you enter:
"M61 Q1 G4 P10"
"M61 Q2 G4 P10"
"M61 Q3 G4 P10"
"M61 Q4 G4 P10"
"M61 Q5 G4 P10"
?
Will the G4 P10 command be executed every time?
Number Q must be changed to trigger the function:

def _update_toolinfo(self, tool):

If you test only:
"M61 Q5 G4 P10"
"M61 Q5 G4 P10"
"M61 Q5 G4 P10"
"M61 Q5 G4 P10"
"M61 Q5 G4 P10"
That way the error won't show up.

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

There is definitely something fishy going on, I can trigger it only in gmoccapy, not in axis. I will investigate what the interpreter / tasl / mot is doing, there can't be a difference between different GUIs IMO. I don't really believe the Dwell is not executed, the problem is probably somewhere else.

One last thing: how does "G43" play into this? I guess that G43 is not executed in the case the dwell is "skipped"?

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

It looks like the interpreter returns/is returned to "idle" without waiting (or without waiting the full time). IMO we should find out how and why that happens and fix that (if possible) and not patch around it for some cases like G43 after tool change.

@gmoccapy
Copy link
Collaborator

gmoccapy commented Aug 7, 2024

@rmu75 As far as I do know, you are not seeing this behaviour in AXIS, as AXIS has no auto G43

If G43 is set once in gmoccapy the GUI will maintain that G-Code active, even after setting a new tool.
May be the interpreter sends its State changes so fast, that gmoccapy misses some time the state change (polling is about 100 ms in gmoccapy)

I did not find a way to simulate the described problem, mostly caused to a lake of time.

Norbert

@zz912
Copy link
Contributor Author

zz912 commented Aug 7, 2024

It looks like the interpreter returns/is returned to "idle" without waiting (or without waiting the full time). IMO we should find out how and why that happens and fix that (if possible) and not patch around it for some cases like G43 after tool change.

I think the interpreter is waiting for NOTHING.
There are more problems here:
https://forum.linuxcnc.org/38-general-linuxcnc-questions/51179-python-interface-makes-race-conditions-mayby#289468
Please read this thread. I spent a lot of time describing and trying to make Interpretr wait for NOTHING.
It would be great if someone with more experience than me could solve this problem.

The problem is not just waiting for G-code commands to be executed. If you want to fix it overall, you also need to solve the waiting for mode switching. For example:

  1. MANUAL Mode
  2. set MDI Mode
  3. run G-code commands
  4. set MANUAL Mode
    If the interpreter were to wait only for the execution of G-code (point 3), point 4 would kill everything.

For example, I did the preparation for stacking HALUI commands here:
#2945

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

@rmu75 As far as I do know, you are not seeing this behaviour in AXIS, as AXIS has no auto G43

If G43 is set once in gmoccapy the GUI will maintain that G-Code active, even after setting a new tool. May be the interpreter sends its State changes so fast, that gmoccapy misses some time the state change (polling is about 100 ms in gmoccapy)

There is something else going on, i tried "M61 Q1 G4 P10" etc..., and the G4 dwell sometimes doesn't happen -- the interp returns to idle "immediately" (it should stay in "waiting for delay"). or is returned. which is to be determined. It probably really is a race condition but for me it is not obvious where / what is involved and if gmoccapy has really anything to do with it. I'm going to the bottom of that (I think I have seen such behaviour on one of my machines) but it will need some time, don't hold your breath.

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

I misspoke somewhat, interp should stay in "reading" and execState should stay in "waiting for delay", but sometimes interp returns to idle and execstate to done without the dwell. Maybe some intermediate steps are involved that I'm currently missing.

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

Ok, now I understand better:

Issuing EMC_TASK_PLAN_EXECUTE --         (  +509,+280,   +18,M61\032Q5\032G4\032P10,)
emcTaskPlanLevel() returned 0
emcTaskPlanExecute(M61 Q5 G4 P10) returned 2
emcTaskPlanLevel() returned 0
emcTaskPlanSetWait() called
emcTaskPlanLevel() returned 0
Issuing EMC_TOOL_SET_NUMBER --   ( +1109,+24,    +0,    +5,)
emcTaskPlanLevel() returned 0
Issuing EMC_TRAJ_DELAY --        (  +219,+360,    +0,10.000000,)
3 2
[Gmoccapy][DEBUG]  RUN (gmoccapy:2619)
[Gmoccapy][DEBUG]  hal signal tool changed (gmoccapy:2638)
[Gmoccapy][DEBUG]  Tool is now 5 (gmoccapy:3497)
[Gmoccapy][DEBUG]  G43 is active (gmoccapy:3499)
Issuing EMC_TASK_SET_MODE --     (  +504,+24,   +19,    +3,)
emcTaskPlanSynch() returned 0
emcTaskPlanLevel() returned 0
Issuing EMC_TASK_PLAN_SYNCH --   (  +516,+24,    +0,)
emcTaskPlanSynch() returned 0
emcTaskPlanClearWait() called
Issuing EMC_TASK_PLAN_EXECUTE --         (  +509,+280,   +20,G43,)
emcTaskPlanLevel() returned 0
emcTaskPlanExecute(G43) returned 0
emcTaskPlanLevel() returned 0
emcTaskPlanLevel() returned 0
Issuing EMC_TRAJ_SET_OFFSET --   (  +223,+424,    +0,0.000000,0.000000,-1.150000,0.000000,0.000000,0.000000,0.000000,0.000000,0.000000,)
mdi_execute_hook: MDI command 'G43' done (remaining: 0)
[Gmoccapy][DEBUG]  IDLE (gmoccapy:2567)

So what happens is

  • gmoccapy sends MDI command
  • tool number is changed
  • gmoccapy on_hal_status_tool_in_spindle_changed is called
  • on_hal_status_tool_in_spindle_changed issues G43
  • G43 cancels G4

So indeed issuing G43 should wait until interp is idle. Issuing G43 should probably happen in the M6 remap anyways.

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

I wonder how this can work at all in case of a remapped M6 -- I guess the tool number is changed only upon successful completion of the remapped procedure.

What remains is gmoccapy can and will cancel macros or multiple commands called from MDI that change the tool / tool number, subject to timing / race condition.

@rmu75
Copy link
Contributor

rmu75 commented Aug 7, 2024

If G43 is set once in gmoccapy the GUI will maintain that G-Code active, even after setting a new tool.
May be the interpreter sends its State changes so fast, that gmoccapy misses some time the state change (polling is about 100 ms in gmoccapy)

gmoccapy doesn't miss a state changes, it reacts too fast, it sends a mode change to the interp that is still occupied with executing the tool change. This mode change cancels / interferes with whatever the interp was doing.

@rmu75
Copy link
Contributor

rmu75 commented Aug 8, 2024

Thinking about it a bit more, that "auto-G43" can't be implemented correctly in gmoccapy (or any GUI really). Just suppose you are executing something like "M61 Q4 F5 G1 Z-10". In this case, after M61 Q4 sets tool number, at some point in the G1 move gmoccapy executes G43, cancelling the move. So this "G43" needs to be sandwiched in between M61 and G1 which is not possible from within the GUI.

So this auto-G43 could be implemented either in the M6 remap (not sure if that's possible for M61) or something has to be done in the interpreter.

@zz912
Copy link
Contributor Author

zz912 commented Aug 8, 2024

"auto-G43" can't be implemented correctly in gmoccapy (or any GUI really).

"auto-G43" can't be implemented correctly in any GUI at THIS TIME. In theory, it would be possible, but tools for determining priorities would have to be created.

  • priority of commands from widget MDI history
  • priority of commands from halui commands
  • priority of command in AUTO mode
  • priority of commands from the python interface
  • priority of commands from remap
    This would be very challenging.

It is a great pity that there is no warning in any log that some G-code commands are arborted.

At THIS TIME, it's really easiest to delete "auto-G43" in Gmoccapy and use remap.

not sure if that's possible for M61

M61 is possible:
http://linuxcnc.org/docs/stable/html/remap/remap.html#_remapping_tool_change_related_codes_t_m6_m61
The question is where is better to place the G43:

  • remap M6 and M61
    or
  • remap T

?
I only use remap M6. It is enough for me.
A friend is using a remap T on a Fanuc.
I don't know which is better.

The solution I'm using now:
deletion of "automatic G43" + use of remap

System solution design:

  • do it for branche master (2.10)
  • delete "automatic G43" in Gmoccapy
  • add a check in Gmoccapy to see if the current correction corresponds to the correction of the selected tool. If not, show the tool correction in red for example
  • write a proper Remap and ask @hansu or @c-morley to integrate it into PNCconf
  • add correct Remap to Gmoccapy sim configuration

@rmu75
Copy link
Contributor

rmu75 commented Aug 8, 2024

The GUI isn't the right place to define machine behaviour anyways. Machine should behave identically independent of selected GUI IMHO.

AFAIK the "T" command only prepares the tool change and could be used ahead of time to e.g. turn the carousel while still doing some machining. Also M61 uses Q. So I would remap M6 and M61 and put G43 there.

I'm not sure what you mean with priority of commands, but "preempting" executing commands with other stuff or manipulating queued commands is not possible / a bad idea IMO.

So I suppose your solution of deactivating "auto-G43" and implementing that in the M6 / M61 remap is the only way to get consistent behaviour. The gmoccapy auto-G43 only works in simple cases and is potentially dangerous.

@zz912
Copy link
Contributor Author

zz912 commented Aug 8, 2024

The gmoccapy auto-G43 only works in simple cases and is potentially dangerous.

It is a very dangerous. In my configuration auto-G43 has already set wrong corrections a few times. Bad corrections mean a 99% crash machine. That's why I spend so much time on this problem.

@zz912
Copy link
Contributor Author

zz912 commented Aug 8, 2024

@zz912
Copy link
Contributor Author

zz912 commented Aug 20, 2024

I would like to ask Robert Schöftner if he would be willing to look into why RETAIN_G43=1 does not work.

It would probably be easier than using the M6 ​​remap.

@rmu75
Copy link
Contributor

rmu75 commented Aug 20, 2024

I did try RETAIN_G43 it and indeed it doesn't seem to work. Unfortunately the code that is involved

if ((settings->active_g_codes[9] == G_43) && ONCE(STEP_RETAIN_G43)) {
isn't easy to understand and there are no test cases.

@zz912
Copy link
Contributor Author

zz912 commented Aug 20, 2024

and there are no test cases.

What do you mean? May I ask for an explanation?

@zz912
Copy link
Contributor Author

zz912 commented Aug 20, 2024

Here are some comments:
59bfd7c

@rmu75
Copy link
Contributor

rmu75 commented Aug 20, 2024

and there are no test cases.

What do you mean? May I ask for an explanation?

there is no test case in tests/ dir that tests RETAIN_G43.

@zz912
Copy link
Contributor Author

zz912 commented Aug 20, 2024

I readed definition of RETAIN_G43 again:
http://linuxcnc.org/docs/stable/html/config/ini-config.html#sub:ini:sec:rs274ngc

RETAIN_G43 = 0 (Default: 0)
When set, you can turn on G43 after loading the first tool, and then not worry about it through the PROGRAM. When you finally unload the last tool, G43 mode is CANCELED.

I think the RETAIN_G43 function is for *.ngc file read only. After the *.ngc program ends, it is deactivated.

Therefore, RETAIN_G43 cannot be tested in MDI or MANUAL code.

In conclusion, I would say that RETAIN_G43 != auto_G43 in Gmoccapy

@andypugh
Copy link
Collaborator

I feel that it would be reasonable to expect RETAIN_G43 to work in MDI too. (and also feel that the GUI has no business at all in affecting this behaviour)

@zz912
Copy link
Contributor Author

zz912 commented Aug 20, 2024

To andy:
Did you read deleted comments in source code?
#2841 (comment)

I think that it works correctly. MDI command is something like 1 program for RETAIN_G43. After execute MDI commad, G49 is activated.
Better definition in manual will be fine.

@andypugh
Copy link
Collaborator

Hmm, OK. But then I don't see the point in RETAIN_G43.
It means that you need a G43 at the beginning of the program anyway, so probably need the postprocessor to be aware of it. At that point the PP will put it in every time. Or, if coding by hand, then you need to remember once, then not bother again.

I have always assumed that RETAIN_G43 was meant to be AUTOMATIC_G43. But clearly not.

@zz912
Copy link
Contributor Author

zz912 commented Aug 20, 2024

I think RETAIN_G43 only makes sense for manually typing G-code. The post-processor automatically places G43 everywhere.

The post-processor can't forget it. A person can forget it.

@zz912
Copy link
Contributor Author

zz912 commented Sep 3, 2024

rmu75:

The GUI isn't the right place to define machine behaviour anyways. Machine should behave identically independent of selected GUI IMHO.

............

So I suppose your solution of deactivating "auto-G43" and implementing that in the M6 / M61 remap is the only way to get consistent behaviour. The gmoccapy auto-G43 only works in simple cases and is potentially dangerous.

I would like to ask @gmoccapy to comment on rmu75's post.

I would like to remove automatic_g43 from Master branche.
I need your approval or another alternative.
Please define under what conditions I could do this.

@gmoccapy
Copy link
Collaborator

gmoccapy commented Sep 3, 2024

You are right, if you mention that the GUI is not the correct place to define machine behavior but also industrial machines behave different depending on the GUI / better said Haidenhain, Siemens, Haas etc.

The Problem is not in automatic programs, as the post processor (being a human very often) is responsible for the G43 handling.

So I would like to mention, why I have introduced the actual behavior. I and some other users are using the machine in semi automatic way. I set my work piece with a 3D zero point setter, after I use MDI commands to mill just one side or a bore etc. If you drill a hole, you use first a center drill, then a drill and last a deburring tool. I have all that tools measured in ma tool table, I just change the tool by hand and I do not have to care about G43.

If you do remove the "auto G43" from gmoccapy we should add at the same time a tool change remap Sim with manual and auto change with auto G43. That must contain also a M61 handling of the G43 settings.

I was until now not able to reproduce the race condition on my machines, so IMHO it is a very very random problem.

But is up to you what to do, as I will only be able to work on gmoccapy in about one year, as I have rebuild a house and need to finish my new workshop to stand up my two new machines. (old workshop is getting to expensive).

Norbert

@zz912
Copy link
Contributor Author

zz912 commented Sep 3, 2024

Thank you for your comment.

I'm closing this PR and starting to work on removing automatic_G43 + M6 M61 remap + its integration into the sim.

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