makerbot / ReplicatorG

An open-source gcode interpreter for driving RepRaps, Makerbots, and other similar CNC beasties
http://replicat.org
GNU General Public License v2.0
404 stars 226 forks source link

Fix for "dithering" behavior #266

Open cbiffle opened 12 years ago

cbiffle commented 12 years ago

I noticed, after upgrading from 0023 to 0031, that my printer was making rapid Z-axis moves during horizontal motions --- like when laying infill.

I've tracked it down to the code that manages "excess" during rounding. See the test case in the first attached commit for a demonstration. In cases where the printer is moving along 1 or 2 axes, and the 3rd is positioned between steps, RepG currently "dithers" the 3rd axis instead of leaving it in place. If you decompile the S3G (I can send you a script if desired) the behavior becomes obvious.

I think I see what this code was trying to do, but I don't believe it's necessary. RepG maintains all coordinates in absolute G-Code units (typically millimeters) internally, so rounding errors won't accumulate from the discretization to steps. So, I've simply removed the code in question from the two affected drivers (MB4GA and MightyBoard).

In reviewing the MakerBot Operators list, I think this is responsible for several phantom problems people have reported. In particular, this can force a Z-axis hold even when the user has disabled it: the printer is being asked to move (very slightly) on Z during most horizontal moves, so it engages the axis at all times. In my case, the dithered moves can become rapid enough that the Z axis loses steps, ruining the print.

The code is also the cause of two aesthetic problems. First, it makes the printer a lot louder. Second, it can introduce jitter into otherwise straight walls, leading to visible ridges. (This is more obvious on machines with low steps/mm, like my original Gen3 Cupcake.)

You'll note that I've imported a couple of testing support libraries, in particular EasyMock. I know the use of EasyMock can be controversial, but I needed it (or something of equivalent power) to get the drivers under test harness. As I'm sure you're aware, this part of ReplicatorG isn't designed for testability, and EasyMock lets me circumvent that. Some refactoring of the drivers could render it unnecessary.

cbiffle commented 12 years ago

45dc178 got pushed to this branch, but it's not related to the bug I described above. It does fix an annoying bug in the same driver, though, so pull it if you like.

FarMcKon commented 12 years ago

Hey, I'm going to pull in some of those as cherry-picks, and I may leave some behind. I don't think dithering removal will be dropped in the immediate next release, but it should make the release after that.

@Override corrections are going in now (thx for doing that, we are a bit understaffed, and that change was made in a rush). I'm also leaving the deprecated code for another 3-6 months as tombstones for other branches doing integration or updates.

Thanks for spotting that, and fixing it, and sending a pull request. Those changes will get rolled in shortly!

cbiffle commented 12 years ago

Happy to help, thanks for taking the changes! I've got another branch in progress where I'm cleaning up the Driver interface -- run a dead code analyzer over Sanguino3GDriver, it shows some important code never getting used. I'll let you know when it's all fixed.