search

hzeller / beagleg

G-code interpreter and stepmotor controller for crazy fast coordinated moves of up to 8 steppers. Uses the Programmable Realtime Unit (PRU) of the Beaglebone.
http://beagleg.org/
GNU General Public License v3.0
122 stars 51 forks source link

Test and fix for moving non-euclidean axes in arcs #61

Closed lucalenardi closed 2 years ago

lucalenardi commented 2 years ago

We are using beagleg (thanks for this super nice project, btw!) to drive stepper motors systems for industrial machines. In most cases, the system is made by XYZ axis, plus one or more axes to drive single or multiple extruders (A, B, E etc). Those tools needs to be synced with the motion during the process, that's why interpolating A, B... axes with X, Y, Z is actually needed.

It works perfectly for points and lines, but not with arcs, where non-euclidean axes are ignored.

This pull request tries to solve this issue by adding a test (with an arc on the XY plane and a linear motion for A) and a small fix in arc_gen.cc to make it pass.

codecov-commenter commented 2 years ago

Codecov Report

Merging #61 (8a5d8e3) into main (53d2597) will increase coverage by 0.05%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   45.79%   45.84%   +0.05%     
==========================================
  Files          37       37              
  Lines        4188     4192       +4     
==========================================
+ Hits         1918     1922       +4     
  Misses       2270     2270
Impacted Files Coverage Δ
src/gcode-parser/arc-gen.cc 97.75% <100.00%> (+0.10%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

hzeller commented 2 years ago

How do you define the additional axis in the G2 or G3 call ?

I guess you can just give it an A position as target position, but I think that is never tested anywhere. So I suggest to also add a test for that in gcode-parser_test.cc so that this feature is documented.

hzeller commented 2 years ago

I suppose the axes other than X, Y and Z would also be good to interpolate for the G5 and G5.1 spline features. I am pretty sure it has the same problem that you fixed for the circles. If you want to have a follow-up pull request :)

hzeller commented 2 years ago

to fix the formatting problem the continuous integration complains about, just run

.github/bin/run-clang-format.sh
hzeller commented 2 years ago

Alright, I checked: the gcode parser was also not taking the additional axis into account. That is now fixed in head (this was the change ... including some unrelated clean-up and improved tests).

I have added a section in the test that now fails (so this is why it is #if-def'ed out), but should pass after your change is in.

lucalenardi commented 2 years ago

I suppose the axes other than X, Y and Z would also be good to interpolate for the G5 and G5.1 spline features. I am pretty sure it has the same problem that you fixed for the circles. If you want to have a follow-up pull request :)

Yes definitely something I will look at.

hzeller commented 2 years ago

Thanks! Merged.

You now have the honor of removing the #if 0 around here, as it will now be working https://github.com/hzeller/beagleg/blob/1cf62595bf927bc221f991adc547cbe8eeac329f/src/gcode-parser/gcode-parser_test.cc#L700-L704