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.

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,
+		// Since we have sent steps to all the motors that will be stepping
-		// this interrupt can now be interruptible
+		// and the rest of this function isn't so time critical, this interrupt
-		// however we must ensure that we don't step again while computing the below, so disable *this* interrupt but allow others to fire
+		// can now be interruptible by other interrupts.
-// 		disableTimerInterrupt();
+		// 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

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) {
+		// Compensate for the cli() in setTimer().
-			uint8_t save_reg = SREG;
+		sei();
-			cli();
-			CLI_SEI_BUG_MEMORY_BARRIER();
-			
-			TIMSK1 |= MASK(OCIE1A);
-			
-			MEMORY_BARRIER();
-			SREG = save_reg;
-		}
 	}
 }
 

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();
@@ -93,18 +90,6 @@ ISR(TIMER1_COMPA_vect) {
 			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.
+		// 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);
 
-		timer1_compa_deferred_enable = 1;
 	} 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

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 )