No more magic numbers! :(((((

avr/Makefile

@@ -22,7 +22,7 @@ HEADERS_LOCAL	=
 HEADERS_BUILD	= $(HEADERS_LOCAL) \
 		  $(addprefix $(INCLUDE_PATH)/$(HEADER_SUBDIR)/,$(HEADERS))
 
-HEADERS		= avr/buffer.h avr/uart.h link.h
+HEADERS		= avr.h avr/uart.h link.h
 OBJS		= send.o recv.o uart.o
 
 RECV_NAME	= recv

avr/recv.c

@@ -4,6 +4,7 @@
 
 #include <tabby/printer.h>
 #include <tabby/avr/uart.h>
+#include <tabby/avr.h>
 
 static volatile tabby_printer_packet header = {
     .type        = 0,
@@ -19,13 +20,13 @@ static volatile uint16_t i = 0,
  */
 ISR(SPI_STC_vect) {
     uint8_t in  = SPDR,
-            out = 0x00;
+            out = 0;
 
     uart_putchar(in, NULL);
 
     switch (i) {
         case 0: {
-            if (in == 0x88) {
+            if (in == TABBY_PRINTER_SYNC_1) {
                 header.data[0] = in;
                 i++;
 
@@ -36,7 +37,7 @@ ISR(SPI_STC_vect) {
         }
 
         case 1: {
-            if (in == 0x33) {
+            if (in == TABBY_PRINTER_SYNC_2) {
                 header.data[1] = in;
                 i++;
             } else {
@@ -80,7 +81,7 @@ ISR(SPI_STC_vect) {
             } else if (b == header.size + 1) {
                 b++;
 
-                out = 0x81;
+                out = TABBY_PRINTER_DEVICE_ID;
             } else if (b == header.size + 2) {
                 b++;
             } else if (b == header.size + 3) {
@@ -92,15 +93,11 @@ ISR(SPI_STC_vect) {
     SPDR = out;
 }
 
-static void setup_clock_external() {
+static void spi_init() {
-    /*
+    SS_INPUT();
-     * Configure MISO as output
+    SC_INPUT();
-     */
+    SI_INPUT();
-    DDRB |= (1 << DDB4);
+    SO_OUTPUT();
-    /*
-     * Configure SS as input
-     */
-    DDRB &= ~(1 << DDB2);
 
     /*
      * Set SPI slave mode, and shift in/out most significant bit first
@@ -108,7 +105,8 @@ static void setup_clock_external() {
     SPCR &= ~((1 << MSTR) | (1 << DORD));
 
     /*
-     * Enable SPI in Mode 3 with interrupts
+     * Enable SPI in Mode 3 with interrupts, clock nominally high, output on
+     * falling edge, input on rising edge
      */
     SPCR |= (1 << CPOL) | (1 << CPHA) | (1 << SPIE) | (1 << SPE);
 
@@ -119,21 +117,9 @@ static void setup_clock_external() {
 }
 
 int main() {
-    /*
-     * Best turn on the serial UART!
-     */
     uart_init();
+     spi_init();
 
-    /*
-     * By default, the Game Boy link port is configured to monitor for external
-     * clock pulses, so we'll go ahead and do that in this case.
-     */
-    setup_clock_external();
-
-    /*
-     * We do actually want to globally enable interrupts here, so here's that
-     * one assembly instruction to do so.
-     */
     sei();
 
     while (1) {

avr/send.c

@@ -5,6 +5,7 @@
 
 #include <tabby/printer.h>
 #include <tabby/avr/uart.h>
+#include <tabby/avr.h>
 
 static tabby_printer_packet header = {
     .type        = 0,
@@ -19,67 +20,52 @@ static uint8_t body[TABBY_PRINTER_MAX_PACKET_SIZE];
 static uint16_t sum = 0x0000;
 
 /*
- * So like, we're abusing pins here.  So badly, it's beyond.  So here's what's
+ * So like, we're abusing pins here.  So badly, it's beyond.  So here's
- * going down: We want to retain the same physical I/O connections between
+ * what's going down: We want to retain the same physical I/O connections
- * both the sending and receiving flavors of the whole Game Boy Printer
+ * between both the sending and receiving flavors of the whole Game Boy
- * endeavor, and that mean/s that we must absolutely bit bang MOSI and MISO
+ * Printer endeavor, and that mean/s that we must absolutely bit bang MOSI
- * opposite from their ordinary roles.  The reason being: There is no notion
+ * and MISO opposite from their ordinary roles.  The reason being: There is
- * of MISO or MOSI from the Game Boy's (nor its peripherals') point of view.
+ * no notion of MISO or MOSI from the Game Boy's (nor its peripherals')
- * The SI and SO pins over a link cable are crossed over, so SI on one end
+ * point of view. The SI and SO pins over a link cable are crossed over, so
- * always connects to SO on the other, and vice-versa.
+ * SI on one end always connects to SO on the other, and vice-versa.  The SI
+ * and SO pins do not switch roles based on role, unlike on AVR SPI
+ * implementations.
  */
 static void spi_init() {
-    /*
+    SC_OUTPUT();
-     * Set SC to output
+    SO_OUTPUT();
-     */
-    DDRB |= (1 << DDB5);
 
-    /*
+    SI_INPUT();
-     * Configure SO as output
+    SI_PULLUP();
-     */
-    DDRB |= (1 << DDB4);
 
-    /*
+    SC_HIGH();
-     * Configure SI as input with pullup
+    SO_LOW();
-     */
-    DDRB  &= ~(1 << DDB3);
-    PORTB |=  (1 << DDB3);
-
-    /*
-     * Set SO clear
-     */
-    PORTB &= ~(1 << PORTB4);
-
-    /*
-     * Set SC high
-     */
-    PORTB |= (1 << PORTB5);
 }
 
 static uint8_t spi_send_byte(uint8_t value) {
     uint8_t i, ret = 0;
 
     for (i=0; i<8; i++) {
-        PORTB &= ~(1 << PORTB5);
+        SC_LOW();
 
         if (value & 0x80) {
-            PORTB |=  (1 << PORTB4);
+            SO_HIGH();
         } else {
-            PORTB &= ~(1 << PORTB4);
+            SO_LOW();
         }
 
-        _delay_us(61);
+        _delay_us(SPI_PULSE_USEC);
 
         ret   <<= 1;
         value <<= 1;
 
-        if (PINB & (1 << PINB3)) {
+        if (SI_IS_HIGH()) {
             ret |= 1;
         }
 
-        PORTB |= (1 << PORTB5);
+        SC_HIGH();
 
-        _delay_us(61);
+        _delay_us(SPI_PULSE_USEC);
     }
 
     return ret;
@@ -105,8 +91,9 @@ static void spi_send_packet() {
 
 int main() {
     uint16_t i = 0,
-             b = 0,
+             b = 0;
-             c;
+
+    uint8_t c;
 
     uart_init();
      spi_init();
@@ -128,7 +115,7 @@ int main() {
 
         switch (i) {
             case 0: {
-                if (c == 0x88) {
+                if (c == TABBY_PRINTER_SYNC_1) {
                     header.data[0] = c;
                     i++;
 
@@ -139,7 +126,7 @@ int main() {
             }
 
             case 1: {
-                if (c == 0x33) {
+                if (c == TABBY_PRINTER_SYNC_2) {
                     header.data[1] = c;
                     i++;
                 } else {
@@ -151,11 +138,11 @@ int main() {
 
             case 2: {
                 switch (c) {
-                    case 0x01:
+                    case TABBY_PRINTER_PACKET_INIT:
-                    case 0x02:
+                    case TABBY_PRINTER_PACKET_JOB:
-                    case 0x04:
+                    case TABBY_PRINTER_PACKET_DATA:
-                    case 0x08:
+                    case TABBY_PRINTER_PACKET_CANCEL:
-                    case 0x0f: {
+                    case TABBY_PRINTER_PACKET_INQUIRY: {
                         header.type = c;
                         i++;
 

include/tabby/avr.h

@@ -0,0 +1,59 @@
+#ifndef _TABBY_AVR_H
+#define _TABBY_AVR_H
+
+#define SPI_PULSE_USEC 61
+
+#define SC_DDR      DDRB
+#define SC_DDB      DDB5
+#define SC_PORT     PORTB
+#define SC_PORT_PIN PORTB5
+
+#define SC_INPUT() \
+    (SC_DDR |= ~(1 << SC_DDB))
+
+#define SC_OUTPUT() \
+    (SC_DDR |= (1 << SC_DDB))
+
+#define SC_HIGH() \
+    (SC_PORT |= (1 << SC_PORT_PIN))
+
+#define SC_LOW() \
+    (SC_PORT &= ~(1 << SC_PORT_PIN))
+
+#define SI_DDR      DDRB
+#define SI_DDB      DDB3
+#define SI_PIN_REG  PINB
+#define SI_PIN      PINB3
+#define SI_PORT     PORTB
+#define SI_PORT_PIN PORTB3
+
+#define SI_INPUT() \
+    (SI_DDR &= ~(1 << SI_DDB))
+
+#define SI_PULLUP() \
+    (SI_PORT |= (1 << SI_PORT_PIN))
+
+#define SI_IS_HIGH() \
+    (SI_PIN_REG & (1 << SI_PIN))
+
+#define SO_DDR      DDRB
+#define SO_DDB      DDB4
+#define SO_PORT     PORTB
+#define SO_PORT_PIN PORTB4
+
+#define SO_OUTPUT() \
+    (SO_DDR |= (1 << SO_DDB))
+
+#define SO_HIGH() \
+    (SO_PORT |= (1 << SO_PORT_PIN))
+
+#define SO_LOW() \
+    (SO_PORT &= ~(1 << SO_PORT_PIN))
+
+#define SS_DDR DDRB
+#define SS_DDB DDB2
+
+#define SS_INPUT() \
+    (SS_DDR &= ~(1 << SS_DDB))
+
+#endif /* _TABBY_AVR_H */

include/tabby/avr/buffer.h

@@ -1,14 +0,0 @@
-#ifndef _TABBY_AVR_BUFFER_H
-#define _TABBY_AVR_BUFFER_H
-
-#define TABBY_AVR_BUFFER_SIZE 1024
-
-typedef struct _tabby_avr_buffer {
-    uint16_t len,
-             cur,
-             read;
-
-    uint8_t data[TABBY_AVR_BUFFER_SIZE];
-} tabby_avr_buffer;
-
-#endif /* _TABBY_AVR_BUFFER_H */

include/tabby/printer.h

@@ -1,6 +1,9 @@
 #ifndef _TABBY_PRINTER_H
 #define _TABBY_PRINTER_H
 
+#define TABBY_PRINTER_SYNC_1    0x88
+#define TABBY_PRINTER_SYNC_2    0x33
+
 #define TABBY_PRINTER_DEVICE_ID 0x81
 #define TABBY_PRINTER_RETRY_COUNT  5
 

src/printer.c

@@ -57,14 +57,14 @@ int tabby_printer_packet_recv(int fd,
 
     while ((len = read(fd, &value, 1)) >= 0) {
         if (i == 0) {
-            if (value == 0x88) {
+            if (value == TABBY_PRINTER_SYNC_1) {
                 header->preamble[0] = value;
                 i++;
             } else {
                 continue;
             }
         } else if (i == 1) {
-            if (value == 0x33) {
+            if (value == TABBY_PRINTER_SYNC_2) {
                 header->preamble[1] = value;
                 i++;
             } else {
@@ -160,8 +160,8 @@ error_io:
 
 static void init_header(tabby_printer_packet *header,
                         tabby_printer_packet_type type, size_t size) {
-    header->preamble[0] = 0x88;
+    header->preamble[0] = TABBY_PRINTER_SYNC_1;
-    header->preamble[1] = 0x33;
+    header->preamble[1] = TABBY_PRINTER_SYNC_2;
     header->type        = type;
     header->compression = TABBY_PRINTER_COMPRESSION_NONE;
     header->size        = (uint16_t)size;