g2core crashes with M0/M2/M3/M7/... + feedhold

rkoe commented 6 years ago

g2core crashes completely (no more communication, no blinking RX-LED on Arduino Due) when the program contains M0 and a feedhold is issued.

Example:

G-code:
```
$xam=1
G0X0
G0X100
M0
```
issue a feedhold during movement

communication log:

{"r":{"fv":0.99,"fb":100.26,"fbs":"100.26-10-g1ff7","fbc":"settings_default.h","hp":3,"hv":0,"id":"0084-e739-18c6-08c","msg":"SYSTEM READY"},"f":[1,0,1]}
$xam=1
G0X0
G0X100
M0
{"r":{"am":1},"f":[1,0,7]}
{"r":{},"f":[1,0,5]}
{"r":{},"f":[1,0,7]}
{"r":{},"f":[1,0,3]}
{"sr":{"line":0,"posx":2.501,"posy":0,"posz":0,"posa":0,"feed":0,"vel":1000,"unit":1,"coor":1,"dist":0,"admo":1,"frmo":1,"momo":0,"stat":5}}
{"sr":{"posx":6.701}}
{"sr":{"posx":10.901}}
!
{"sr":{"posx":15.076,"vel":893.75,"stat":6}}

The sr-line after the "!" is sometimes there, sometimes not. Afterwards, g2core has crashed.

justinclift commented 6 years ago

Just to double check, this is with the latest commit of the edge branch yeah? :smile:

rkoe commented 6 years ago

I've used the master-branch.
I've tested more: It's not only M0, but maybe all M-codes. I've tested it with M0, M2, M3, M7. If a M-code is in the g2core-buffer and a feedhold is issued, g2core crashes. The minimal example and communication log is exactly as above (but with M0 replaced by M2/M3/M7).

justinclift commented 6 years ago

Ahhh, that's sounding like it narrows down the problem area pretty well. :smile:

aldenhart commented 6 years ago

I was unable to reproduce this on edge, so the problem is probably isolated to master. I think the best approach is to promote edge in the near future.

Please post any experiences you might have on edge.

justinclift commented 6 years ago

@rkoe As a data point, a bunch of stuff has been commited to the edge branch over the last few days, so if you have time to check with that it'd be nifty. :smile:

amx1 commented 6 years ago

@aldenhart , @justinclift

I checked this on 101.02-17-g51f9-dirty too. There was no error.

$sys
$xam=1
G0X0
G0X100
M0
! (after two seconds)
~ 

{"r":{"sys":{"fb":101.03,"fv":0.99,"fbs":"101.02-17-g51f9-dirty","fbc":"settings_cnc3040.h","hp":"ArduinoDue","hv":"na","id":"0084-8ce7-0084-6bd","jt"
:0.75,"ct":0.01,"zl":0,"sl":0,"lim":0,"saf":0,"m48":1,"froe":0,"fro":1,"troe":0,"tro":1,"mt":2,"tv":1,"ej":1,"jv":2,"qv":1,"sv":1,"si":250,"gpl":0,"gu
n":1,"gco":1,"gpa":2,"gdi":0}},"f":[1,0,5]}                                                                                                           
{"r":{"am":1},"f":[1,0,7]}                                                                                                                            
{"r":{},"f":[1,0,5]}                                                                                                                                  
{"qr":47}                                                                                                                                             
{"r":{},"f":[1,0,7]}                                                                                                                                  
{"qr":46}                                                                                                                                             
{"sr":{"vel":2308.38,"stat":5,"cycs":1,"mots":1,"mpox":97.32324,"posx":97.32324}}                                                                     
{"sr":{"vel":4500,"mpox":81.08478,"posx":80.97244}}                                                                                                   
{"r":{},"f":[1,0,3]}                                                                                                                                  
{"qr":45}                                                                                                                                             
{"sr":{"mpox":62.43576,"posx":62.32341}}                                                                                                              
{"sr":{"vel":4499.24,"stat":6,"hold":4,"mpox":43.78759,"posx":43.67538}}                                                                              
{"sr":{"vel":1425.8,"mpox":29.65761,"posx":29.62272}}                                                                                                 
{"qr":10}                                                                                                                                             
{"qr":12}                                                                                                                                             
{"sr":{"vel":0,"cycs":0,"mots":0,"hold":0,"momo":4,"mpox":28.46641,"posx":28.46641}}                                                                  
{"qr":9}                                                                                                                                              
{"qr":12}                                                                                                                                             
{"sr":{"vel":2280.78,"stat":5,"cycs":1,"mots":1,"momo":0,"mpox":25.78458,"posx":25.78458}}                                                            
{"sr":{"vel":4045.56,"mpox":10.47403,"posx":10.47403}}                                                                                                
{"sr":{"vel":521.38,"mpox":0.28926,"posx":0.28926}}                                                                                                   
{"qr":46}                                                                                                                                             
{"sr":{"vel":884.6,"mpox":0.62871,"posx":0.62871}}                                                                                                    
{"sr":{"vel":4459,"mpox":13.01242,"posx":13.01242}}                                                                                                   
{"sr":{"vel":4500,"mpox":31.65363,"posx":31.65363}}                                                                                                   
{"sr":{"mpox":50.30265,"posx":50.30265}}                                                                                                              
{"sr":{"mpox":68.95168,"posx":68.95168}}                                                                                                              
{"sr":{"vel":4441.53,"mpox":87.58383,"posx":87.69449}}                                                                                                
{"sr":{"vel":792.61,"mpox":99.51252,"posx":99.51252}}                                                                                                 
{"qr":47}                                                                                                                                             
{"qr":48}                                                                                                                                             
{"sr":{"vel":0,"stat":3,"cycs":0,"mots":0,"mpox":100,"posx":100}}                                                                                     
{"sr":{"stat":3}}

justinclift commented 6 years ago

Thanks @amx1. :smile:

It sounds like whatever this bug is, it's fixed (or now hidden) by changes in the edge branch.

justinclift commented 6 years ago

Not sure... since it seems like it's not present in edge (where our development/fix code goes anyway), should we consider this issue as "fixed" and close this?

amx1 commented 6 years ago

I don't know if this needs more testing

justinclift commented 6 years ago

k. We should probably leave it open for now then. :smile:

aldenhart commented 6 years ago

I had tested it as well and found that it was not a problem. I think we should close this once we confirm that it's not an issue on master, either.

hamedty commented 2 years ago

I can confirm this issue still exists on the latest edge. The way I reproduce it is: 1- send:

G1 Z100 F1000
M100 ({uda0:"0xaaaa"})

2- issue a feed hold and a cycle-start in the middle of the movement.

3- send: M100 ({uda0:"0xbbbb"})

and you wont see 0xbbbb. The weird thing is if you send 0xbbbb two more times it will eventually work! and also if you dont send 0xaaaa in step 1, everything will be fine!

hamedty commented 2 years ago

Also this has something to do with FEEDHOLD_TYPE_ACTIONS. FEEDHOLD_TYPE_HOLD is fine.

hamedty commented 2 years ago

sorry for spamming you guys, but this is how far I went:

by calling feed hold actions, we enter p2. So _enter_p2 -> planner_reset -> jc.reset(); and so M100 read and write pointers are reset.

I don't know how to resolve the issue and why we need to jc.reset to enter P2.

ril3y commented 2 years ago

thanks

hamedty commented 2 years ago

Hi Riley, I didn't actually solve the issue. I am just trying to point everyone to where I think is the root cause:

If we have 2 planners (p2), we also need 2 jc buffer. I think jc.reset() in the middle of hold is a wrong thing to do.

I personally commented that line and make sure that no one issues json command in P2. but that is a workaround for my condition. That is not a global solution.

justinclift commented 2 years ago

Ahhh. Re-opening then. :smile:

synthetos / g2

g2core crashes with M0/M2/M3/M7/... + feedhold #346