synthetos / TinyG

Affordable Industrial Grade Motion Control
https://github.com/synthetos/TinyG/wiki
895 stars 293 forks source link

G28.3 incorrectly applies 'work' coordinates #137

Closed DanalEstes closed 9 years ago

DanalEstes commented 9 years ago

G28.3 is documented to set the absolute position of an axis (and set that axis to 'homed' status internally). G28.3 functions correctly when no work offsets are active. However, if an offset it active, that offset is incorrectly applied to the actions of the g28.3 command. All examples from edge (440.14) build. When there is no offset in force, everything works:

$mpox
X machine posn:      0.000 mm
tinyg [mm] ok> 
$posx
X position:          0.000 mm
tinyg [mm] ok> 
g0 x5
tinyg [mm] ok> 
posx:0.195,vel:159.43,stat:5
posx:1.562,vel:473.21
posx:3.554,vel:463.22
posx:4.854,vel:138.46
posx:5.000,vel:0.00,stat:3
$mpox
X machine posn:      5.000 mm
tinyg [mm] ok> 
$posx
X position:          5.000 mm
tinyg [mm] ok> 
g28.3 x0
tinyg [mm] ok> 
$mpox
X machine posn:      0.000 mm
tinyg [mm] ok> 
$posx
X position:          0.000 mm
tinyg [mm] ok> 

All correct so far. Now we apply a work offset:

$mpox
X machine posn:      0.000 mm
tinyg [mm] ok> 
$posx
X position:          0.000 mm
tinyg [mm] ok> 
G10 L2 P1 x3
tinyg [mm] ok> 
g54
tinyg [mm] ok> 
$mpox
X machine posn:      0.000 mm
tinyg [mm] ok> 
$posx
X position:         -3.000 mm
tinyg [mm] ok> 
g0 x5
tinyg [mm] ok> 
posx:-2.816,posy:0.000,posz:0.000,posa:0.000,feed:0.00,vel:154.50,unit:1,coor:1,dist:0,frmo:1,stat:5
posx:-1.324,vel:564.62
posx:1.340,vel:683.61
posx:3.846,vel:482.61
posx:4.932,vel:82.80
posx:5.000,vel:0.00,stat:3
$mpox
X machine posn:      8.000 mm
tinyg [mm] ok> 
$posx
X position:          5.000 mm

Everything is still working fine. However, resetting the axis produces an offset result:

g28.3 x0
tinyg [mm] ok> 
$mpox
X machine posn:      3.000 mm
tinyg [mm] ok> 

This is not correct. The documentation states that G28.3 sets a new absolute position (and sets the "homed" status for that axes). It is not supposed to apply the currently active work offset.

aldenhart commented 9 years ago

Thanks so much for this. I won't be able to look at this for about a week, but will pull in on my return. Much appreciated.

DanalEstes commented 9 years ago

After I forked and tested this in TinyG edge, it occured to me to look at the same module in G2. Looks like it has been fixed already, and the exact same way I fixed G. Namely:

// REMOVED  value[axis] = cm.offset[cm.gm.coord_system][axis] + _to_millimeters(origin[axis]);  // G2 Issue #26
            value[axis] = _to_millimeters(origin[axis]);

This only needs pull (or whatever) in G, not G2.

giseburt commented 9 years ago

@DanalEstes @ril3y I have pushed a new master-candidate branch with these changes and a potential fix for #136 as well.

Could you test these, please?

Thank you! -Rob

DanalEstes commented 9 years ago

Will do.

On Fri, Jul 10, 2015 at 9:24 PM, Rob Giseburt notifications@github.com wrote:

@DanalEstes https://github.com/DanalEstes @ril3y https://github.com/ril3y I have pushed a new master-candidate branch with these changes and a potential fix for #136 https://github.com/synthetos/TinyG/pull/136 as well.

Could you test these, please?

Thank you! -Rob

— Reply to this email directly or view it on GitHub https://github.com/synthetos/TinyG/issues/137#issuecomment-120564466.

DanalEstes commented 9 years ago

Quick test on a TinyG with no motors looks great on both fixes. Headed out to the shop to test on a TinyG on a 24x60x5 inch CNC Router.

More in a minute.

On Fri, Jul 10, 2015 at 9:34 PM, Danal Estes danal.estes@gmail.com wrote:

Will do.

On Fri, Jul 10, 2015 at 9:24 PM, Rob Giseburt notifications@github.com wrote:

@DanalEstes https://github.com/DanalEstes @ril3y https://github.com/ril3y I have pushed a new master-candidate branch with these changes and a potential fix for #136 https://github.com/synthetos/TinyG/pull/136 as well.

Could you test these, please?

Thank you! -Rob

— Reply to this email directly or view it on GitHub https://github.com/synthetos/TinyG/issues/137#issuecomment-120564466.

DanalEstes commented 9 years ago

Went to the shop, everything looks great. Did lots of probes in both modes. Everything was absolutely correct. Seems like both of these issues can be closed. THANK YOU.

As a side note, please note that the Chilipeppr probe widget doesn't always work correctly, regardless of 440.14 vs. 440.15. If you decide to test this, it might be a good idea to issue the Gcode commands separately.

And, an observation: The status reports issued while the probe is in motion are in coordinate system 0 (G53). This causes Chilipeppr's DROs to read in machine coords while the probe is moving, and then they "pop" back to work (assuming a work offset is active) when the probe finishes and a coord 1 staus report is issued. There is no right/wrong answer here, it is all just how you want it to work. Just thought you would want to know.

As mentioned, thank you for work on these issues. These fixes are powerful enablers for the newly clarified machine/work coordinate buttons in Chilipeppr and for follow-on work that will be coming soon. And, since I have the opportunity, please allow me to directly thank you guys for TinyG in its entirety. The 24x60x5 in CNC Router that I mentioned earlier was on Mach3/Smoothstepper until about a month ago. Now, it is being controlled by Chilipeppr/TinyG. What a difference! One example to throw some numbers: I could never get the machine to rapid reliably above about 150 i/m on Mach; it now rapids at a little over 450, and is perfectly smooth. Remarkable.

So, double thanks: TinyG as a whole, and these individual fixes.

Danal

P.S. As mentioned in other threads, I am looking forward to linking the "release checker" in Chilipeppr to a sythetos site for the hex files, and deleting mine!

On Fri, Jul 10, 2015 at 10:00 PM, Danal Estes danal.estes@gmail.com wrote:

Quick test on a TinyG with no motors looks great on both fixes. Headed out to the shop to test on a TinyG on a 24x60x5 inch CNC Router.

More in a minute.

On Fri, Jul 10, 2015 at 9:34 PM, Danal Estes danal.estes@gmail.com wrote:

Will do.

On Fri, Jul 10, 2015 at 9:24 PM, Rob Giseburt notifications@github.com wrote:

@DanalEstes https://github.com/DanalEstes @ril3y https://github.com/ril3y I have pushed a new master-candidate branch with these changes and a potential fix for #136 https://github.com/synthetos/TinyG/pull/136 as well.

Could you test these, please?

Thank you! -Rob

— Reply to this email directly or view it on GitHub https://github.com/synthetos/TinyG/issues/137#issuecomment-120564466.

giseburt commented 9 years ago

Thank you @DanalEstes for testing that, and the kind words.

Your ability to go faster with TinyG is likely due to our jerk-managed motion planning. We have put a lot of work into it, so it's very nice to hear about it with such tangible proof!

ril3y commented 9 years ago

Wholly agree with robs comment! Very cool proof thanks for the feedback! Would you mind me using your story in our "propaganda" :)

Riley

On Sat, Jul 11, 2015 at 7:08 PM, Rob Giseburt notifications@github.com wrote:

Thank you @DanalEstes https://github.com/DanalEstes for testing that, and the kind words.

Your ability to go faster with TinyG is likely due to our jerk-managed motion planning. We have put a lot of work into it, so it's very nice to hear about it with such tangible proof!

— Reply to this email directly or view it on GitHub https://github.com/synthetos/TinyG/issues/137#issuecomment-120668591.

DanalEstes commented 9 years ago

Not at all! I'll even send some pics, if you want. A little mini-feature on the blog.

On Sun, Jul 12, 2015 at 9:40 AM, ril3y notifications@github.com wrote:

Wholly agree with robs comment! Very cool proof thanks for the feedback! Would you mind me using your story in our "propaganda" :)

Riley

On Sat, Jul 11, 2015 at 7:08 PM, Rob Giseburt notifications@github.com wrote:

Thank you @DanalEstes https://github.com/DanalEstes for testing that, and the kind words.

Your ability to go faster with TinyG is likely due to our jerk-managed motion planning. We have put a lot of work into it, so it's very nice to hear about it with such tangible proof!

— Reply to this email directly or view it on GitHub https://github.com/synthetos/TinyG/issues/137#issuecomment-120668591.

— Reply to this email directly or view it on GitHub https://github.com/synthetos/TinyG/issues/137#issuecomment-120725907.