-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
Fix initial extruder direction for second/third/fourth extruder if they run inverted to the first #3161
Conversation
@@ -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; | |||
|
There was a problem hiding this comment.
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.
Is called only once per block. Looks ok to me. |
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? |
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
The |
Fix initial extruder direction for second/third/fourth extruder if they run inverted to the first
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.