Port i2s, spi_master, and ledc to helper functions for option tables (#2577)

* opt_checkint_range: extend range check to default value

* opt_checklstring added

* i2s: rework luaM_ and option table handling

* spi_master: rework option tables handling

* ledc: rework option tables handling

components/modules/common.c

@@ -49,17 +49,17 @@ int opt_checkint(lua_State *L, const char *name, int default_val)
 
 int opt_checkint_range(lua_State *L, const char *name, int default_val, int min_val, int max_val)
 {
+  int result = default_val;
+
   if (opt_get(L, name, LUA_TNUMBER)) {
-    int result = lua_tointeger(L, -1);
+    result = lua_tointeger(L, -1);
     lua_pop(L, 1);
+  }
   if (!(result >= min_val && result <= max_val)) {
     const char* msg = lua_pushfstring(L, "must be in range %d-%d", min_val, max_val);
     opt_error(L, name, msg);
   }
   return result;
-  } else {
-    return default_val;
-  }
 }
 
 bool opt_checkbool(lua_State *L, const char *name, bool default_val)
@@ -72,3 +72,14 @@ bool opt_checkbool(lua_State *L, const char *name, bool default_val)
     return default_val;
   }
 }
+
+const char *opt_checklstring(lua_State *L, const char *name, const char *default_val, size_t *l)
+{
+  if (opt_get(L, name, LUA_TSTRING)) {
+    const char *result = lua_tolstring(L, -1, l);
+    lua_pop(L, 1);
+    return result;
+  } else {
+    return default_val;
+  }
+}

components/modules/common.h

@@ -20,6 +20,17 @@ int opt_checkint_range(lua_State *L, const char *name, int default_val, int min_
 */
 bool opt_checkbool(lua_State *L, const char *name, bool default_val);
 
+/* Fetch an optional string from a table on the top of the stack. If the
+   option name is not present or the table is nil, returns default_val. Errors
+   if the key name is present in the table but is not a string.
+
+   Note that l is updated with the string length only when the option name
+   resolves to a string. It is not updated when default_val is returned since
+   it is not possible to determine the length of default_val in case it contains
+   embedded \0.
+*/
+const char *opt_checklstring(lua_State *L, const char *name, const char *default_val, size_t *l);
+
 /* Like luaL_argerror() but producing a more suitable error message */
 int opt_error(lua_State *L, const char* name, const char *extramsg);
 

components/modules/i2s.c

@@ -17,6 +17,8 @@
 
 #include "driver/i2s.h"
 
+#include "common.h"
+
 #define MAX_I2C_NUM 2
 #define I2S_CHECK_ID(id)   if(id >= MAX_I2C_NUM) luaL_error( L, "i2s not exists" )
 
@@ -139,48 +141,29 @@ static int node_i2s_start( lua_State *L )
   i2s_pin_config_t pin_config;
   memset( &pin_config, 0, sizeof( pin_config ) );
 
-  lua_getfield (L, 2, "mode");
+  // temporarily copy option table to top of stack for opt_ functions
-  i2s_config.mode = luaL_optint(L, -1, I2S_MODE_MASTER | I2S_MODE_TX);
+  lua_pushvalue(L, 2);
+  i2s_config.mode = opt_checkint(L, "mode", I2S_MODE_MASTER | I2S_MODE_TX);
+  i2s_config.sample_rate = opt_checkint_range(L, "rate", 44100, 1000, MAX_INT);
   //
-  lua_getfield (L, 2, "rate");
+  is->data_bits_per_sample   = opt_checkint(L, "bits", 16);
-  i2s_config.sample_rate = luaL_optint(L, -1, 44100);
-  //
-  lua_getfield (L, 2, "bits");
-  is->data_bits_per_sample   = luaL_optint(L, -1, 16);
   is->i2s_bits_per_sample    = is->data_bits_per_sample < I2S_BITS_PER_SAMPLE_16BIT ? I2S_BITS_PER_SAMPLE_16BIT : is->data_bits_per_sample;
   i2s_config.bits_per_sample = is->i2s_bits_per_sample;
   //
-  lua_getfield (L, 2, "channel");
+  i2s_config.channel_format = opt_checkint(L, "channel", I2S_CHANNEL_FMT_RIGHT_LEFT);
-  i2s_config.channel_format = luaL_optint(L, -1, I2S_CHANNEL_FMT_RIGHT_LEFT);
+  i2s_config.communication_format = opt_checkint(L, "format", I2S_COMM_FORMAT_I2S | I2S_COMM_FORMAT_I2S_MSB);
-  //
+  i2s_config.dma_buf_count = opt_checkint_range(L, "buffer_count", 2, 2, 128);
-  lua_getfield (L, 2, "format");
+  i2s_config.dma_buf_len = opt_checkint_range(L, "buffer_len", i2s_config.sample_rate / 100, 8, 1024);
-  i2s_config.communication_format = luaL_optint(L, -1, I2S_COMM_FORMAT_I2S | I2S_COMM_FORMAT_I2S_MSB);
-  //
-  lua_getfield (L, 2, "buffer_count");
-  i2s_config.dma_buf_count = luaL_optint(L, -1, 2);
-  //
-  lua_getfield (L, 2, "buffer_len");
-  i2s_config.dma_buf_len = luaL_optint(L, -1, i2s_config.sample_rate / 100);
-  //
   i2s_config.intr_alloc_flags = ESP_INTR_FLAG_LEVEL1;
   //
-  lua_getfield (L, 2, "bck_pin");
+  pin_config.bck_io_num = opt_checkint(L, "bck_pin", I2S_PIN_NO_CHANGE);
-  pin_config.bck_io_num = luaL_optint(L, -1, I2S_PIN_NO_CHANGE);
+  pin_config.ws_io_num = opt_checkint(L, "ws_pin", I2S_PIN_NO_CHANGE);
+  pin_config.data_out_num  = opt_checkint(L, "data_out_pin", I2S_PIN_NO_CHANGE);
+  pin_config.data_in_num = opt_checkint(L, "data_in_pin", I2S_PIN_NO_CHANGE);
   //
-  lua_getfield (L, 2, "ws_pin");
+  i2s_dac_mode_t dac_mode = opt_checkint_range(L, "dac_mode", I2S_DAC_CHANNEL_DISABLE, 0, I2S_DAC_CHANNEL_MAX-1);
-  pin_config.ws_io_num = luaL_optint(L, -1, I2S_PIN_NO_CHANGE);
   //
-  lua_getfield (L, 2, "data_out_pin");
+  adc1_channel_t adc1_channel = opt_checkint_range(L, "adc1_channel", ADC1_CHANNEL_MAX, ADC1_CHANNEL_0, ADC1_CHANNEL_MAX);
-  pin_config.data_out_num  = luaL_optint(L, -1, I2S_PIN_NO_CHANGE);
-  //
-  lua_getfield (L, 2, "data_in_pin");
-  pin_config.data_in_num = luaL_optint(L, -1, I2S_PIN_NO_CHANGE);
-  //
-  lua_getfield(L, 2, "dac_mode");
-  i2s_dac_mode_t dac_mode = luaL_optint(L, -1, I2S_DAC_CHANNEL_DISABLE);
-  //
-  lua_getfield(L, 2, "adc1_channel");
-  adc1_channel_t adc1_channel = luaL_optint(L, -1, ADC1_CHANNEL_MAX);
 
   // handle optional callback functions TX and RX
   lua_settop( L, top );
@@ -278,7 +261,7 @@ static int node_i2s_read( lua_State *L )
   int i2s_id = luaL_checkinteger( L, 1 );
   I2S_CHECK_ID( i2s_id );
 
-  size_t bytes = luaL_checkinteger( L, 2 );
+  const size_t bytes = luaL_checkinteger( L, 2 );
   int wait_ms = luaL_optint(L, 3, 0);
   char * data = luaM_malloc( L, bytes );
   size_t read;
@@ -286,7 +269,7 @@ static int node_i2s_read( lua_State *L )
     return luaL_error( L, "I2S driver error" );
 
   lua_pushlstring(L, data, read);
-  luaM_free(L, data);
+  luaM_freemem(L, data, bytes);
 
   return 1;
 }

components/modules/ledc.c

@@ -6,6 +6,8 @@
 
 #include "driver/ledc.h"
 
+#include "common.h"
+
 typedef struct {
   int timer;
   int channel;
@@ -14,41 +16,19 @@ typedef struct {
 
 static int lledc_new_channel( lua_State *L )
 {
-  int t=1;
+  const int top = lua_gettop(L);
-  luaL_checkanytable (L, t);
+  luaL_checkanytable (L, 1);
 
   /* Setup timer */
   ledc_timer_config_t ledc_timer;
 
-  lua_getfield(L, t, "bits");
+  ledc_timer.duty_resolution = opt_checkint_range (L, "bits", LEDC_TIMER_13_BIT, 0, LEDC_TIMER_BIT_MAX-1);
-  ledc_timer.duty_resolution = luaL_optint (L, -1, LEDC_TIMER_13_BIT);
-  if(ledc_timer.duty_resolution < LEDC_TIMER_10_BIT || ledc_timer.duty_resolution > LEDC_TIMER_15_BIT)
-    return luaL_error (L, "bits field out of range");
 
-  lua_getfield(L, t, "frequency");
+  ledc_timer.freq_hz = opt_checkint_range(L, "frequency", -1, 1, 40000000UL);
-  if (lua_type (L, -1) == LUA_TNUMBER) {
-    ledc_timer.freq_hz = luaL_checkinteger(L, -1);
-  } else {
-    return luaL_error(L, "missing or invalid 'frequency' field");
-  }
 
-  lua_getfield(L, t, "mode");
+  ledc_timer.speed_mode = opt_checkint_range(L, "mode", -1, 0, LEDC_SPEED_MODE_MAX-1);
-  if (lua_type (L, -1) == LUA_TNUMBER) {
-    ledc_timer.speed_mode = luaL_checkinteger(L, -1);
-    if(ledc_timer.speed_mode != LEDC_HIGH_SPEED_MODE && ledc_timer.speed_mode != LEDC_LOW_SPEED_MODE)
-      return luaL_error (L, "Invalid mode");
-  } else {
-    return luaL_error(L, "missing or invalid 'mode' field");
-  }
 
-  lua_getfield(L, t, "timer");
+  ledc_timer.timer_num = opt_checkint_range(L, "timer", -1, 0, LEDC_TIMER_MAX-1);
-  if (lua_type (L, -1) == LUA_TNUMBER) {
-    ledc_timer.timer_num = luaL_checkinteger(L, -1);
-    if(ledc_timer.timer_num < LEDC_TIMER_0 || ledc_timer.timer_num > LEDC_TIMER_3)
-      return luaL_error (L, "Invalid timer");
-  } else {
-    return luaL_error(L, "missing or invalid 'timer' field");
-  }
 
   /* Setup channel */
   ledc_channel_config_t channel_config = {
@@ -57,30 +37,13 @@ static int lledc_new_channel( lua_State *L )
     .intr_type = LEDC_INTR_DISABLE
   };
 
-  lua_getfield(L, t, "channel");
+  channel_config.channel = opt_checkint_range(L, "channel", -1, 0, LEDC_CHANNEL_MAX-1);
-  if (lua_type (L, -1) == LUA_TNUMBER) {
-    channel_config.channel = luaL_checkinteger(L, -1);
-    if(channel_config.channel < LEDC_CHANNEL_0 || channel_config.channel > LEDC_CHANNEL_7)
-      return luaL_error (L, "Invalid channel");
-  } else {
-    return luaL_error(L, "missing or invalid 'channel' field");
-  }
 
-  lua_getfield(L, t, "duty");
+  channel_config.duty = opt_checkint_range(L, "duty", -1, 0, 1<<(LEDC_TIMER_BIT_MAX-1));
-  if (lua_type (L, -1) == LUA_TNUMBER) {
-    channel_config.duty = luaL_checkinteger(L, -1);
-  } else {
-    return luaL_error(L, "missing or invalid 'duty' field");
-  }
 
-  lua_getfield(L, t, "gpio");
+  channel_config.gpio_num = opt_checkint_range(L, "gpio", -1, 0, GPIO_NUM_MAX-1);
-  if (lua_type (L, -1) == LUA_TNUMBER) {
+
-    channel_config.gpio_num = luaL_checkinteger(L, -1);
+  lua_settop(L, top);
-    if(!GPIO_IS_VALID_GPIO(channel_config.gpio_num))
-      return luaL_error (L, "Invalid gpio");
-  } else {
-    return luaL_error(L, "missing or invalid 'gpio' field");
-  }
 
   esp_err_t timerErr = ledc_timer_config(&ledc_timer);
   if(timerErr != ESP_OK)

components/modules/spi_master.c

@@ -12,6 +12,8 @@
 
 #include "spi_common.h"
 
+#include "common.h"
+
 #define SPI_MASTER_TAG "spi.master"
 
 #define UD_HOST_STR "spi.master"
@@ -50,10 +52,6 @@ typedef struct {
 #define GET_UD_DEVICE \
   lspi_device_t *ud = (lspi_device_t *)luaL_checkudata( L, 1, UD_DEVICE_STR );
 
-#define CONFIG_TRANS_FROM_FIELD(field) \
-  lua_getfield( L, stack, #field ); \
-  trans.field = luaL_optint( L, -1, 0 );
-
 static int lspi_device_free( lua_State *L )
 {
   GET_UD_DEVICE;
@@ -97,20 +95,21 @@ static int lspi_device_transfer( lua_State *L )
     const char * const options[] = {"std", "dio", "qio"};
     const uint32_t options_flags[] = {0, SPI_TRANS_MODE_DIO, SPI_TRANS_MODE_QIO};
 
-    CONFIG_TRANS_FROM_FIELD(cmd);
+    // temporarily copy option table to top of stack for opt_ functions
-    CONFIG_TRANS_FROM_FIELD(addr);
+    lua_pushvalue( L, stack );
     //
-    lua_getfield( L, stack, "rxlen" );
+    trans.cmd  = opt_checkint( L, "cmd", 0 );
-    rx_len = luaL_optint( L, -1, 0 );
+    trans.addr = opt_checkint( L, "addr", 0 );
     //
-    lua_getfield( L, stack, "addr_mode" );
+    rx_len = opt_checkint( L, "rxlen", 0 );
-    trans.flags |= luaL_optbool( L, -1, false ) ? SPI_TRANS_MODE_DIOQIO_ADDR : 0;
+    //
+    trans.flags |= opt_checkbool( L, "addr_mode", false ) ? SPI_TRANS_MODE_DIOQIO_ADDR : 0;
     //
     lua_getfield( L, stack, "mode" );
     trans.flags |= options_flags[ luaL_checkoption( L, -1, options[0], options ) ];
     //
-    lua_getfield( L, stack, "txdata" );
+    data_len = 0;
-    data = luaL_optlstring( L, -1, "", &data_len );
+    data = opt_checklstring( L, "txdata", "", &data_len );
 
     lua_settop( L, stack );
   }
@@ -194,15 +193,12 @@ static const LUA_REG_TYPE lspi_device_map[] = {
   lspi_host_t *ud = (lspi_host_t *)luaL_checkudata( L, 1, UD_HOST_STR );
 //
 #define CONFIG_BUS_PIN_FROM_FIELD(pin) \
-  lua_getfield( L, stack, #pin ); \
+  config.pin ## _io_num = opt_checkint( L, #pin, -1 );
-  config.pin ## _io_num = luaL_optint( L, -1, -1 );
 //
 #define CONFIG_DEVICE_FROM_INT_FIELD(field) \
-  lua_getfield( L, stack, #field ); \
+  config.field = opt_checkint( L, #field, 0 );
-  config.field = luaL_optint( L, -1, 0 );
 #define CONFIG_DEVICE_FROM_BOOL_FIELD(field, mask) \
-  lua_getfield( L, stack, #field ); \
+  config.flags |= opt_checkbool( L, #field, false ) ? mask : 0;
-  config.flags |= luaL_optbool( L, -1, false ) ? mask : 0;
 
 static int lspi_host_free( lua_State *L )
 {
@@ -220,6 +216,7 @@ static int lspi_host_free( lua_State *L )
 int lspi_master( lua_State *L )
 {
   int stack = 0;
+  int top = lua_gettop( L );
 
   int host = luaL_checkint( L, ++stack );
   luaL_argcheck( L,
@@ -231,13 +228,15 @@ int lspi_master( lua_State *L )
 
   spi_bus_config_t config;
   memset( &config, 0, sizeof( config ) );
-  //
+
+  // temporarily copy option table to top of stack for opt_ functions
+  lua_pushvalue( L, stack );
   CONFIG_BUS_PIN_FROM_FIELD(sclk);
   CONFIG_BUS_PIN_FROM_FIELD(mosi);
   CONFIG_BUS_PIN_FROM_FIELD(miso);
   CONFIG_BUS_PIN_FROM_FIELD(quadwp);
   CONFIG_BUS_PIN_FROM_FIELD(quadhd);
-  lua_pop( L, 5 );
+  lua_settop( L, top );
 
   int use_dma = luaL_optint( L, ++stack, 1 );
   luaL_argcheck( L, use_dma >= 0 && use_dma <= 2, stack, "out of range" );
@@ -267,21 +266,16 @@ static int lspi_host_device( lua_State *L )
   spi_device_interface_config_t config;
   memset( &config, 0, sizeof( config ) );
 
+  // temporarily copy option table to top of stack for opt_ functions
+  lua_pushvalue( L, stack );
+
   // mandatory fields
-  lua_getfield( L, stack, "cs" );
+  config.mode = (uint8_t)opt_checkint_range( L, "mode", -1, 0, 3 );
-  config.spics_io_num = luaL_optint( L, -1, -1 );
   //
-  lua_getfield( L, stack, "mode" );
+  config.clock_speed_hz = opt_checkint_range( L, "freq", -1, 0, SPI_MASTER_FREQ_80M );
-  int mode = luaL_optint( L, -1, -1 );
-  luaL_argcheck( L, mode >= 0, stack, "mode setting missing" );
-  config.mode = (uint8_t)mode;
-  //
-  lua_getfield( L, stack, "freq" );
-  int freq = luaL_optint( L, -1, -1 );
-  luaL_argcheck( L, freq >= 0, stack, "freq setting missing" );
-  config.clock_speed_hz = freq;
   //
   // optional fields
+  config.spics_io_num = opt_checkint( L, "cs", -1 );
   CONFIG_DEVICE_FROM_INT_FIELD(command_bits);
   CONFIG_DEVICE_FROM_INT_FIELD(address_bits);
   CONFIG_DEVICE_FROM_INT_FIELD(dummy_bits);