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

Fix for "dithering" behavior #266

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

cbiffle
Copy link

@cbiffle cbiffle commented Mar 9, 2012

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 and others added 5 commits March 8, 2012 18:37
The Makerbot-derived printer drivers contain a math error that
manifests several different ways: Z-hold unexpectedly engaged during
a print; wobbles on otherwise straight edges; increased loss of
steps (including Z steps) during rapid motion, as in fills.

This test case demonstrates the bug.  The next commit fixes it.
This fixes the dithering problem; the Makerbot4GAlternateDriverTest
now passes.
Which, incidentally, doesn't pass.
I've added @OverRide annotations to all overrides.  This revealed
a bug: the enableMotor/disableMotor, despite being documented as
overrides, weren't.  As a result, the Sanguino3GDriver behavior
was being invoked, generating DC motor control messages even when
the axis has been hijacked for a stepper extruder.  Because this
driver reuses the same motor control channel for the stepper fan,
this meant the fan turned on...until the first motion.  Then it
started toggling on and off with the stepper motor.

That's kind of silly, so with this change it obeys the flag passed
to enableStepperExtruderFan(boolean).
@cbiffle
Copy link
Author

cbiffle commented Mar 9, 2012

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
Copy link
Contributor

FarMcKon commented Mar 9, 2012

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
Copy link
Author

cbiffle commented Mar 10, 2012

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.

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.

2 participants