Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Get rid of defered enabling of the step interrupt again.

This makes the code cleaner and the reduction of code
probably easily compensates for keeping global interrupts
enabled for a bit longer. Talked to macscifi about this.

Saves about 300 bytes of binary size.
  • Loading branch information...
commit 53490bb318d4fd0de954b7c609c786099fd1532c 1 parent fcd11a6
@Traumflug Traumflug authored
Showing with 29 additions and 52 deletions.
  1. +5 −8 dda.c
  2. +4 −16 dda_queue.c
  3. +20 −26 timer.c
  4. +0 −2  timer.h
View
13 dda.c
@@ -482,8 +482,6 @@ void dda_start(DDA *dda) {
Next, we work out how long until our next step using the selected acceleration algorithm and set the timer.
Then we decide if this was the last step for this move, and if so mark this dda as dead so next timer interrupt we can start a new one.
Finally we de-assert any asserted step pins.
-
- \todo take into account the time that interrupt takes to run
*/
void dda_step(DDA *dda) {
uint8_t endstop_stop; ///< Stop due to endstop trigger
@@ -598,10 +596,11 @@ void dda_step(DDA *dda) {
}
#if STEP_INTERRUPT_INTERRUPTIBLE
- // since we have sent steps to all the motors that will be stepping and the rest of this function isn't so time critical,
- // this interrupt can now be interruptible
- // however we must ensure that we don't step again while computing the below, so disable *this* interrupt but allow others to fire
-// disableTimerInterrupt();
+ // Since we have sent steps to all the motors that will be stepping
+ // and the rest of this function isn't so time critical, this interrupt
+ // can now be interruptible by other interrupts.
+ // The step interrupt is disabled before entering dda_step() to ensure
+ // that we don't step again while computing the below.
sei();
#endif
@@ -695,8 +694,6 @@ void dda_step(DDA *dda) {
else
steptimeout = 0;
- cli();
-
#ifdef ACCELERATION_RAMPING
// we don't hit maximum speed exactly with acceleration calculation, so limit it here
// the nice thing about _not_ setting dda->c to dda->c_min is, the move stops at the exact same c as it started, so we have to calculate c only once for the time being
View
20 dda_queue.c
@@ -74,13 +74,10 @@ void queue_step() {
current_movebuffer->live = current_movebuffer->waitfor_temp = 0;
serial_writestr_P(PSTR("Temp achieved\n"));
}
-
- #if STEP_INTERRUPT_INTERRUPTIBLE
- sei();
- #endif
}
else {
- // NOTE: dda_step makes this interrupt interruptible after steps have been sent but before new speed is calculated.
+ // NOTE: dda_step makes this interrupt interruptible for some time,
+ // see STEP_INTERRUPT_INTERRUPTIBLE.
dda_step(current_movebuffer);
}
}
@@ -135,18 +132,9 @@ void enqueue_home(TARGET *t, uint8_t endstop_check, uint8_t endstop_stop_cond) {
SREG = save_reg;
if (isdead) {
- timer1_compa_deferred_enable = 0;
next_move();
- if (timer1_compa_deferred_enable) {
- uint8_t save_reg = SREG;
- cli();
- CLI_SEI_BUG_MEMORY_BARRIER();
-
- TIMSK1 |= MASK(OCIE1A);
-
- MEMORY_BARRIER();
- SREG = save_reg;
- }
+ // Compensate for the cli() in setTimer().
+ sei();
}
}
View
46 timer.c
@@ -41,8 +41,6 @@ volatile uint8_t clock_flag_10ms = 0;
volatile uint8_t clock_flag_250ms = 0;
volatile uint8_t clock_flag_1s = 0;
-volatile uint8_t timer1_compa_deferred_enable = 0;
-
/// comparator B is the system clock, happens every TICK_TIME
ISR(TIMER1_COMPB_vect) {
// set output compare register to the next clock tick
@@ -83,7 +81,6 @@ ISR(TIMER1_COMPA_vect) {
// disable this interrupt. if we set a new timeout, it will be re-enabled when appropriate
TIMSK1 &= ~MASK(OCIE1A);
- timer1_compa_deferred_enable = 0;
// stepper tick
queue_step();
@@ -92,19 +89,7 @@ ISR(TIMER1_COMPA_vect) {
#ifdef DEBUG_LED_PIN
WRITE(DEBUG_LED_PIN, 0);
#endif
-
- // Enable the timer1_compa interrupt, if needed,
- // but only do it after disabling global interrupts.
- // This will cause push any possible timer1a interrupt
- // to the far side of the return, protecting the
- // stack from recursively clobbering memory.
-
- cli();
- CLI_SEI_BUG_MEMORY_BARRIER();
-
- if (timer1_compa_deferred_enable) {
- TIMSK1 |= MASK(OCIE1A);
- }
+
return;
}
@@ -136,17 +121,20 @@ void timer_init()
}
#ifdef HOST
-/// specify how long until the step timer should fire
+/*! Specify how long until the step timer should fire.
+ \param delay in CPU ticks
+
+ 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 setTimer(uint32_t delay)
{
// save interrupt flag
uint8_t sreg = SREG;
uint16_t step_start = 0;
-
- // disable interrupts
- cli();
- CLI_SEI_BUG_MEMORY_BARRIER();
-
+
// re-enable clock interrupt in case we're recovering from emergency stop
TIMSK1 |= MASK(OCIE1B);
@@ -184,14 +172,20 @@ void setTimer(uint32_t delay)
OCR1A = step_start;
}
- // Defer the enabling of the timer1_CompA interrupts.
-
- timer1_compa_deferred_enable = 1;
+ // Enable this interrupt, but only do it after disabling
+ // global interrupts. This will cause push any possible
+ // timer1a interrupt to the far side of the return, protecting the
+ // stack from recursively clobbering memory.
+ cli();
+ CLI_SEI_BUG_MEMORY_BARRIER();
+ TIMSK1 |= MASK(OCIE1A);
+
} else {
+ // TODO: as the interrupt is designed to fire only once,
+ // doing a setTimer(0) should be obsolete.
// flag: move has ended
next_step_time = 0;
TIMSK1 &= ~MASK(OCIE1A);
- timer1_compa_deferred_enable = 0;
}
// restore interrupt flag
View
2  timer.h
@@ -15,8 +15,6 @@ extern volatile uint8_t clock_flag_10ms;
extern volatile uint8_t clock_flag_250ms;
extern volatile uint8_t clock_flag_1s;
-extern volatile uint8_t timer1_compa_deferred_enable;
-
// If the specific bit is set, execute the following block exactly once
// and then clear the flag.
#define ifclock(F) for (;F;F=0 )
Please sign in to comment.
Something went wrong with that request. Please try again.