crowbx.c

@@ -17,6 +17,12 @@ enum _programs {
 };
 static uint8_t current_program = NUM_PGM;
 
+void (*pgm_init[NUM_PGM])(void) = {
+	poly_init,
+	poly2_init,
+	mono_init,
+};
+
 void (*note_on[NUM_PGM])(uint8_t n) = {
 	poly_note_on,
 	poly2_note_on,
@@ -80,17 +86,7 @@ program_change(uint8_t pgm)
 		gate_off(v);
 	}
 
-	switch (pgm) {
-	case PGM_POLY:
-		poly_init();
-		return;
-	case PGM_POLY2:
-		poly2_init();
-		return;
-	case PGM_MONO:
-		mono_init();
-		return;
-	}
+	(*pgm_init[current_program])();
 }
 
 int
@@ -109,38 +105,33 @@ main(void)
 		(*update_clock[current_program])(clock - prev_clock);
 		prev_clock = clock;
 
-		if (!midi_read()) {
+		uint8_t status, data1, data2;
+		if (midi_read(&status, &data1, &data2) == MIDI_READ_NO_DATA) {
 			continue;
 		}
 
-		uint16_t data = midi_data();
-
-		if (midi_status() == MIDI_NOTE_ON) {
-			uint8_t note = data >> 8;
-			uint8_t velocity = data & 0xff;
-
-			if (velocity > 0) {
-				(*note_on[current_program])(note);
+		switch (status) {
+		case MIDI_NOTE_ON:
+			if (data2 > 0) {
+				(*note_on[current_program])(data1);
 			} else {
-				(*note_off[current_program])(note);
+				(*note_off[current_program])(data1);
 			}
-		} else if (midi_status() == MIDI_NOTE_OFF) {
-			uint8_t note = data >> 8;
-			(*note_off[current_program])(note);
-		} else if (midi_status() == MIDI_PROGRAM_CHANGE) {
-			uint8_t pgm = data >> 8;
-			program_change(pgm);
-		} else if (midi_status() == MIDI_CONTROL_CHANGE) {
-			uint8_t ctl = data >> 8;
-			uint8_t val = data & 0xff;
-			(*control_change[current_program])(ctl, val);
-		} else if (midi_status() == MIDI_PITCH_BEND) {
-			uint8_t lsb = data >> 8;
-			uint8_t msb = data & 0xff;
-			(*pitch_bend[current_program])((msb << 7) | lsb);
+			break;
+		case MIDI_NOTE_OFF:
+			(*note_off[current_program])(data1);
+			break;
+		case MIDI_PROGRAM_CHANGE:
+			program_change(data1);
+			break;
+		case MIDI_CONTROL_CHANGE:
+			(*control_change[current_program])(data1, data2);
+			break;
+		case MIDI_PITCH_BEND:
+			(*pitch_bend[current_program])((data2 << 7) | data1);
+			break;
 		}
 	}
 
 	return 0;
 }
-

list.h

@@ -5,8 +5,8 @@
 
 struct list {
 	uint8_t head;
-	uint8_t *array;
-	uint8_t sup;
+	uint8_t * const array;
+	const uint8_t sup;
 };
 
 #define INIT_LIST(name, size) \

midi-test.c

@@ -101,14 +101,14 @@ test_program_change(void)
 	assert_status(MIDI_PROGRAM_CHANGE, 1, 0x1200);
 
 	// a little silly, running status for program change messages
-	midi_parse(0);
-	assert_status(MIDI_PROGRAM_CHANGE, 1, 0);
+	midi_parse(3);
+	assert_status(MIDI_PROGRAM_CHANGE, 1, 0x0300);
 
 	// throw in a different message
 	midi_parse(0x80); // note off
 	midi_parse(0x12);
-	midi_parse(0);
-	assert_status(MIDI_NOTE_OFF, 1, 0x1200);
+	midi_parse(0x56);
+	assert_status(MIDI_NOTE_OFF, 1, 0x1256);
 
 	midi_parse(0xc0);
 	midi_parse(0x34);

midi-test.h

@@ -6,11 +6,17 @@
 // Access to midi.c internals
 void status_init(void);
 void midi_parse(uint8_t);
-uint8_t midi_available(void);
+uint8_t midi_load(uint8_t *status, uint8_t *data1, uint8_t *data2);
 
-#define assert_status(status, avail, data) assert(midi_status() == status); \
-	assert(!midi_available() == !avail); \
-	if (avail) assert(midi_data() == data)
+#define assert_status(status, avail, data) { \
+	uint8_t _status, _avail, _data1, _data2; \
+	_avail = midi_load(&_status, &_data1, &_data2); \
+	assert((_avail != MIDI_READ_OK) == !avail); \
+	if (_avail == MIDI_READ_OK) { \
+		assert(((_data1 << 8) | _data2) == data); \
+		assert(_status == status); \
+	} \
+}
 
 // Fake uart functions to let midi.c compile
 void uart_init(void) { }

midi.c

@@ -8,6 +8,7 @@ UART I/O and MIDI parsing.
 
 #include "midi.h"
 #include "uart.h"
+#include <string.h>
 
 uint8_t midi_channel;
 
@@ -18,7 +19,12 @@ midi_set_channel(uint8_t channel)
 }
 
 uint8_t status;
-uint16_t data;
+
+enum {
+	MIDI_MAX_DATA_LEN = 2,
+};
+
+uint8_t data[MIDI_MAX_DATA_LEN];
 uint8_t parsed_len;
 
 const uint8_t statuses[MIDI_NUM_STATUSES] = {
@@ -43,7 +49,7 @@ void
 clear_data()
 {
 	parsed_len = 0;
-	data = 0;
+	memset(data, 0, sizeof(data)); // TODO why is this necessary?
 }
 
 uint8_t
@@ -52,13 +58,6 @@ midi_status(void)
 	return status;
 }
 
-uint16_t midi_data(void)
-{
-	uint16_t result = data;
-	clear_data();
-	return result;
-}
-
 uint8_t
 midi_available(void)
 {
@@ -91,26 +90,19 @@ midi_parse(uint8_t midi_byte)
 		return;
 	}
 
-	if (midi_byte & 0x80) {
+	if (midi_byte >= 128) {
+		// We were expecting a 7-bit midi data value. Something is wrong.
 		reset_status();
 		clear_data();
 		return;
 	}
 
 	if (parsed_len >= data_len(status)) {
+		// Data overflow.
 		clear_data();
 	}
 
-	switch (parsed_len) {
-	case 0:
-		data = midi_byte << 8;
-		parsed_len++;
-		break;
-	case 1:
-		data |= midi_byte;
-		parsed_len++;
-		break;
-	}
+	data[parsed_len++] = midi_byte;
 }
 
 void
@@ -128,14 +120,30 @@ midi_init()
 }
 
 uint8_t
-midi_read(void)
+midi_load(uint8_t *status, uint8_t *data1, uint8_t *data2)
+{
+	if (!midi_available()) {
+		return MIDI_READ_NO_DATA;
+	}
+
+	*status = midi_status();
+	*data1 = data[0];
+	*data2 = data[1];
+	clear_data();
+
+	return MIDI_READ_OK;
+}
+
+uint8_t
+midi_read(uint8_t *status, uint8_t *data1, uint8_t *data2)
 {
 	uint16_t uart_data = uart_read();
 	if (uart_data >> 8) {
 		// uart error
-		return 0;
+		return MIDI_READ_NO_DATA;
 	}
 
 	midi_parse(uart_data & 0xff);
-	return midi_available();
+
+	return midi_load(status, data1, data2);
 }

midi.h

@@ -18,10 +18,14 @@ enum _cc {
 	CC_VIB_RATE = 76,
 };
 
-uint8_t midi_read(void);
 void midi_init(void);
 void midi_set_channel(uint8_t channel);
-uint8_t midi_status(void);
-uint16_t midi_data(void);
+
+enum {
+	MIDI_READ_OK,
+	MIDI_READ_NO_DATA,
+};
+
+uint8_t midi_read(uint8_t *status, uint8_t *data1, uint8_t *data2);
 
 #endif