From d27dd13a54badd0f8ecb21b3c06dbde7a2ea8cd3 Mon Sep 17 00:00:00 2001 From: Sonny Jeon Date: Sun, 15 Jan 2012 18:25:12 -0700 Subject: [PATCH] Fix bug with premature step end. Refactored _delay_ms() and square() for better portability. - Fixed a premature step end bug dating back to Simen's 0.7b edge version is fixed, from which this code is forked from. Caused by Timer2 constantly overflowing calling the Step Reset Interrupt every 128usec. Now Timer2 is always disabled after a step end and should free up some cycles for the main program. Could be more than one way to fix this problem. I'm open to suggestions. - _delay_ms() refactored to accept only constants to comply with current compilers. square() removed since not available with some compilers. --- config.h | 2 +- main.c | 2 +- motion_control.c | 2 +- nuts_bolts.c | 8 ++++++++ nuts_bolts.h | 3 +++ planner.c | 11 ++++++----- script/stream.py | 1 + stepper.c | 38 ++++++++++++++++++-------------------- 8 files changed, 39 insertions(+), 28 deletions(-) diff --git a/config.h b/config.h index ac68acb..9f44602 100644 --- a/config.h +++ b/config.h @@ -108,7 +108,7 @@ // this delay will increase the maximum dwell time linearly, but also reduces the responsiveness of // run-time command executions, like status reports, since these are performed between each dwell // time step. Also, keep in mind that the Arduino delay timer is not very accurate for long delays. -#define DWELL_TIME_STEP 50 // Integer (milliseconds) +#define DWELL_TIME_STEP 50 // Integer (1-255) (milliseconds) // ----------------------------------------------- diff --git a/main.c b/main.c index 0ddd410..06b03b8 100644 --- a/main.c +++ b/main.c @@ -39,9 +39,9 @@ system_t sys; int main(void) { // Initialize system - sei(); // Enable interrupts serial_init(BAUD_RATE); // Setup serial baud rate and interrupts st_init(); // Setup stepper pins and interrupt timers + sei(); // Enable interrupts memset(&sys, 0, sizeof(sys)); // Clear all system variables sys.abort = true; // Set abort to complete initialization diff --git a/motion_control.c b/motion_control.c index 176accc..eba3346 100644 --- a/motion_control.c +++ b/motion_control.c @@ -182,7 +182,7 @@ void mc_dwell(double seconds) { uint16_t i = floor(1000/DWELL_TIME_STEP*seconds); plan_synchronize(); - _delay_ms(floor(1000*seconds-i*DWELL_TIME_STEP)); // Delay millisecond remainder + delay_ms(floor(1000*seconds-i*DWELL_TIME_STEP)); // Delay millisecond remainder while (i > 0) { // NOTE: Check and execute runtime commands during dwell every <= DWELL_TIME_STEP milliseconds. protocol_execute_runtime(); diff --git a/nuts_bolts.c b/nuts_bolts.c index 8a6960c..5e018f7 100644 --- a/nuts_bolts.c +++ b/nuts_bolts.c @@ -21,6 +21,7 @@ #include "nuts_bolts.h" #include #include +#include int read_double(char *line, uint8_t *char_counter, double *double_ptr) { @@ -36,3 +37,10 @@ int read_double(char *line, uint8_t *char_counter, double *double_ptr) return(true); } + +// Delays variable defined milliseconds. Compiler compatibility fix for _delay_ms(), +// which only accepts constants in future compiler releases. +void delay_ms(uint16_t ms) +{ + while ( ms-- ) { _delay_ms(1); } +} \ No newline at end of file diff --git a/nuts_bolts.h b/nuts_bolts.h index f1b867f..945129c 100644 --- a/nuts_bolts.h +++ b/nuts_bolts.h @@ -83,4 +83,7 @@ extern system_t sys; // a pointer to the result variable. Returns true when it succeeds int read_double(char *line, uint8_t *char_counter, double *double_ptr); +// Delays variable-defined milliseconds. Compiler compatibility fix for _delay_ms(). +void delay_ms(uint16_t ms); + #endif diff --git a/planner.c b/planner.c index 6e09945..579116f 100644 --- a/planner.c +++ b/planner.c @@ -376,8 +376,8 @@ void plan_buffer_line(double x, double y, double z, double feed_rate, uint8_t in delta_mm[X_AXIS] = (target[X_AXIS]-pl.position[X_AXIS])/settings.steps_per_mm[X_AXIS]; delta_mm[Y_AXIS] = (target[Y_AXIS]-pl.position[Y_AXIS])/settings.steps_per_mm[Y_AXIS]; delta_mm[Z_AXIS] = (target[Z_AXIS]-pl.position[Z_AXIS])/settings.steps_per_mm[Z_AXIS]; - block->millimeters = sqrt(square(delta_mm[X_AXIS]) + square(delta_mm[Y_AXIS]) + - square(delta_mm[Z_AXIS])); + block->millimeters = sqrt(delta_mm[X_AXIS]*delta_mm[X_AXIS] + delta_mm[Y_AXIS]*delta_mm[Y_AXIS] + + delta_mm[Z_AXIS]*delta_mm[Z_AXIS]); double inverse_millimeters = 1.0/block->millimeters; // Inverse millimeters to remove multiple divides // Calculate speed in mm/minute for each axis. No divide by zero due to previous checks. @@ -476,9 +476,10 @@ void plan_set_current_position(double x, double y, double z) { // To correlate status reporting work position correctly, the planner must force the steppers to // empty the block buffer and synchronize with the planner, as the real-time machine position and - // the planner position at the end of the buffer are different. This will only be called with a - // G92 is executed, which typically is used only at the beginning of a g-code program. - // TODO: Find a robust way to avoid a planner synchronize, but may require a bit of ingenuity. + // the planner position at the end of the buffer can be and are usually different. This function is + // only called with a G92, which typically is used only at the beginning of a g-code program or + // between different operations. + // TODO: Find a robust way to avoid a planner synchronize, but this may require a bit of ingenuity. plan_synchronize(); // Update the system coordinate offsets from machine zero diff --git a/script/stream.py b/script/stream.py index 10189fd..17cded3 100755 --- a/script/stream.py +++ b/script/stream.py @@ -19,6 +19,7 @@ import re import time import sys import argparse +# import threading RX_BUFFER_SIZE = 128 diff --git a/stepper.c b/stepper.c index 248c7e6..4ebda7b 100644 --- a/stepper.c +++ b/stepper.c @@ -130,30 +130,25 @@ static uint8_t iterate_trapezoid_cycle_counter() // config_step_timer. It pops blocks from the block_buffer and executes them by pulsing the stepper pins appropriately. // It is supported by The Stepper Port Reset Interrupt which it uses to reset the stepper port after each pulse. // The bresenham line tracer algorithm controls all three stepper outputs simultaneously with these two interrupts. -// NOTE: ISR_NOBLOCK allows SIG_OVERFLOW2 to trigger on-time regardless of time in this handler. - -// TODO: ISR_NOBLOCK is the same as the old SIGNAL with sei() method, but is optimizable by the compiler. On -// an oscilloscope there is a weird hitch in the step pulse during high load operation. Very infrequent, but -// when this does happen most of the time the pulse falling edge is randomly delayed by 20%-50% of the total -// intended pulse time, but sometimes it pulses less than 3usec. The former likely caused by the serial -// interrupt doing its thing, not that big of a deal, but the latter cause is unknown and worrisome. Need -// to track down what is causing this problem. Functionally, this shouldn't cause any noticeable issues -// as long as stepper drivers have a pulse minimum of 1usec or so (Pololu and any Allegro IC are ok). -// ** This seems to be an inherent issue that dates all the way back to Simen's v0.6b or earlier. ** - -ISR(TIMER1_COMPA_vect,ISR_NOBLOCK) +ISR(TIMER1_COMPA_vect) { if (busy) { return; } // The busy-flag is used to avoid reentering this interrupt - busy = true; // Set the direction pins a couple of nanoseconds before we step the steppers STEPPING_PORT = (STEPPING_PORT & ~DIRECTION_MASK) | (out_bits & DIRECTION_MASK); // Then pulse the stepping pins STEPPING_PORT = (STEPPING_PORT & ~STEP_MASK) | out_bits; - // Reset step pulse reset timer so that The Stepper Port Reset Interrupt can reset the signal after - // exactly settings.pulse_microseconds microseconds. - TCNT2 = step_pulse_time; - + // Enable step pulse reset timer so that The Stepper Port Reset Interrupt can reset the signal after + // exactly settings.pulse_microseconds microseconds, independent of the main Timer1 prescaler. + TCNT2 = step_pulse_time; // Reload timer counter + TCCR2B = (1<