From 9dda3349be2e410d550123f5cd0cdf217294c75c Mon Sep 17 00:00:00 2001 From: Markus Hitter Date: Mon, 5 Mar 2012 21:32:20 +0100 Subject: [PATCH] Revert the new relative handling for X, Y and Z. These were commits 9dbfa7217e0de8b140846ab480d6b4a7fc9b6791 and 2b596cb05e621ed822071486f812eb334328267a. There are several reasons why this new approach didn't work out well: - The machine coordinate system is lost on relative movements. OK, we could keep tracking it, but this would mean even more code, so even more chances for bugs. - With the lost coordinate system, no software endstops are possible. - Neither of X, Y, Z will ever overflow. - If a movement planner would appear one day, he'd have to handle relative movements as well. Even more code duplication. --- dda.c | 48 +++++++-------------- dda.h | 1 - gcode_parse.c | 4 +- gcode_parse.h | 2 + gcode_process.c | 111 ++++++++++++++++++++---------------------------- home.c | 12 +++--- 6 files changed, 73 insertions(+), 105 deletions(-) diff --git a/dda.c b/dda.c index db77f67..9dc85d1 100644 --- a/dda.c +++ b/dda.c @@ -225,41 +225,25 @@ void dda_create(DDA *dda, TARGET *target) { // TODO TODO: We should really make up a loop for all axes. // Think of what happens when a sixth axis (multi colour extruder) // appears? - if (target->all_relative) { - x_delta_um = labs(target->X); - um_to_steps_x(dda->x_delta, x_delta_um); - dda->x_direction = (target->X >= 0)?1:0; + x_delta_um = (uint32_t)labs(target->X - startpoint.X); + y_delta_um = (uint32_t)labs(target->Y - startpoint.Y); + z_delta_um = (uint32_t)labs(target->Z - startpoint.Z); - y_delta_um = labs(target->Y); - um_to_steps_y(dda->y_delta, y_delta_um); - dda->y_direction = (target->Y >= 0)?1:0; + um_to_steps_x(steps, target->X); + dda->x_delta = labs(steps - startpoint_steps.X); + startpoint_steps.X = steps; + um_to_steps_y(steps, target->Y); + dda->y_delta = labs(steps - startpoint_steps.Y); + startpoint_steps.Y = steps; + um_to_steps_z(steps, target->Z); + dda->z_delta = labs(steps - startpoint_steps.Z); + startpoint_steps.Z = steps; - z_delta_um = labs(target->Z); - um_to_steps_z(dda->z_delta, z_delta_um); - dda->z_direction = (target->Z >= 0)?1:0; - } - else { - x_delta_um = (uint32_t)labs(target->X - startpoint.X); - y_delta_um = (uint32_t)labs(target->Y - startpoint.Y); - z_delta_um = (uint32_t)labs(target->Z - startpoint.Z); + dda->x_direction = (target->X >= startpoint.X)?1:0; + dda->y_direction = (target->Y >= startpoint.Y)?1:0; + dda->z_direction = (target->Z >= startpoint.Z)?1:0; - um_to_steps_x(steps, target->X); - dda->x_delta = labs(steps - startpoint_steps.X); - startpoint_steps.X = steps; - um_to_steps_y(steps, target->Y); - dda->y_delta = labs(steps - startpoint_steps.Y); - startpoint_steps.Y = steps; - um_to_steps_z(steps, target->Z); - dda->z_delta = labs(steps - startpoint_steps.Z); - startpoint_steps.Z = steps; - - dda->x_direction = (target->X >= startpoint.X)?1:0; - dda->y_direction = (target->Y >= startpoint.Y)?1:0; - dda->z_direction = (target->Z >= startpoint.Z)?1:0; - } - - // This if() matches Sprinter's behaviour as of March 2012. - if (target->all_relative || target->e_relative) { + if (target->e_relative) { e_delta_um = labs(target->E); um_to_steps_e(dda->e_delta, e_delta_um); dda->e_direction = (target->E >= 0)?1:0; diff --git a/dda.h b/dda.h index 508893f..e96b57f 100644 --- a/dda.h +++ b/dda.h @@ -106,7 +106,6 @@ typedef struct { int32_t E; uint32_t F; - uint8_t all_relative :1; ///< bool: relative coordinates? uint8_t e_relative :1; ///< bool: e axis relative? Overrides all_relative } TARGET; diff --git a/gcode_parse.c b/gcode_parse.c index 2c63649..592aaf0 100644 --- a/gcode_parse.c +++ b/gcode_parse.c @@ -346,10 +346,10 @@ void gcode_parse_char(uint8_t c) { next_target.seen_G = 1; next_target.G = 1; - if (next_target.target.all_relative) { + if (next_target.option_all_relative) { next_target.target.X = next_target.target.Y = next_target.target.Z = 0; } - if (next_target.target.all_relative || next_target.target.e_relative) { + if (next_target.option_all_relative || next_target.option_e_relative) { next_target.target.E = 0; } } diff --git a/gcode_parse.h b/gcode_parse.h index 298eb17..0fac353 100644 --- a/gcode_parse.h +++ b/gcode_parse.h @@ -40,6 +40,8 @@ typedef struct { uint8_t seen_checksum :1; ///< seen a checksum? uint8_t seen_semi_comment :1; ///< seen a semicolon? uint8_t seen_parens_comment :1; ///< seen an open parenthesis + uint8_t option_all_relative :1; ///< relative or absolute coordinates? + uint8_t option_e_relative :1; ///< same for e axis (M82/M83) uint8_t option_inches :1; ///< inches or millimeters? }; uint16_t flags; diff --git a/gcode_process.c b/gcode_process.c index 268b4be..86b3a44 100644 --- a/gcode_process.c +++ b/gcode_process.c @@ -41,7 +41,7 @@ uint8_t next_tool; #if E_STARTSTOP_STEPS > 0 /// move E by a certain amount at a certain speed static void SpecialMoveE(int32_t e, uint32_t f) { - TARGET t = { 0L, 0L, 0L, e, f, 1, 1 }; + TARGET t = { 0L, 0L, 0L, e, f, 1 }; enqueue(&t); } #endif /* E_STARTSTOP_STEPS > 0 */ @@ -59,66 +59,44 @@ static void SpecialMoveE(int32_t e, uint32_t f) { void process_gcode_command() { uint32_t backup_f; + // convert relative to absolute + if (next_target.option_all_relative) { + next_target.target.X += startpoint.X; + next_target.target.Y += startpoint.Y; + next_target.target.Z += startpoint.Z; + } + + // E relative movement. + // Matches Sprinter's behaviour as of March 2012. + if (next_target.option_all_relative || next_target.option_e_relative) + next_target.target.e_relative = 1; + else + next_target.target.e_relative = 0; + // implement axis limits #ifdef X_MIN - if (next_target.target.all_relative) { - if (next_target.target.X += startpoint.X < X_MIN * 1000.) - next_target.target.X = X_MIN * 1000. + startpoint.X; - } - else { - if (next_target.target.X < X_MIN * 1000.) - next_target.target.X = X_MIN * 1000.; - } + if (next_target.target.X < X_MIN * 1000.) + next_target.target.X = X_MIN * 1000.; #endif #ifdef X_MAX - if (next_target.target.all_relative) { - if (next_target.target.X += startpoint.X > X_MAX * 1000.) - next_target.target.X = X_MAX * 1000. - startpoint.X; - } - else { - if (next_target.target.X > X_MAX * 1000.) - next_target.target.X = X_MAX * 1000.; - } + if (next_target.target.X > X_MAX * 1000.) + next_target.target.X = X_MAX * 1000.; #endif #ifdef Y_MIN - if (next_target.target.all_relative) { - if (next_target.target.Y += startpoint.Y < Y_MIN * 1000.) - next_target.target.Y = Y_MIN * 1000. + startpoint.Y; - } - else { - if (next_target.target.Y < Y_MIN * 1000.) - next_target.target.Y = Y_MIN * 1000.; - } + if (next_target.target.Y < Y_MIN * 1000.) + next_target.target.Y = Y_MIN * 1000.; #endif #ifdef Y_MAX - if (next_target.target.all_relative) { - if (next_target.target.Y += startpoint.Y > Y_MAX * 1000.) - next_target.target.Y = Y_MAX * 1000. - startpoint.Y; - } - else { - if (next_target.target.Y > Y_MAX * 1000.) - next_target.target.Y = Y_MAX * 1000.; - } + if (next_target.target.Y > Y_MAX * 1000.) + next_target.target.Y = Y_MAX * 1000.; #endif #ifdef Z_MIN - if (next_target.target.all_relative) { - if (next_target.target.Z += startpoint.Z < Z_MIN * 1000.) - next_target.target.Z = Z_MIN * 1000. + startpoint.Z; - } - else { - if (next_target.target.Z < Z_MIN * 1000.) - next_target.target.Z = Z_MIN * 1000.; - } + if (next_target.target.Z < Z_MIN * 1000.) + next_target.target.Z = Z_MIN * 1000.; #endif #ifdef Z_MAX - if (next_target.target.all_relative) { - if (next_target.target.Z += startpoint.Z > Z_MAX * 1000.) - next_target.target.Z = Z_MAX * 1000. - startpoint.Z; - } - else { - if (next_target.target.Z > Z_MAX * 1000.) - next_target.target.Z = Z_MAX * 1000.; - } + if (next_target.target.Z > Z_MAX * 1000.) + next_target.target.Z = Z_MAX * 1000.; #endif @@ -276,12 +254,14 @@ void process_gcode_command() { //? All coordinates from now on are absolute relative to the origin //? of the machine. This is the RepRap default. //? + //? If you ever want to switch back and forth between relative and + //? absolute movement keep in mind, X, Y and Z follow the machine's + //? coordinate system while E doesn't change it's position in the + //? coordinate system on relative movements. + //? - // No queue_wait() needed. - next_target.target.all_relative = 0; - // TODO: What's about startpoint, next_target an friends? - // What should a machine do when returning from relative - // to absolute mode? + // No wait_queue() needed. + next_target.option_all_relative = 0; break; case 91: @@ -292,8 +272,8 @@ void process_gcode_command() { //? All coordinates from now on are relative to the last position. //? - // No queue_wait() needed. - next_target.target.all_relative = 1; + // No wait_queue() needed. + next_target.option_all_relative = 1; break; case 92: @@ -319,9 +299,7 @@ void process_gcode_command() { axisSelected = 1; } if (next_target.seen_E) { - #ifdef E_ABSOLUTE - startpoint.E = next_target.target.E; - #endif + startpoint.E = next_target.target.E; axisSelected = 1; } @@ -424,21 +402,26 @@ void process_gcode_command() { case 82: //? --- M82 - Set E codes absolute --- //? - //? This is the default. M82/M83 is not documented in the - //? RepRap wiki, behaviours was taken from Sprinter as of March 2012. + //? This is the default and overrides G90/G91. + //? M82/M83 is not documented in the RepRap wiki, behaviour + //? was taken from Sprinter as of March 2012. + //? + //? While E does relative movements, it doesn't change its + //? position in the coordinate system. See also comment on G90. + //? // No wait_queue() needed. - next_target.target.e_relative = 0; + next_target.option_e_relative = 0; break; - // M83 - Set E codes relative while in Absolute Coordinates (G90) mode case 83: //? --- M83 - Set E codes relative --- //? //? Counterpart to M82. + //? // No wait_queue() needed. - next_target.target.e_relative = 1; + next_target.option_e_relative = 1; break; // M84- stop idle hold diff --git a/home.c b/home.c index 443b72a..325b1c3 100644 --- a/home.c +++ b/home.c @@ -34,7 +34,7 @@ void home() { /// find X MIN endstop void home_x_negative() { #if defined X_MIN_PIN - TARGET t = { 0L, 0L, 0L, 0L, 0L, 1, 1 }; + TARGET t = startpoint; t.X = -1000000; #ifdef SLOW_HOMING @@ -70,7 +70,7 @@ void home_x_positive() { #warning X_MAX_PIN defined, but not X_MAX. home_x_positive() disabled. #endif #if defined X_MAX_PIN && defined X_MAX - TARGET t = { 0L, 0L, 0L, 0L, 0L, 1, 1 }; + TARGET t = startpoint; t.X = +1000000; #ifdef SLOW_HOMING @@ -104,7 +104,7 @@ void home_x_positive() { /// fund Y MIN endstop void home_y_negative() { #if defined Y_MIN_PIN - TARGET t = { 0L, 0L, 0L, 0L, 0L, 1, 1 }; + TARGET t = startpoint; t.Y = -1000000; #ifdef SLOW_HOMING @@ -140,7 +140,7 @@ void home_y_positive() { #warning Y_MAX_PIN defined, but not Y_MAX. home_y_positive() disabled. #endif #if defined Y_MAX_PIN && defined Y_MAX - TARGET t = { 0L, 0L, 0L, 0L, 0L, 1, 1 }; + TARGET t = startpoint; t.Y = +1000000; #ifdef SLOW_HOMING @@ -174,7 +174,7 @@ void home_y_positive() { /// find Z MIN endstop void home_z_negative() { #if defined Z_MIN_PIN - TARGET t = { 0L, 0L, 0L, 0L, 0L, 1, 1 }; + TARGET t = startpoint; t.Z = -1000000; #ifdef SLOW_HOMING @@ -211,7 +211,7 @@ void home_z_positive() { #warning Z_MAX_PIN defined, but not Z_MAX. home_z_positive() disabled. #endif #if defined Z_MAX_PIN && defined Z_MAX - TARGET t = { 0L, 0L, 0L, 0L, 0L, 1, 1 }; + TARGET t = startpoint; t.Z = +1000000; #ifdef SLOW_HOMING