Fix new packet-based comms protocol

7 files changed, 113 insertions, 110 deletions

diff --git a/firmware/Makefile b/firmware/Makefile
index 1e668e1..be1709b 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -10,9 +10,10 @@ OBJCOPY := arm-none-eabi-objcopy
 OBJDUMP := arm-none-eabi-objdump
 SIZE    := arm-none-eabi-size
 
-CFLAGS  = -Wall -Wpedantic -Wstrict-aliasing -g -std=gnu11 -Os  
+#CFLAGS  = -Wall -Wpedantic -Wstrict-aliasing -g -std=gnu11 -Os  
+CFLAGS  = -Wall -Wpedantic -Wstrict-aliasing -g -std=gnu11 -O1
 CFLAGS += -mlittle-endian -mcpu=cortex-m0 -march=armv6-m -mthumb
-#CFLAGS += -ffunction-sections -fdata-sections -Wl,--gc-sections 
+CFLAGS += -ffunction-sections -fdata-sections -Wl,--gc-sections 
 CFLAGS += -Wl,-Map=main.map
 
 # Technically we're using an STM32F030F4, but apart from the TSSOP20 package that one is largely identical to the
@@ -43,7 +44,7 @@ sources.tar.xz.zip: sources.tar.xz
 sources.c: sources.tar.xz.zip
 	xxd -i $< | head -n -1 | sed 's/=/__attribute__((section(".source_tarball"))) =/' > $@
 
-main.elf: main.c mac.c adc.c serial.c startup_stm32f030x6.s system_stm32f0xx.c $(HAL_PATH)/Src/stm32f0xx_ll_utils.c cmsis_exports.c
+main.elf: main.c adc.c serial.c startup_stm32f030x6.s system_stm32f0xx.c $(HAL_PATH)/Src/stm32f0xx_ll_utils.c cmsis_exports.c sources.c
 	$(CC) $(CFLAGS) -o $@ $^
 	$(OBJCOPY) -O ihex $@ $(@:.elf=.hex)
 	$(OBJCOPY) -O binary $@ $(@:.elf=.bin)
diff --git a/firmware/global.h b/firmware/global.h
index 6116adc..3e585f6 100644
--- a/firmware/global.h
+++ b/firmware/global.h
@@ -1,9 +1,31 @@
 #ifndef __GLOBAL_H__
 #define __GLOBAL_H__
 
+#define COLOR_SPEC_WHITE            0x00
+#define COLOR_SPEC_SINGLE_COLOR     0x01
+#define COLOR_SPEC_RGB              0x02
+#define COLOR_SPEC_RGBW             0x03
+#define COLOR_SPEC_COLD_WARM_WHITE  0x04
+#define COLOR_SPEC_WWA              0x05 /* cold white/warm white/amber */
+
+#define OLSNDOT_V1 0x01
+
 #define FIRMWARE_VERSION 2
 #define HARDWARE_VERSION 2
 
+/* Maximum bit count supported by serial command protocol. The brightness data is assumed to be of this bit width, but
+ * only the uppermost NBITS bits are used. */
+#define MAX_BITS 16
+
+/* Bit count of this device. Note that to change this you will also have to adapt the per-bit timer period lookup table
+ * in main.c.  */
+#define NBITS 14
+
+#define NCHANNELS 32
+#define CHANNEL_SPEC 'H'
+#define COLOR_SPEC COLOR_SPEC_RGBW
+#define DEVICE_TYPE OLSNDOT_V1
+
 #define TS_CAL1 (*(uint16_t *)0x1FFFF7B8)
 #define VREFINT_CAL (*(uint16_t *)0x1FFFF7BA)
 
diff --git a/firmware/mac.c b/firmware/mac.c
deleted file mode 100644
index b2fb48a..0000000
--- a/firmware/mac.c
+++ /dev/null
@@ -1,3 +0,0 @@
-#include "mac.h"
-
-uint32_t device_mac = MAC_ADDR;
diff --git a/firmware/mac.h b/firmware/mac.h
deleted file mode 100644
index 26aaff6..0000000
--- a/firmware/mac.h
+++ /dev/null
@@ -1,22 +0,0 @@
-#ifndef __MAC_H__
-#define __MAC_H__
-
-#include <unistd.h>
-
-/* Device MAC address.
- *
- * 32 bits might seem a little short for a device MAC, but at 20 bus nodes the probablility of a collision is about 1 in
- * 10 million. Check for yourself using the python code below.
- *
- *     #!/usr/bin/env python3
- *     from operator import mul
- *     from functools import reduce
- *     m = 32
- *     n = 20
- *     print(reduce(mul, [2**m-i for i in range(n)]) / ((2**m)**n))
- *     # -> 0.9999999557621786
- */
-
-extern uint32_t device_mac;
-
-#endif /* __MAC_H__ */
diff --git a/firmware/main.c b/firmware/main.c
index 7fc78c3..ec1e576 100644
--- a/firmware/main.c
+++ b/firmware/main.c
@@ -38,15 +38,6 @@
 #include "serial.h"
 #include "adc.h"
 
-/* Bit count of this device. Note that to change this you will also have to adapt the per-bit timer period lookup table
- * below.
- */
-#define NBITS 14
-
-/* Maximum bit count supported by serial command protocol. The brightness data is assumed to be of this bit width, but
- * only the uppermost NBITS bits are used. */
-#define MAX_BITS 16
-
 void do_transpose(void);
 
 /* Bit-golfed modulation data generated from the above values by the main loop, ready to be sent out to the shift
@@ -58,6 +49,56 @@ volatile uint32_t brightness_by_bit[NBITS] = { 0 };
 uint32_t sys_time = 0;
 uint32_t sys_time_seconds = 0;
 
+/* This value sets how long a batch of ADC conversions used for temperature measurement is started before the end of the
+ * longest cycle. Here too the above caveats apply.
+ *
+ * This value is in TIM1/TIM3 timer counts. */
+#define ADC_PRETRIGGER 150 /* trigger with about 12us margin to TIM1 CC IRQ */
+
+/* Bit timing base value. This is the lowes bit interval used */
+#define PERIOD_BASE 4
+
+/* This value is a constant offset added to every bit period to allow for the timer IRQ handler to execute. This is set
+ * empirically using a debugger and a logic analyzer. */
+#define TIMER_CYCLES_FOR_SPI_TRANSMISSIONS 120
+
+/* This is the same as above, but for the reset cycle of the bit period. */
+#define RESET_PERIOD_LENGTH 40
+
+/* Defines for brevity */
+#define A TIMER_CYCLES_FOR_SPI_TRANSMISSIONS
+#define B PERIOD_BASE
+
+/* This is a constant offset containing some empirically determined correction values */
+#define C (1 /* reset pulse comp */ - 3 /* analog snafu comp */)
+
+/* This lookup table maps bit positions to timer period values. This is a lookup table to allow for the compensation for
+ * non-linear effects of ringing at lower bit durations.
+ */
+static uint16_t timer_period_lookup[NBITS] = {
+    /* LSB here */
+    A - C + (B<< 0),
+    A - C + (B<< 1),
+    A - C + (B<< 2),
+    A - C + (B<< 3),
+    A - C + (B<< 4),
+    A - C + (B<< 5),
+    A - C + (B<< 6),
+    A - C + (B<< 7),
+    A - C + (B<< 8),
+    A - C + (B<< 9),
+    A - C + (B<<10),
+    A - C + (B<<11),
+    A - C + (B<<12),
+    A - C + (B<<13),
+    /* MSB here */
+};
+
+/* Don't pollute the global namespace */
+#undef A
+#undef B
+#undef C
+
 int main(void) {
     /* Get all the good clocks and PLLs on this thing up and running. We're running from an external 25MHz crystal,
      * which we're first dividing down by 5 to get 5 MHz, then PLL'ing up by 6 to get 30 MHz as our main system clock.
@@ -90,14 +131,14 @@ int main(void) {
 
 
     /* Enable all the periphery we need */
-    RCC->AHBENR  |= RCC_AHBENR_GPIOAEN | RCC_AHBENR_GPIOBEN;
+    RCC->AHBENR  |= RCC_AHBENR_GPIOAEN | RCC_AHBENR_GPIOBEN | RCC_AHBENR_DMAEN;
     RCC->APB2ENR |= RCC_APB2ENR_SPI1EN | RCC_APB2ENR_TIM1EN | RCC_APB2ENR_USART1EN | RCC_APB2ENR_ADCEN;
     RCC->APB1ENR |= RCC_APB1ENR_TIM3EN;
 
     /* Configure all the GPIOs */
     GPIOA->MODER |=
           (3<<GPIO_MODER_MODER0_Pos)  /* PA0  - Current measurement analog input */
-        | (1<<GPIO_MODER_MODER1_Pos)  /* PA1  - RS485 TX enable */
+        | (2<<GPIO_MODER_MODER1_Pos)  /* PA1  - RS485 TX enable */
         | (2<<GPIO_MODER_MODER2_Pos)  /* PA2  - RS485 TX */
         | (2<<GPIO_MODER_MODER3_Pos)  /* PA3  - RS485 RX */
         /* PA4 reserved because */
@@ -123,7 +164,8 @@ int main(void) {
 
     /* Alternate function settings */
     GPIOA->AFR[0] |=
-          (1<<GPIO_AFRL_AFRL2_Pos)   /* USART1_TX */
+          (1<<GPIO_AFRL_AFRL1_Pos)   /* USART1_RTS (RS485 DE) */
+        | (1<<GPIO_AFRL_AFRL2_Pos)   /* USART1_TX */
         | (1<<GPIO_AFRL_AFRL3_Pos)   /* USART1_RX */
         | (0<<GPIO_AFRL_AFRL5_Pos)   /* SPI1_SCK  */
         | (0<<GPIO_AFRL_AFRL7_Pos);  /* SPI1_MOSI */
@@ -142,12 +184,14 @@ int main(void) {
 
     TIM1->PSC = 1; /* Prescale by 2, resulting in a 16MHz timer frequency and 62.5ns timer step size. */
     /* CH2 - clear/!MR, CH3 - strobe/STCP */
-    TIM1->CCMR2 = (6<<TIM_CCMR2_OC3M_Pos) | TIM_CCMR2_OC3PE;
-    TIM1->CCER |= TIM_CCER_CC3E | TIM_CCER_CC3NE | TIM_CCER_CC3P | TIM_CCER_CC3NP;
+    TIM1->CCMR2 = (6<<TIM_CCMR2_OC3M_Pos) | TIM_CCMR2_OC3PE | (6<<TIM_CCMR2_OC4M_Pos);
+    TIM1->CCER |= TIM_CCER_CC3E | TIM_CCER_CC3NE | TIM_CCER_CC3P | TIM_CCER_CC3NP | TIM_CCER_CC4E;
     TIM1->BDTR = TIM_BDTR_MOE | (1<<TIM_BDTR_DTG_Pos); /* really short dead time */
     TIM1->DIER = TIM_DIER_UIE; /* Enable update (overrun) interrupt */
     TIM1->ARR = 1;
     TIM1->CR1 |= TIM_CR1_CEN;
+    /* Trigger at the end of the longest bit cycle. This means this does not trigger in shorter bit cycles. */
+    TIM1->CCR4  = timer_period_lookup[NBITS-1] - ADC_PRETRIGGER;
 
     /* Configure Timer 1 update (overrun) interrupt on NVIC.  Used only for update (overrun) for strobe timing. */
     NVIC_EnableIRQ(TIM1_BRK_UP_TRG_COM_IRQn);
@@ -184,7 +228,8 @@ int main(void) {
         | USART_CR1_RE;
     USART1->CR3 = USART_CR3_DEM; /* RS485 DE enable (output on RTS) */
     // USART1->BRR = 30;
-    USART1->BRR = 40; // 750000
+    //USART1->BRR = 40; // 750000
+    USART1->BRR = 60; // 500000
     USART1->CR1 |= USART_CR1_UE;
 
     /* Configure USART1 interrupt on NVIC. Used only for RX. */
@@ -214,58 +259,13 @@ void do_transpose(void) {
     for (uint32_t i=0; i<NBITS; i++) {
         uint32_t mask = 1<<i<<(MAX_BITS-NBITS); /* Bit mask for this bit value. */
         uint32_t bv = 0; /* accumulator thing */
-        for (uint32_t j=0; j<32; j++) {
+        for (uint32_t j=0; j<32; j++)
             if (rx_buf.set_fb_rq.framebuf[j] & mask)
-            bv |= 1<<j;
-        }
+                bv |= 1<<j;
         brightness_by_bit[i] = bv;
     }
 }
 
-/* Bit timing base value. This is the lowes bit interval used */
-#define PERIOD_BASE 4
-
-/* This value is a constant offset added to every bit period to allow for the timer IRQ handler to execute. This is set
- * empirically using a debugger and a logic analyzer. */
-#define TIMER_CYCLES_FOR_SPI_TRANSMISSIONS 120
-
-/* This is the same as above, but for the reset cycle of the bit period. */
-#define RESET_PERIOD_LENGTH 40
-
-/* Defines for brevity */
-#define A TIMER_CYCLES_FOR_SPI_TRANSMISSIONS
-#define B PERIOD_BASE
-
-/* This is a constant offset containing some empirically determined correction values */
-#define C (1 /* reset pulse comp */ - 3 /* analog snafu comp */)
-
-/* This lookup table maps bit positions to timer period values. This is a lookup table to allow for the compensation for
- * non-linear effects of ringing at lower bit durations.
- */
-static uint16_t timer_period_lookup[NBITS] = {
-    /* LSB here */
-    A - C + (B<< 0),
-    A - C + (B<< 1),
-    A - C + (B<< 2),
-    A - C + (B<< 3),
-    A - C + (B<< 4),
-    A - C + (B<< 5),
-    A - C + (B<< 6),
-    A - C + (B<< 7),
-    A - C + (B<< 8),
-    A - C + (B<< 9),
-    A - C + (B<<10),
-    A - C + (B<<11),
-    A - C + (B<<12),
-    A - C + (B<<13),
-    /* MSB here */
-};
-
-/* Don't pollute the global namespace */
-#undef A
-#undef B
-#undef C
-
 /* Timer 1 main IRQ handler. This is used only for overflow ("update" or UP event in ST's terminology). */
 void TIM1_BRK_UP_TRG_COM_IRQHandler(void) {
     /* The index of the currently active bit. On entry of this function, this is the bit index of the upcoming period.
diff --git a/firmware/serial.c b/firmware/serial.c
index fcdaed7..aaf7ca3 100644
--- a/firmware/serial.c
+++ b/firmware/serial.c
@@ -23,7 +23,6 @@
 
 #include "serial.h"
 #include "adc.h"
-#include "mac.h"
 
 unsigned int uart_overruns = 0;
 unsigned int frame_overruns = 0;
@@ -47,13 +46,17 @@ void send_frame_formatted(uint8_t *buf, int len) {
         while (*p && p!=end)
             tx_char(*p++);
         p++, q++;
-    } while (p < end);
+    } while (p <= end);
     tx_char('\0');
 }
 
 void send_status_reply(void) {
     tx_buf.desc_reply.firmware_version = FIRMWARE_VERSION;
     tx_buf.desc_reply.hardware_version = HARDWARE_VERSION;
+    tx_buf.desc_reply.nbits = NBITS;
+    tx_buf.desc_reply.channel_spec = CHANNEL_SPEC;
+    tx_buf.desc_reply.nchannels = NCHANNELS;
+    tx_buf.desc_reply.color_spec = COLOR_SPEC;
     tx_buf.desc_reply.uptime_s = sys_time_seconds;
     tx_buf.desc_reply.vcc_mv = adc_vcc_mv;
     tx_buf.desc_reply.temp_celsius = adc_temp_celsius;
@@ -66,16 +69,18 @@ void send_status_reply(void) {
 /* This is the higher-level protocol handler for the serial protocol. It gets passed the number of data bytes in this
  * frame (which may be zero) and returns a pointer to the buffer where the next frame should be stored.
  */
-volatile uint8_t *packet_received(int len) {
+static volatile inline void packet_received(int len) {
     static enum {
         PROT_ADDRESSED = 0,
         PROT_EXPECT_FRAME_SECOND_HALF = 1,
         PROT_IGNORE = 2,
     } protocol_state = PROT_IGNORE; 
     /* Use mac frames as delimiters to synchronize this protocol layer */
-    if (len == 0) { /* Discovery packet */
+    if (len == 1 && rx_buf.byte_data[0] == 0x00) { /* Discovery packet */
         if (sys_time < 100 && sys_time_seconds == 0) { /* Only respond during the first 100ms after boot */
-            send_frame_formatted((uint8_t*)&device_mac, sizeof(device_mac));
+            tx_buf.device_type_reply.mac = MAC_ADDR;
+            tx_buf.device_type_reply.device_type = DEVICE_TYPE;
+            send_frame_formatted(tx_buf.byte_data, sizeof(tx_buf.device_type_reply));
         }
 
     } else if (len == 1) { /* Command packet */
@@ -91,13 +96,13 @@ volatile uint8_t *packet_received(int len) {
         protocol_state = PROT_IGNORE;
 
     } else if (len == 4) { /* Address packet */
-        if (rx_buf.mac_data == device_mac) { /* we are addressed */
+        if (rx_buf.mac_data == MAC_ADDR) { /* we are addressed */
             protocol_state = PROT_ADDRESSED; /* start listening for frame buffer data */
         } else { /* we are not addressed */
             protocol_state = PROT_IGNORE; /* ignore packet */
         }
 
-    } else if (len == sizeof(rx_buf.set_fb_rq.framebuf)) {
+    } else if (len == sizeof(rx_buf.set_fb_rq)) {
         if (protocol_state == PROT_ADDRESSED) { /* First of two half-framebuffer data frames */
             /* Kick off buffer transfer. This triggers the main loop to copy data out of the receive buffer and paste it
              * properly formatted into the frame buffer. */
@@ -117,11 +122,6 @@ volatile uint8_t *packet_received(int len) {
         invalid_frames++;
         protocol_state = PROT_IGNORE; /* go into "hang mode" until next zero-length packet */
     }
-
-    /* By default, return rx_buf.byte_data . This means if an invalid protocol state is reached ("hang mode"), the next
-     * frame is still written to rx_buf. This is not a problem since whatever garbage is written at that point will be
-     * overwritten before the next buffer transfer. */
-    return rx_buf.byte_data;
 }
 
 void USART1_IRQHandler(void) {
@@ -157,9 +157,6 @@ void USART1_IRQHandler(void) {
      *     http://www.stuartcheshire.org/papers/COBSforToN.pdf
      */
 
-    /* This pointer stores where we write data. The higher-level protocol logic decides on a frame-by-frame-basis where
-     * the next frame's data will be stored. */
-    static volatile uint8_t *writep = rx_buf.byte_data;
     /* Index inside the current frame payload */
     static int rxpos = 0;
     /* COBS state machine. This implementation might be a little too complicated, but it works well enough and I find it
@@ -184,8 +181,9 @@ void USART1_IRQHandler(void) {
         uint8_t data = USART1->RDR; /* This automatically acknowledges the IRQ */
 
         if (data == 0x00) { /* End-of-packet */
-            /* Process higher protocol layers on this packet. */
-            writep = packet_received(rxpos);
+            if (cobs_state != COBS_WAIT_SYNC) /* Has a packet been received? */
+                /* Process higher protocol layers on this packet. */
+                packet_received(rxpos);
 
             /* Reset for next packet. */
             cobs_state = COBS_WAIT_START;
@@ -206,7 +204,7 @@ void USART1_IRQHandler(void) {
                 }
 
                 /* Write processed payload byte to current receive buffer */
-                writep[rxpos++] = data;
+                rx_buf.byte_data[rxpos++] = data;
             }
         }
     }
diff --git a/firmware/serial.h b/firmware/serial.h
index 5066c43..94cfd57 100644
--- a/firmware/serial.h
+++ b/firmware/serial.h
@@ -27,7 +27,6 @@ void send_status_reply(void);
 /* Internal low-level stuff */
 void tx_char(uint8_t c);
 void send_frame_formatted(uint8_t *buf, int len);
-volatile uint8_t *packet_received(int len);
 
 /* Error counters for debugging */
 extern unsigned int uart_overruns;
@@ -38,6 +37,10 @@ union tx_buf_union {
     struct __attribute__((packed)) {
         uint8_t  firmware_version,
                  hardware_version;
+        uint8_t  nbits;
+        uint8_t  channel_spec;
+        uint8_t color_spec;
+        uint16_t nchannels;
         uint32_t uptime_s,
                  uart_overruns,
                  frame_overruns,
@@ -45,6 +48,10 @@ union tx_buf_union {
          int16_t vcc_mv,
                  temp_celsius;
     } desc_reply;
+    struct __attribute__((packed)) {
+        uint32_t mac;
+        uint16_t  device_type;
+    } device_type_reply;
     uint8_t byte_data[0];
 };
 
@@ -57,7 +64,7 @@ union rx_buf_union {
      *                                 |<----------------NBITS---------------->|  |<>|--ignored
      *                                 | (MSB)      brightness data      (LSB) |  |<>|--ignored
      */
-    struct __attribute__((packed)) { uint32_t framebuf[32]; uint8_t end[0]; } set_fb_rq;
+    struct __attribute__((packed)) { uint16_t framebuf[32]; uint8_t end[0]; } set_fb_rq;
     uint8_t byte_data[0];
     uint32_t mac_data;
 };