From 9dd7426f9d6a0f691f9168353c3b028899553ba8 Mon Sep 17 00:00:00 2001 From: Markus Hitter Date: Sun, 1 Mar 2015 23:25:09 +0100 Subject: [PATCH] DDA: on short schedules, repeat step routine immediately ... ... instead of trying to fire an interrupt as quickly as possible. This affects ACCELERATION_TEMPORAL only. It almost doubles the achievable step rate. Measured maximum step rate (X axis only, 100 mm moves) is 40'000 steps/s on a 16 MHz electronics, so approx. 50'000 steps/s on a 20 MHz controller, which is even a bit faster than the ACCELERATION_RAMPING algorithm. Tests with temporary test code were run and judging by these tests, clock interrupts are now very reliable up to the point where processing speed is simply exhaused. Performance with ACCELERATION_RAMPING: this costs 10 bytes binary size and exactly 2 clock cycles per step interrupt or 0.6% performance even. We could avoid this with a lot of #ifdefs, but considering ACCELERATION_TEMPORAL will one day be the default acceleration, skip these #ifdefs, also for better code readability. $ cd testcases $ ./run-in-simulavr.sh short-moves.gcode smooth-curves.gcode triangle-odd.gcode SIZES ATmega... '168 '328(P) '644(P) '1280 FLASH : 20528 bytes 144% 67% 33% 16% RAM : 2188 bytes 214% 107% 54% 27% EEPROM : 32 bytes 4% 2% 2% 1% short-moves.gcode statistics: LED on occurences: 838. LED on time minimum: 304 clock cycles. LED on time maximum: 715 clock cycles. LED on time average: 310.717 clock cycles. smooth-curves.gcode statistics: LED on occurences: 8585. LED on time minimum: 309 clock cycles. LED on time maximum: 712 clock cycles. LED on time average: 360.051 clock cycles. triangle-odd.gcode statistics: LED on occurences: 1636. LED on time minimum: 304 clock cycles. LED on time maximum: 710 clock cycles. LED on time average: 332.32 clock cycles. --- dda.c | 92 ++++++++++++++++++++++++++++++++--------------------- dda_queue.c | 4 +-- timer.c | 66 +++++++++++++++++++++++--------------- timer.h | 2 +- 4 files changed, 98 insertions(+), 66 deletions(-) diff --git a/dda.c b/dda.c index 4ec4ff0..a7c5412 100644 --- a/dda.c +++ b/dda.c @@ -527,7 +527,7 @@ void dda_start(DDA *dda) { dda->live = 1; // set timeout for first step - timer_set(dda->c); + timer_set(dda->c, 0); } // else just a speed change, keep dda->live = 0 @@ -644,47 +644,63 @@ void dda_step(DDA *dda) { later. In turn we promise here to send a maximum of four such short-delays consecutively and to give sufficient time on average. */ - uint32_t c_candidate; - uint8_t i; + // This is the time which led to this call of dda_step(). + move_state.last_time = move_state.time[dda->axis_to_step] + + dda->step_interval[dda->axis_to_step]; - if (dda->axis_to_step == X) { - x_step(); - move_state.steps[X]--; - move_state.time[X] += dda->step_interval[X]; - move_state.last_time = move_state.time[X]; - } - if (dda->axis_to_step == Y) { - y_step(); - move_state.steps[Y]--; - move_state.time[Y] += dda->step_interval[Y]; - move_state.last_time = move_state.time[Y]; - } - if (dda->axis_to_step == Z) { - z_step(); - move_state.steps[Z]--; - move_state.time[Z] += dda->step_interval[Z]; - move_state.last_time = move_state.time[Z]; - } - if (dda->axis_to_step == E) { - e_step(); - move_state.steps[E]--; - move_state.time[E] += dda->step_interval[E]; - move_state.last_time = move_state.time[E]; - } + do { + uint32_t c_candidate; + enum axis_e i; - dda->c = 0xFFFFFFFF; - for (i = X; i < AXIS_COUNT; i++) { - if (move_state.steps[i]) { - c_candidate = move_state.time[i] + dda->step_interval[i] - move_state.last_time; - if (c_candidate < dda->c) { - dda->axis_to_step = i; - dda->c = c_candidate; + if (dda->axis_to_step == X) { + x_step(); + move_state.steps[X]--; + move_state.time[X] += dda->step_interval[X]; + } + if (dda->axis_to_step == Y) { + y_step(); + move_state.steps[Y]--; + move_state.time[Y] += dda->step_interval[Y]; + } + if (dda->axis_to_step == Z) { + z_step(); + move_state.steps[Z]--; + move_state.time[Z] += dda->step_interval[Z]; + } + if (dda->axis_to_step == E) { + e_step(); + move_state.steps[E]--; + move_state.time[E] += dda->step_interval[E]; + } + unstep(); + + // Find the next stepper to step. + dda->c = 0xFFFFFFFF; + for (i = X; i < AXIS_COUNT; i++) { + if (move_state.steps[i]) { + c_candidate = move_state.time[i] + dda->step_interval[i] - + move_state.last_time; + if (c_candidate < dda->c) { + dda->axis_to_step = i; + dda->c = c_candidate; + } } } - } - #endif + + // No stepper to step found? Then we're done. + if (dda->c == 0xFFFFFFFF) { + dda->live = 0; + dda->done = 1; + break; + } + } while (timer_set(dda->c, 1)); + + #endif /* ACCELERATION_TEMPORAL */ // If there are no steps left or an endstop stop happened, we have finished. + // + // TODO: with ACCELERATION_TEMPORAL this duplicates some code. See where + // dda->live is zero'd, about 10 lines above. if ((move_state.steps[X] == 0 && move_state.steps[Y] == 0 && move_state.steps[Z] == 0 && move_state.steps[E] == 0) #ifdef ACCELERATION_RAMPING @@ -709,7 +725,9 @@ void dda_step(DDA *dda) { } else { psu_timeout = 0; - timer_set(dda->c); + #ifndef ACCELERATION_TEMPORAL + timer_set(dda->c, 0); + #endif } // turn off step outputs, hopefully they've been on long enough by now to register with the drivers diff --git a/dda_queue.c b/dda_queue.c index a3580bb..1e149a8 100644 --- a/dda_queue.c +++ b/dda_queue.c @@ -82,7 +82,7 @@ void queue_step() { DDA* current_movebuffer = &movebuffer[mb_tail]; if (current_movebuffer->live) { if (current_movebuffer->waitfor_temp) { - timer_set(HEATER_WAIT_TIMEOUT); + timer_set(HEATER_WAIT_TIMEOUT, 0); if (temp_achieved()) { current_movebuffer->live = current_movebuffer->done = 0; serial_writestr_P(PSTR("Temp achieved\n")); @@ -165,7 +165,7 @@ void next_move() { if (current_movebuffer->waitfor_temp) { serial_writestr_P(PSTR("Waiting for target temp\n")); current_movebuffer->live = 1; - timer_set(HEATER_WAIT_TIMEOUT); + timer_set(HEATER_WAIT_TIMEOUT, 0); } else { dda_start(current_movebuffer); diff --git a/timer.c b/timer.c index 0342bad..db09ef9 100644 --- a/timer.c +++ b/timer.c @@ -101,16 +101,39 @@ void timer_init() { /*! Specify how long until the step timer should fire. \param delay in CPU ticks + \param check_short tells wether to check for impossibly short requests. This + should be set to 1 for calls from the step interrupt. Short requests + then return 1 and do not schedule a timer interrupt. The calling code + usually wants to handle this case. + + Calls from elsewhere should set it to 0. In this case a timer + interrupt is always scheduled. At the risk that this scheduling + doesn't delay the requested time, but up to a full timer counter + overflow ( = 65536 / F_CPU = 3 to 4 milliseconds). + + \return a flag wether the requested time was too short to allow scheduling + an interrupt. This is meaningful for ACCELERATION_TEMPORAL, where + requested delays can be zero or even negative. In this case, the + calling code should repeat the stepping code immediately and also + assume the timer to not change his idea of when the last step + happened. + + Strategy of this timer is to schedule timer interrupts not starting at the + time of the call, but starting at the time of the previous timer interrupt + fired. This ignores the processing time taken in the step interrupt so far, + offering smooth and even step distribution. Flipside of this coin is, + schedules issued at an arbitrary time can result in drastically wrong delays. + See also discussion of parameter check_short and the return value. + This enables the step interrupt, but also disables interrupts globally. So, if you use it from inside the step interrupt, make sure to do so as late as possible. If you use it from outside the step interrupt, do a sei() after it to make the interrupt actually fire. */ -void timer_set(uint32_t delay) { +char timer_set(int32_t delay, char check_short) { uint16_t step_start = 0; #ifdef ACCELERATION_TEMPORAL uint16_t current_time; - uint32_t earliest_time, actual_time; #endif /* ACCELERATION_TEMPORAL */ // An interrupt would make all our timing calculations invalid, @@ -126,33 +149,22 @@ void timer_set(uint32_t delay) { next_step_time = delay; #ifdef ACCELERATION_TEMPORAL - // 300 = safe number of cpu cycles until the interrupt actually happens - current_time = TCNT1; - earliest_time = (uint32_t)current_time + 300; - if (current_time < step_start) // timer counter did overflow recently - earliest_time += 0x00010000; - actual_time = (uint32_t)step_start + next_step_time; + if (check_short) { + current_time = TCNT1; - // Setting the interrupt earlier than it can happen obviously doesn't - // make sense. To keep the "belongs to one move" idea, add an extra, - // remember this extra and compensate the extra if a longer delay comes in. - if (earliest_time > actual_time) { - step_extra_time += (earliest_time - actual_time); - next_step_time = earliest_time - (uint32_t)step_start; - } - else if (step_extra_time) { - if (step_extra_time < actual_time - earliest_time) { - next_step_time -= step_extra_time; - step_extra_time = 0; - } - else { - step_extra_time -= (actual_time - earliest_time); - next_step_time -= (actual_time - earliest_time); - } - } + // 200 = safe number of cpu cycles after current_time to allow a new + // interrupt happening. This is mostly the time needed to complete the + // current interrupt. + if ((current_time - step_start) + 200 > delay) + return 1; + } #endif /* ACCELERATION_TEMPORAL */ - // Now we know how long we actually want to delay, so set the timer. + // From here on we assume the requested delay is long enough to allow + // completion of the current interrupt before the next one is about to + // happen. + + // Now we know how long we actually want to delay, so set the timer. if (next_step_time < 65536) { // set the comparator directly to the next real step OCR1A = (next_step_time + step_start) & 0xFFFF; @@ -177,6 +189,8 @@ void timer_set(uint32_t delay) { // Tell simulator sim_timer_set(); #endif + + return 0; } /// stop timers - emergency stop diff --git a/timer.h b/timer.h index c185ef3..a7118e1 100644 --- a/timer.h +++ b/timer.h @@ -26,7 +26,7 @@ timer stuff */ void timer_init(void) __attribute__ ((cold)); -void timer_set(uint32_t delay); +char timer_set(int32_t delay, char check_short); void timer_stop(void);