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 initial extruder direction for second/third/fourth extruder if they run inverted to the first #3161

Merged
merged 1 commit into from
Mar 16, 2016
Merged

Conversation

Alex9779
Copy link
Contributor

In #3144 I described my problem and here is my solution.

To sum up the problem:
The current implementation does not have any possibility to initialise the stepper interrupt for every extruder. So the interrupt uses the direction for the E axis of the first extruder.
If you now do not change the directions but use a second/third/fourth extruder which is inverted then this extruder runs the wrong direction initially until you send a command to do a negative extraction. Then the routine to set the stepper directions is called again and the direction of the extruder is set to the right value.

My fix just lets the interrupt remember the last used extruder initialised with zero and checks that last_extruder against the active_extruder of the current_block.
That way the set_stepper_directions routine is called a bit more frequently (but not so often like what I describe in #3144 where I proposed just to disable the whole condition and let the routine run every time) but the direction of other extruder is correct right from the beginning without any need to have some negative extrusion set in startup script or something else.

@@ -48,6 +48,7 @@ block_t* current_block; // A pointer to the block currently being traced
// Variables used by The Stepper Driver Interrupt
static unsigned char out_bits = 0; // The next stepping-bits to be output
static unsigned int cleaning_buffer_counter;
static unsigned char last_extruder = 0;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used only in trapezoid_generator_reset(). So does not need to be global.. Declare it in the function.

@Blue-Marlin
Copy link
Contributor

Is called only once per block. Looks ok to me.

@Alex9779
Copy link
Contributor Author

Dumb question: if I declare it in the function how would it be preserved when the function exits to test it when the function is called again on the next block?

@Alex9779
Copy link
Contributor Author

Oh ok got it... You want me to declare it static in the function...

@@ -574,8 +574,11 @@ void set_stepper_direction() {
// block begins.
FORCE_INLINE void trapezoid_generator_reset() {

if (current_block->direction_bits != out_bits) {
static unsigned char last_extruder = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might as well make it (a) int8_t for consistency, and (b) initialize it to -1 so that even extruder 0 will be initialized on the first move.

static int8_t last_extruder = -1;

Also, before it gets merged, if you can squash all the commits down to a single commit using git rebase -i RCBugFix with "fixup" that would help make it a little cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(a) Sorry but where would I be able to see that it should be that type? I looked in planner.h where the struct for the block is defined and there it is an unsigned char...
(b) Well yeah ok that would be super correct even if the first extruder is set correct when starting the interrupt.
[c] Sure I can rebase if we discussed all the stuff :)

@thinkyhead
Copy link
Member

The static looks funny inside an inline but apparently that's totally cool with C++!

thinkyhead added a commit that referenced this pull request Mar 16, 2016
Fix initial extruder direction for second/third/fourth extruder if they run inverted to the first
@thinkyhead thinkyhead merged commit e7e1866 into MarlinFirmware:RCBugFix Mar 16, 2016
@Alex9779 Alex9779 deleted the fix_InitialExtruderDirection branch March 16, 2016 20:18
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants