Add twinkle and a few more params and make the display less janky and

more robust.

11 files changed, 321 insertions, 76 deletions

diff --git a/include/pattern/twinkle.h b/include/pattern/twinkle.h
new file mode 100644
index 0000000..3dfe85f
--- /dev/null
+++ b/include/pattern/twinkle.h
@@ -0,0 +1,6 @@
+#pragma once
+
+#include <stdint.h>
+#include <stdlib.h>
+
+uint8_t twinkle(uint32_t time, size_t x, uint8_t);
diff --git a/include/state_params.i b/include/state_params.i
index 15a5b43..875328d 100644
--- a/include/state_params.i
+++ b/include/state_params.i
@@ -10,3 +10,7 @@ STATE_PARAM(int,       x_scale,     "X Scale",       255)
 STATE_PARAM(bool,      power,       "Power",         true)     
 STATE_PARAM(bool,      cool,        "Cool",          false)    
 STATE_PARAM(bool,      invert,      "Invert",        false)    
+STATE_PARAM(bool,   use_static_color,   "Use Static Color",  false)
+STATE_PARAM(color_int_t,   static_color,   "Static Color",  0xffddbb)
+STATE_PARAM(bool,      twinkle,     "Twinkle",       true)    
+STATE_PARAM(uint8_t,   twink_amt,   "Twinkle Amount",  0)       
diff --git a/include/ws2812b_writer.h b/include/ws2812b_writer.h
index e8fa84f..ac4eb05 100644
--- a/include/ws2812b_writer.h
+++ b/include/ws2812b_writer.h
@@ -4,18 +4,23 @@
 
 #include "drv/ws2812b.h"
 #include "freertos/FreeRTOS.h"
+#include "freertos/semphr.h"
 #include "freertos/task.h"
 
 extern const int NUMBER_PARAMS;
 
-typedef struct {
-  ws2812b_t* drv;
+typedef uint32_t color_int_t;
 
-  struct {
+typedef struct {
 #define STATE_PARAM(t, n, ...) t n;
 #include "state_params.i"
 #undef STATE_PARAM
-  } state;
+} ws_state_t;
+
+typedef struct {
+  ws2812b_t* drv;
+  ws_state_t state;
+  SemaphoreHandle_t state_lock;
 } ws_params_t;
 
 void reset_parameters(ws_params_t* params);
diff --git a/main/CMakeLists.txt b/main/CMakeLists.txt
index 8445ad7..c595f30 100644
--- a/main/CMakeLists.txt
+++ b/main/CMakeLists.txt
@@ -1,9 +1,9 @@
-idf_component_register(SRCS
-    "main.c"
-    "station.c"
-    "ws2812b_writer.c"
-    "tcp_server.c"
-    "sockbuf.c"
-    "http_server.c"
+file(GLOB_RECURSE MAIN_SRCS "${CMAKE_CURRENT_LIST_DIR}/*.c")
+# Avoid pulling in sources from the nested driver component, which has its own
+# CMakeLists and registration.
+list(FILTER MAIN_SRCS EXCLUDE REGEX ".*/drv/.*")
+
+idf_component_register(
+  SRCS ${MAIN_SRCS}
   INCLUDE_DIRS "../include"
   REQUIRES esp_driver_spi spi_flash esp_http_server esp_wifi nvs_flash)
diff --git a/main/drv/CMakeLists.txt b/main/drv/CMakeLists.txt
index 79db121..6a897d0 100644
--- a/main/drv/CMakeLists.txt
+++ b/main/drv/CMakeLists.txt
@@ -1,3 +1,7 @@
-idf_component_register(SRCS "ws2812b.c"
+file(GLOB_RECURSE WS2812B_SRCS
+  "${CMAKE_CURRENT_LIST_DIR}/*.c")
+
+idf_component_register(
+  SRCS ${WS2812B_SRCS}
   REQUIRES driver spi_flash esp_driver_spi
   INCLUDE_DIRS "../../include")
diff --git a/main/drv/ws2812b.c b/main/drv/ws2812b.c
index d0699ee..75c9873 100644
--- a/main/drv/ws2812b.c
+++ b/main/drv/ws2812b.c
@@ -60,17 +60,16 @@ ws2812b_buffer_t* ws2812b_new_buffer(uint32_t n)
 
 esp_err_t ws2812b_wait(ws2812b_t* drv)
 {
-  esp_err_t err;
-
   spi_transaction_t* rt;
-  err = spi_device_get_trans_result(
-      drv->spi_handle, &rt, 10 / portTICK_PERIOD_MS);
-  drv->flags &= ~WS2812B_FLAG_DIRTY;
-  if (err != ESP_OK) {
-    return err;
+  /* Block until the previous transfer is fully complete so we never reuse
+   * a buffer while the SPI peripheral is still sending it. Occasional
+   * timeouts here were causing visible flicker. */
+  esp_err_t err =
+      spi_device_get_trans_result(drv->spi_handle, &rt, portMAX_DELAY);
+  if (err == ESP_OK) {
+    drv->flags &= ~WS2812B_FLAG_DIRTY;
   }
-
-  return ESP_OK;
+  return err;
 }
 
 esp_err_t ws2812b_write(ws2812b_t* drv, ws2812b_buffer_t* buffer)
@@ -78,7 +77,10 @@ esp_err_t ws2812b_write(ws2812b_t* drv, ws2812b_buffer_t* buffer)
   compile(buffer);
 
   if (drv->flags & WS2812B_FLAG_DIRTY) {
-    ws2812b_wait(drv);
+    esp_err_t wait_err = ws2812b_wait(drv);
+    if (wait_err != ESP_OK) {
+      return wait_err;
+    }
   }
 
   esp_err_t err;
diff --git a/main/http_server.c b/main/http_server.c
index 0345324..aabd9fb 100644
--- a/main/http_server.c
+++ b/main/http_server.c
@@ -1,4 +1,8 @@
 #include "http_server.h"
+#include "freertos/semphr.h"
+#include <stdlib.h>
+#include <string.h>
+#include <strings.h>
 
 #define TAG "httpd"
 
@@ -92,13 +96,56 @@ static void handle_set_uint32_t(httpd_req_t* req, uint32_t* val)
   *val = tmp;
 }
 
+static void handle_set_color_int_t(httpd_req_t* req, color_int_t* val)
+{
+  char buf[128];
+  int buf_len = httpd_req_get_url_query_len(req) + 1;
+
+  if (buf_len > 1) {
+    if (buf_len > sizeof(buf)) {
+      ESP_LOGI(TAG, "Invalid request. Query string too long.");
+      httpd_resp_set_status(req, HTTPD_400);
+      return;
+    }
+
+    if (httpd_req_get_url_query_str(req, buf, buf_len) == ESP_OK) {
+      char param[32];
+      if (httpd_query_key_value(buf, "set", param, sizeof(param)) == ESP_OK) {
+        char* p = param;
+        if (*p == '#') {
+          p++;
+        } else if (!strncasecmp(p, "0x", 2)) {
+          p += 2;
+        }
+        char* end = NULL;
+        unsigned long parsed = strtoul(p, &end, 16);
+        if (end == p || *end != '\0' || parsed > 0xFFFFFFUL) {
+          ESP_LOGI(TAG, "Invalid request. Invalid color value.");
+          httpd_resp_set_status(req, HTTPD_400);
+          return;
+        }
+        *val = (color_int_t)parsed;
+      }
+    }
+  }
+
+  httpd_resp_set_status(req, HTTPD_204);
+}
+
 #define STATE_PARAM(typ, attr, ...)                              \
   static esp_err_t handle_set_##attr(httpd_req_t* req)           \
   {                                                              \
     ESP_LOGI(TAG, "Handling [%s] (%s)", #attr, req->uri);        \
     ws_params_t* params = (ws_params_t*)req->user_ctx;           \
     httpd_resp_set_hdr(req, "Access-Control-Allow-Origin", "*"); \
+    if (xSemaphoreTake(params->state_lock, portMAX_DELAY) !=     \
+        pdTRUE) {                                                \
+      httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR,  \
+                          "lock failed");                        \
+      return ESP_FAIL;                                           \
+    }                                                            \
     handle_set_##typ(req, &params->state.attr);                  \
+    xSemaphoreGive(params->state_lock);                          \
     httpd_resp_send(req, NULL, 0);                               \
     return ESP_OK;                                               \
   }                                                              \
@@ -166,6 +213,20 @@ static size_t write_bool_value(
   return len - newlen;
 }
 
+static size_t write_color_int_t_value(
+    char** out, size_t len, const char* attr, const char* dn, color_int_t value)
+{
+  size_t newlen = snprintf(
+      *out,
+      len,
+      "\"%s\":{\"type\":\"color\",\"value\":\"%06lx\",\"disp\":\"%s\"}",
+      attr,
+      (unsigned long)(value & 0xFFFFFF),
+      dn);
+  *out += newlen;
+  return len - newlen;
+}
+
 static esp_err_t handle_get_params(httpd_req_t* req)
 {
   httpd_resp_set_hdr(req, "Access-Control-Allow-Origin", "*");
@@ -176,6 +237,13 @@ static esp_err_t handle_get_params(httpd_req_t* req)
   char* out_ptr = out_buffer;
 
   ws_params_t* ws_params = (ws_params_t*)req->user_ctx;
+  ws_state_t snapshot;
+  if (xSemaphoreTake(ws_params->state_lock, portMAX_DELAY) != pdTRUE) {
+    httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "lock failed");
+    return ESP_FAIL;
+  }
+  snapshot = ws_params->state;
+  xSemaphoreGive(ws_params->state_lock);
   bool write_comma = false;
   httpd_resp_set_type(req, "application/json");
   if (len > 1) {
@@ -189,7 +257,7 @@ static esp_err_t handle_get_params(httpd_req_t* req)
     len--;                                                   \
   }                                                          \
   len = write_##typ##_value(                                 \
-      &out_ptr, len, #attr, display, ws_params->state.attr); \
+      &out_ptr, len, #attr, display, snapshot.attr);         \
   write_comma = true;
 #include "state_params.i"
 #undef STATE_PARAM
diff --git a/main/main.c b/main/main.c
index da6d2bf..88f6994 100644
--- a/main/main.c
+++ b/main/main.c
@@ -3,6 +3,7 @@
 #include "esp_flash_spi_init.h"
 #include "esp_system.h"
 #include "freertos/FreeRTOS.h"
+#include "freertos/semphr.h"
 #include "freertos/task.h"
 #include "http_server.h"
 #include "sdkconfig.h"
@@ -62,7 +63,10 @@ portTASK_FUNCTION(time_keeper, params_)
   for (;;) {
     // Wait for the next cycle.
     vTaskDelayUntil(&last_wake, freq);
-    params->state.time += params->state.timetick;
+    if (xSemaphoreTake(params->state_lock, portMAX_DELAY) == pdTRUE) {
+      params->state.time += params->state.timetick;
+      xSemaphoreGive(params->state_lock);
+    }
   }
 }
 
@@ -102,6 +106,8 @@ void app_main(void)
   ESP_ERROR_CHECK(error);
 
   ws_params.drv = ws2812b_init(spi);
+  ws_params.state_lock = xSemaphoreCreateMutex();
+  configASSERT(ws_params.state_lock != NULL);
 
   printf("Configuration complete!!\n");
 
@@ -109,9 +115,29 @@ void app_main(void)
   wifi_init_station("Wort", "JoshIsBau5");
   ESP_LOGI("main", "Complete!");
 
-  xTaskCreate(time_keeper, "time_keeper", 1024, &ws_params, 1, NULL);
-  xTaskCreate(ws2812b_write_task, "ws2812b_writer", 4096, &ws_params, 1, NULL);
-  xTaskCreate(tcp_server, "tcp_server", 4096, &ws_params, 2, NULL);
+  xTaskCreate(
+      time_keeper,
+      "time_keeper",
+      /* stack_depth = */ 1024,
+      &ws_params,
+      /* priority = */ 1,
+      NULL);
+  /* Pin the LED writer to core 1 to avoid preemption by Wi-Fi/IDF tasks. */
+  xTaskCreatePinnedToCore(
+      ws2812b_write_task,
+      "ws2812b_writer",
+      /* stack_depth = */ 4096,
+      &ws_params,
+      /* priority = */ 1,
+      NULL,
+      /* core_id = */ 1);
+  xTaskCreate(
+      tcp_server,
+      "tcp_server",
+      /* stack_depth = */ 4096,
+      &ws_params,
+      /* priority = */ 2,
+      NULL);
 
   start_webserver(&ws_params);
 }
diff --git a/main/pattern/twinkle.c b/main/pattern/twinkle.c
new file mode 100644
index 0000000..d23b111
--- /dev/null
+++ b/main/pattern/twinkle.c
@@ -0,0 +1,53 @@
+#include <stdint.h>
+#include <stdlib.h>
+
+#include "pattern/twinkle.h"
+
+// The shape of a "spike"
+uint8_t w_arr[91] = {1,   2,   4,   7,   11,  17,  24,  33,  44,  56,  70,  85,
+                     101, 117, 134, 151, 167, 183, 197, 210, 222, 232, 240, 247,
+                     251, 254, 255, 255, 253, 250, 246, 240, 234, 227, 219, 211,
+                     202, 194, 185, 176, 167, 158, 149, 140, 132, 123, 116, 108,
+                     101, 94,  87,  81,  75,  69,  64,  59,  54,  50,  46,  42,
+                     39,  35,  32,  30,  27,  25,  22,  20,  18,  17,  15,  14,
+                     12,  11,  10,  9,   8,   7,   6,   6,   5,   4,   4,   3,
+                     3,   3,   2,   2,   2,   1,   1};
+
+static inline uint8_t byte_scale(uint8_t n, uint8_t sc)
+{
+  return n * sc / 255;
+}
+
+static uint8_t calc_w(uint8_t n, uint8_t shift)
+{
+  uint8_t s = byte_scale(shift, 164);
+  uint8_t n_p = byte_scale(n, 255 - s) + s;
+  if (n_p < 164) {
+    return 0;
+  }
+
+  return w_arr[n_p - 164];
+}
+
+/* Produces a "pseudo-random" number given seed 'x' */
+static uint8_t pseudo_random(uint8_t x)
+{
+  return (1103515245 * x + 12345) & 0xFF;  // & 0xFF ensures the result is 0-255
+}
+
+/* Produces a randomize "twinkle" effect. */
+uint8_t twinkle(uint32_t time, size_t x, uint8_t amt)
+{
+  uint8_t time_offset = pseudo_random(x);
+
+  uint32_t adj_time = time - time_offset;
+  uint32_t time8 = adj_time & 0xff;
+  uint32_t speed = pseudo_random((adj_time >> 8) + x) & 0x3;
+
+  time8 *= speed;
+  if (time8 <= 255) {
+    return calc_w(time8, amt);
+  } else {
+    return 0;
+  }
+}
diff --git a/main/tcp_server.c b/main/tcp_server.c
index 8e6b7de..237ed97 100644
--- a/main/tcp_server.c
+++ b/main/tcp_server.c
@@ -1,7 +1,9 @@
 #include "tcp_server.h"
 
 #include <ctype.h>
+#include <stdlib.h>
 
+#include "freertos/semphr.h"
 #include "lwip/sockets.h"
 #include "sockbuf.h"
 #include "station.h"
@@ -91,6 +93,29 @@ static void handle_uint32_t_option(
   *ptr = tmp;
 }
 
+static void handle_color_int_t_option(
+    const char* attr, sockbuf_t* sockbuf, color_int_t* ptr)
+{
+  char buf_[32];
+  char* value = read_token(sockbuf, buf_, sizeof(buf_));
+
+  if (!value) {
+    sockbuf_write(sockbuf, "Missing color value\n");
+    return;
+  }
+
+  char* end = NULL;
+  unsigned long parsed = strtoul(value, &end, 16);
+  if (end == value || *end != '\0' || parsed > 0xFFFFFFUL) {
+    sockbuf_write(sockbuf, "Invalid color; expected hex like RRGGBB\n");
+    return;
+  }
+
+  *ptr = (color_int_t)parsed;
+  snprintf(buf_, sizeof(buf_), "%s = 0x%06lx\n", attr, parsed);
+  sockbuf_write(sockbuf, buf_);
+}
+
 static void handle_bool_option(const char* attr, sockbuf_t* sockbuf, bool* ptr)
 {
   char buf_[32];
@@ -118,21 +143,30 @@ static void handle_bool_option(const char* attr, sockbuf_t* sockbuf, bool* ptr)
 static void handle_set_cmd(
     sockbuf_t* sockbuf, ws_params_t* ws_params, const char* key)
 {
+  if (xSemaphoreTake(ws_params->state_lock, portMAX_DELAY) != pdTRUE) {
+    sockbuf_write(sockbuf, "Failed to lock state\n");
+    return;
+  }
+
+  bool handled = false;
   if (false) {
   }
 #define STATE_PARAM(ty, attr, ...)                                \
   else if (!strcmp_(key, #attr))                                  \
   {                                                               \
     handle_##ty##_option(#attr, sockbuf, &ws_params->state.attr); \
+    handled = true;                                               \
   }
 #include "state_params.i"
 #undef STATE_PARAM
 
-  else {
+  if (!handled) {
     sockbuf_write(sockbuf, "Unknown attribute: '");
     sockbuf_write(sockbuf, key);
     sockbuf_write(sockbuf, "'\n");
   }
+
+  xSemaphoreGive(ws_params->state_lock);
 }
 
 static void print_uint32_t(sockbuf_t* sockbuf, const char* attr, uint32_t val)
@@ -163,10 +197,25 @@ static void print_int(sockbuf_t* sockbuf, const char* attr, int val)
   sockbuf_write(sockbuf, buf);
 }
 
+static void print_color_int_t(
+    sockbuf_t* sockbuf, const char* attr, color_int_t val)
+{
+  char buf[128];
+  snprintf(buf, sizeof(buf), "%s: 0x%06lx :: color\n", attr, (unsigned long)val);
+  sockbuf_write(sockbuf, buf);
+}
+
 static void handle_print_cmd(sockbuf_t* sockbuf, ws_params_t* ws_params)
 {
-#define STATE_PARAM(ty, attr, ...) \
-  print_##ty(sockbuf, #attr, ws_params->state.attr);
+  ws_state_t snapshot;
+  if (xSemaphoreTake(ws_params->state_lock, portMAX_DELAY) != pdTRUE) {
+    sockbuf_write(sockbuf, "Failed to lock state\n");
+    return;
+  }
+  snapshot = ws_params->state;
+  xSemaphoreGive(ws_params->state_lock);
+
+#define STATE_PARAM(ty, attr, ...) print_##ty(sockbuf, #attr, snapshot.attr);
 #include "state_params.i"
 #undef STATE_PARAM
 }
diff --git a/main/ws2812b_writer.c b/main/ws2812b_writer.c
index 3b90cd3..8bf0d91 100644
--- a/main/ws2812b_writer.c
+++ b/main/ws2812b_writer.c
@@ -3,6 +3,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "pattern/twinkle.h"
+
 const int NUMBER_PARAMS =
 #define STATE_PARAM(...) +1
 #include "state_params.i"
@@ -27,75 +29,91 @@ uint8_t amp(uint8_t in, uint8_t n);
 
 void reset_parameters(ws_params_t* params)
 {
+  if (params->state_lock) {
+    xSemaphoreTake(params->state_lock, portMAX_DELAY);
+  }
   memset(&params->state, 0, sizeof(params->state));
 
 #define STATE_PARAM(type, attr, displ, def, ...) params->state.attr = def;
 #include "state_params.i"
 #undef STATE_PARAM
+  if (params->state_lock) {
+    xSemaphoreGive(params->state_lock);
+  }
 }
 
-void calculate_colors(ws_params_t* ws_params, ws2812b_buffer_t* buffer)
+void calculate_colors(const ws_state_t* state, ws2812b_buffer_t* buffer)
 {
   uint32_t red = 0, green = 0, blue = 0;
   int time;
 
   for (int i = 0; i < SIZE; ++i) {
-    int x = i * 255 / ws_params->state.x_scale;
-    time = ws_params->state.time;
+    int x = i * 255 / state->x_scale;
+    time = state->time;
 
-    if (!ws_params->state.power) {
+    if (!state->power) {
       ws2812b_buffer_set_rgb(buffer, i, 0, 0, 0);
       continue;
     }
 
-    red = byte_scale(
-              byte_sin(time / 1000 + x * 4), 255 - ws_params->state.n_red) +
-          ws_params->state.n_red;
-    green = 255 - red;
+    uint8_t brightness = state->brightness;
+    if (!state->use_static_color) {
+      red = byte_scale(byte_sin(time / 1000 + x * 4), 255 - state->n_red) +
+            state->n_red;
+      green = 255 - red;
 
-    if (ws_params->state.cool) {
-      uint32_t tmp = green;
-      green = blue;
-      blue = tmp;
-    }
+      if (state->cool) {
+        uint32_t tmp = green;
+        green = blue;
+        blue = tmp;
+      }
 
-    /* Add a little white flair that comes around every once in a while. */
-
-    uint32_t whitesum = 0;
-    if (ws_params->state.n_snow) {
-      uint32_t white[] = {
-          /* Parallax "snow" */
-          time / 179 + x * 8,
-          time / 193 + x * 12,
-          time / 211 + x * 16,
-          time / 233 + x * 8,
-          // (ws_params->state.time + 128) / 233 + x * 8,
-      };
-
-      for (int i = 0; i < sizeof(white) / sizeof(uint32_t); ++i) {
-        if ((white[i] / 256) % ws_params->state.n_snow == 0) {
-          white[i] = amp(byte_sin(white[i]), 20);
-        } else {
-          white[i] = 0;
+      /* Add a little white flair that comes around every once in a while. */
+
+      uint32_t whitesum = 0;
+      if (state->n_snow) {
+        uint32_t white[] = {
+            /* Parallax "snow" */
+            time / 179 + x * 8,
+            time / 193 + x * 12,
+            time / 211 + x * 16,
+            time / 233 + x * 8,
+            // (state->time + 128) / 233 + x * 8,
+        };
+
+        for (int i = 0; i < sizeof(white) / sizeof(uint32_t); ++i) {
+          if ((white[i] / 256) % state->n_snow == 0) {
+            white[i] = amp(byte_sin(white[i]), 20);
+          } else {
+            white[i] = 0;
+          }
+          whitesum += white[i];
         }
-        whitesum += white[i];
       }
-    }
 
-    uint8_t brightness = ws_params->state.brightness;
-    red = min(red + whitesum, 255);
-    green = min(green + whitesum, 255);
-    blue = min(blue + whitesum, 255);
+      red = min(red + whitesum, 255);
+      green = min(green + whitesum, 255);
+      blue = min(blue + whitesum, 255);
+    } else {
+      red = (state->static_color >> 16) & 0xff;
+      green = (state->static_color >> 8) & 0xff;
+      blue = state->static_color & 0xff;
+    }
 
-    if (ws_params->state.invert) {
+    if (state->invert) {
       red = 255 - red;
       green = 255 - green;
       blue = 255 - blue;
     }
 
-    red = byte_scale(red, brightness);
-    green = byte_scale(green, brightness);
-    blue = byte_scale(blue, brightness);
+    uint8_t twink = 0xff;
+    if (state->twinkle) {
+      twink = twinkle(time / 300, x, state->twink_amt);
+    }
+
+    red = byte_scale(red, byte_scale(brightness, twink));
+    green = byte_scale(green, byte_scale(brightness, twink));
+    blue = byte_scale(blue, byte_scale(brightness, twink));
 
     ws2812b_buffer_set_rgb(buffer, i, red, green, blue);
   }
@@ -110,11 +128,21 @@ portTASK_FUNCTION(ws2812b_write_task, params)
 
   for (;;) {
     // Copy the parameters to avoid a change occuring during the calculation.
-    ws_params_t copy_params;
-    memcpy(&copy_params, ws_params, sizeof(ws_params_t));
-
-    calculate_colors(&copy_params, buffer);
-    ws2812b_write(copy_params.drv, buffer);
+    ws_state_t state_snapshot;
+    if (xSemaphoreTake(ws_params->state_lock, portMAX_DELAY) != pdTRUE) {
+      vTaskDelay(1);
+      continue;
+    }
+    state_snapshot = ws_params->state;
+    xSemaphoreGive(ws_params->state_lock);
+
+    calculate_colors(&state_snapshot, buffer);
+    if (ws2812b_write(ws_params->drv, buffer) != ESP_OK) {
+      /* If we fail to write, back off briefly instead of hammering the SPI
+       * bus, which can otherwise cause visible flicker. */
+      vTaskDelay(1);
+      continue;
+    }
     vTaskDelay(1);
   }
 }