Moved ringbuffer stuff into serial routine for some significant space and speed savings

mendel/Makefile

@@ -14,7 +14,7 @@
 
 PROGRAM = mendel
 
-SOURCES = $(PROGRAM).c ringbuffer.c serial.c dda.c gcode.c timer.c clock.c temp.c sermsg.c dda_queue.c
+SOURCES = $(PROGRAM).c serial.c dda.c gcode.c timer.c clock.c temp.c sermsg.c dda_queue.c
 
 ##############################################################################
 #                                                                            #

mendel/dda.c

@@ -376,9 +376,11 @@ void dda_step(DDA *dda) {
 		}
 	}
 
+	#if STEP_INTERRUPT_INTERRUPTIBLE
 	// this interrupt can now be interruptible
 	disableTimerInterrupt();
 	sei();
+	#endif
 
 	// this generates too much debug output for all but the slowest step rates
 	if (0 && DEBUG) {
@@ -418,8 +420,4 @@ void dda_step(DDA *dda) {
 	// turn off step outputs, hopefully they've been on long enough by now to register with the drivers
 	// if not, too bad. or insert a (very!) small delay here, or fire up a spare timer or something
 	unstep();
-
-	// reset interruptible so we can return in the same state we started
-	cli();
-	enableTimerInterrupt();
 }

mendel/dda_queue.c

@@ -9,10 +9,11 @@ uint8_t	mb_tail = 0;
 DDA movebuffer[MOVEBUFFER_SIZE];
 
 uint8_t queue_full() {
-	if (mb_tail == 0)
+// 	if (mb_tail == 0)
-		return mb_head == (MOVEBUFFER_SIZE - 1);
+// 		return mb_head == (MOVEBUFFER_SIZE - 1);
-	else
+// 	else
-		return mb_head == (mb_tail - 1);
+// 		return mb_head == (mb_tail - 1);
+	return (((mb_tail - mb_head - 1) & (MOVEBUFFER_SIZE - 1)) == 0)?255:0;
 }
 
 uint8_t queue_empty() {
@@ -44,10 +45,14 @@ void enqueue(TARGET *t) {
 }
 
 void next_move() {
+	#if STEP_INTERRUPT_INTERRUPTIBLE
+	if (!queue_empty()) {
+	#else
 	if (queue_empty()) {
 		disableTimerInterrupt();
 	}
 	else {
+	#endif
 		// next item
 		uint8_t t = mb_tail;
 		t++;

mendel/gcode.c

@@ -505,6 +505,10 @@ void process_gcode_command(GCODE_COMMAND *gcmd) {
 				if (gcmd->seen_S)
 					d_factor = gcmd->S;
 				break;
+			case 133:
+				if (gcmd->seen_S)
+					i_limit = gcmd->S;
+				break;
 
 			// unknown mcode: spit an error
 			default:

mendel/machine.h

@@ -16,8 +16,8 @@
 #define	X_STEPS_PER_REV						3200.0
 #define	Y_STEPS_PER_REV						X_STEPS_PER_REV
 
-// we need far more speed than precision on Z due to the threaded rod drive, turn off microstepping
+// we need far more speed than precision on Z due to the threaded rod drive, maybe just half stepping
-#define	Z_STEPS_PER_REV						200.0
+#define	Z_STEPS_PER_REV						400.0
 
 #define	X_COG_CIRCUMFERENCE				(4.77 * 16.0)
 #define	Y_COG_CIRCUMFERENCE				X_COG_CIRCUMFERENCE
@@ -78,4 +78,15 @@
 // should be the same for all machines! ;)
 #define	PI	3.1415926535
 
+/*
+	firmware build options
+*/
+
+// this option makes the step interrupt interruptible.
+// this should help immensely with dropped serial characters, but may also make debugging infuriating due to the complexities arising from nested interrupts
+#define		STEP_INTERRUPT_INTERRUPTIBLE	1
+
+// Xon/Xoff flow control. Should be redundant
+// #define	XONXOFF
+
 #endif	/* _MACHINE_H */

mendel/ringbuffer.h

@@ -4,14 +4,14 @@
 #include	<stdint.h>
 #include	<avr/interrupt.h>
 
-// ringbuffer head/tail/length precision. change to uint16_t if you want a buffer bigger than 252 bytes or so
+// ringbuffer head/tail/length precision. change to uint16_t if you want a buffer bigger than 250 bytes or so
 #define	RB_BITS	uint8_t
 
 typedef struct {
 	volatile RB_BITS		read_pointer;
 	volatile RB_BITS		write_pointer;
 	volatile RB_BITS		size;
-	volatile RB_BITS		data[];
+	volatile uint8_t		data[];
 } ringbuffer;
 
 // initialize a ringbuffer

mendel/serial.c

@@ -3,14 +3,47 @@
 #include	"ringbuffer.h"
 #include	"arduino.h"
 
-#define		BUFSIZE		64 + sizeof(ringbuffer)
+#define		BUFSIZE			64
-#define		BAUD		38400
+#define		BAUD				38400
 
 #define		ASCII_XOFF	19
 #define		ASCII_XON		17
 
-volatile uint8_t _rx_buffer[BUFSIZE];
+volatile uint8_t rxhead = 0;
-volatile uint8_t _tx_buffer[BUFSIZE];
+volatile uint8_t rxtail = 0;
+volatile uint8_t rxbuf[BUFSIZE];
+
+volatile uint8_t txhead = 0;
+volatile uint8_t txtail = 0;
+volatile uint8_t txbuf[BUFSIZE];
+
+#define	buf_canread(buffer)			((buffer ## head - buffer ## tail    ) & (BUFSIZE - 1))
+#define	buf_canwrite(buffer)		((buffer ## tail - buffer ## head - 1) & (BUFSIZE - 1))
+
+#define	buf_push(buffer, data)	do { buffer ## buf[buffer ## head] = data; buffer ## head = (buffer ## head + 1) & (BUFSIZE - 1); } while (0)
+#define	buf_pop(buffer, data)		do { data = buffer ## buf[buffer ## tail]; buffer ## tail = (buffer ## tail + 1) & (BUFSIZE - 1); } while (0)
+
+/*
+	ringbuffer logic:
+	head = written data pointer
+	tail = read data pointer
+
+	when head == tail, buffer is empty
+	when head + 1 == tail, buffer is full
+	thus, number of available spaces in buffer is (tail - head) & bufsize
+
+	can write:
+	(tail - head - 1) & (BUFSIZE - 1)
+
+	write to buffer:
+	buf[head++] = data; head &= (BUFSIZE - 1);
+
+	can read:
+	(head - tail) & (BUFSIZE - 1)
+
+	read from buffer:
+	data = buf[tail++]; tail &= (BUFSIZE - 1);
+*/
 
 volatile uint8_t flowflags = 0;
 #define		FLOWFLAG_SEND_XOFF	1
@@ -18,38 +51,38 @@ volatile uint8_t flowflags = 0;
 #define		FLOWFLAG_SENT_XOFF	4
 #define		FLOWFLAG_SENT_XON		8
 
-#define	rx_buffer	((ringbuffer *) _rx_buffer)
-#define	tx_buffer	((ringbuffer *) _tx_buffer)
-
 void serial_init()
 {
-	ringbuffer_init(rx_buffer, BUFSIZE);
-	ringbuffer_init(tx_buffer, BUFSIZE);
-
 #if BAUD > 38401
 	UCSR0A = MASK(U2X0);
 #else
 	UCSR0A = 0;
 #endif
-	UCSR0B = (1 << RXEN0) | (1 << TXEN0);
-	UCSR0C = (1 << UCSZ01) | (1 << UCSZ00);
-
 #if BAUD > 38401
 	UBRR0 = (((F_CPU / 8) / BAUD) - 0.5);
 #else
 	UBRR0 = (((F_CPU / 16) / BAUD) - 0.5);
 #endif
 
-	UCSR0B |= (1 << RXCIE0) | (1 << UDRIE0);
+	UCSR0B = MASK(RXEN0) | MASK(TXEN0);
+	UCSR0C = MASK(UCSZ01) | MASK(UCSZ00);
+
+	UCSR0B |= MASK(RXCIE0) | MASK(UDRIE0);
 }
 
+/*
+	Interrupts
+*/
+
 ISR(USART_RX_vect)
 {
-	ringbuffer_writechar(rx_buffer, UDR0);
+	if (buf_canwrite(rx))
+		buf_push(rx, UDR0);
 }
 
 ISR(USART_UDRE_vect)
 {
+	#if XONXOFF
 	if (flowflags & FLOWFLAG_SEND_XOFF) {
 		UDR0 = ASCII_XOFF;
 		flowflags = (flowflags & ~FLOWFLAG_SEND_XOFF) | FLOWFLAG_SENT_XOFF;
@@ -58,48 +91,58 @@ ISR(USART_UDRE_vect)
 		UDR0 = ASCII_XON;
 		flowflags = (flowflags & ~FLOWFLAG_SEND_XON) | FLOWFLAG_SENT_XON;
 	}
-	else if (ringbuffer_canread(tx_buffer))
-		UDR0 = ringbuffer_readchar(tx_buffer);
 	else
-		UCSR0B &= ~(1 << UDRIE0);
+	#endif
+	if (buf_canread(tx)) {
+		buf_pop(tx, UDR0);
+	}
+	else
+		UCSR0B &= ~MASK(UDRIE0);
 }
 
-uint16_t serial_rxchars()
+/*
-{
+	Read
-	return ringbuffer_canread(rx_buffer);
+*/
-}
 
-uint16_t serial_txchars()
+uint8_t serial_rxchars()
 {
-	return ringbuffer_canread(tx_buffer);
+	return buf_canread(rx);
 }
 
 uint8_t serial_popchar()
 {
-	return ringbuffer_readchar(rx_buffer);
+	uint8_t c = 0;
+	if (buf_canread(rx))
+		buf_pop(rx, c);
+	return c;
 }
 
-uint16_t serial_recvblock(uint8_t *block, int blocksize)
+/*
-{
+	Write
-	return ringbuffer_readblock(rx_buffer, block, blocksize);
+*/
-}
+
+// uint8_t serial_txchars()
+// {
+// 	return buf_canwrite(tx);
+// }
 
 void serial_writechar(uint8_t data)
 {
 	// check if interrupts are enabled
 	if (SREG & MASK(SREG_I)) {
 		// if they are, we should be ok to block
-		for (;ringbuffer_canwrite(tx_buffer) == 0;);
+		for (;buf_canwrite(tx) == 0;);
-		ringbuffer_writechar(tx_buffer, data);
+		buf_push(tx, data);
 	}
 	else {
 		// interrupts are disabled- maybe we're in one?
 		// anyway, instead of blocking, only write if we have room
-		if (ringbuffer_canwrite(tx_buffer))
+		if (buf_canwrite(tx)) {
-			ringbuffer_writechar(tx_buffer, data);
+			buf_push(tx, data);
+		}
 	}
 	// enable TX interrupt so we can send this character
-	UCSR0B |= (1 << UDRIE0);
+	UCSR0B |= MASK(UDRIE0);
 }
 
 void serial_writeblock(void *data, int datalen)
@@ -108,11 +151,13 @@ void serial_writeblock(void *data, int datalen)
 		serial_writechar(((uint8_t *) data)[i]);
 }
 
+/*
+	Write from FLASH
+*/
+
 void serial_writechar_P(PGM_P data)
 {
-	for (;ringbuffer_canwrite(tx_buffer) == 0;);
+	serial_writechar(pgm_read_byte(data));
-	ringbuffer_writechar(tx_buffer, pgm_read_byte(data));
-	UCSR0B |= (1 << UDRIE0);
 }
 
 void serial_writeblock_P(PGM_P data, int datalen)
@@ -124,7 +169,7 @@ void serial_writeblock_P(PGM_P data, int datalen)
 void serial_writestr_P(PGM_P data)
 {
 	uint8_t i = 0;
-	// yes, this is *supposed* to be assignment rather than comparison
+	// yes, this is *supposed* to be assignment rather than comparison, so we break when r is assigned zero
 	for (uint8_t r; (r = pgm_read_byte(&data[i])); i++)
 		serial_writechar(r);
 }
@@ -134,12 +179,12 @@ void serial_writestr_P(PGM_P data)
 		if (flowflags & FLOWFLAG_SENT_XOFF)
 			flowflags = FLOWFLAG_SEND_XON;
 		// enable TX interrupt so we can send this character
-		UCSR0B |= (1 << UDRIE0);
+		UCSR0B |= MASK(UDRIE0);
 	}
 
 	void xoff() {
 		flowflags = FLOWFLAG_SEND_XOFF;
 		// enable TX interrupt so we can send this character
-		UCSR0B |= (1 << UDRIE0);
+		UCSR0B |= MASK(UDRIE0);
 	}
 #endif

mendel/serial.h

@@ -8,9 +8,9 @@
 // initialise serial subsystem
 void serial_init(void);
 
-// return number of characters in the receive and send buffer
+// return number of characters in the receive buffer, and number of spaces in the send buffer
-uint16_t serial_rxchars(void);
+uint8_t serial_rxchars(void);
-uint16_t serial_txchars(void);
+// uint8_t serial_txchars(void);
 
 // read one character
 uint8_t serial_popchar(void);
@@ -18,7 +18,7 @@ uint8_t serial_popchar(void);
 void serial_writechar(uint8_t data);
 
 // read/write many characters
-uint16_t serial_recvblock(uint8_t *block, int blocksize);
+// uint8_t serial_recvblock(uint8_t *block, int blocksize);
 void serial_writeblock(void *data, int datalen);
 
 // write from flash

mendel/temp.c

@@ -35,6 +35,7 @@ int16_t		heater_d     = 0;
 int32_t		p_factor			= 680;
 int32_t		i_factor			= 18;
 int32_t		d_factor			= 200;
+int16_t		i_limit	= 500;
 
 uint8_t		temp_flags		= 0;
 #define		TEMP_FLAG_PRESENT		1
@@ -152,10 +153,10 @@ void temp_tick() {
 		// integral
 		heater_i += t_error;
 		// prevent integrator wind-up
-		if (heater_i > I_LIMIT)
+		if (heater_i > i_limit)
-			heater_i = I_LIMIT;
+			heater_i = i_limit;
-		else if (heater_i < -I_LIMIT)
+		else if (heater_i < -i_limit)
-			heater_i = -I_LIMIT;
+			heater_i = -i_limit;
 
 		// derivative
 		// note: D follows temp rather than error so there's no large derivative when the target changes

mendel/temp.h

@@ -8,6 +8,7 @@
 extern int32_t p_factor;
 extern int32_t i_factor;
 extern int32_t d_factor;
+extern int16_t i_limit;
 
 #define		PID_SCALE			1024L
 #define		I_LIMIT				4000

mendel/timer.c

@@ -7,21 +7,28 @@
 #include	"dda.h"
 
 ISR(TIMER1_COMPA_vect) {
-	if (movebuffer[mb_tail].live) {
+	// do our next step
-		// this interrupt can be interruptible
+	// NOTE: dda_step makes this interrupt interruptible after steps have been sent but before new speed is calculated.
-		// TODO: remove when not debugging
+	if (movebuffer[mb_tail].live)
-// 		disableTimerInterrupt();
-// 		sei();
-
 		dda_step(&(movebuffer[mb_tail]));
 
-// 		cli();
+	#if STEP_INTERRUPT_INTERRUPTIBLE
-// 		enableTimerInterrupt();
+		// this interrupt can now be interruptible
-	}
+		disableTimerInterrupt();
+		sei();
+	#endif
 
 	// fall directly into dda_start instead of waiting for another step
 	if (movebuffer[mb_tail].live == 0)
 		next_move();
+
+	#if STEP_INTERRUPT_INTERRUPTIBLE
+		// return from interrupt in a way that prevents this interrupt nesting with itself at high step rates
+		cli();
+		// check queue, if empty we don't need to interrupt again until re-enabled in dda_create
+		if (!queue_empty())
+			enableTimerInterrupt();
+	#endif
 }
 
 void setupTimerInterrupt()