Browse Source

Fix a number of issues with the gpio.pulse family of functions (#2260)

* Fix some subtle timing issues with gpio.pulse
* Add the pulse:update method
* Allow getstate to work on stopped pulsers
* Make gpio.mode(, gpio.OUTPUT) actually set the output mode
* Added some more documentation
Philip Gladstone 6 years ago
parent
commit
97e34ce520

+ 4 - 0
app/include/rtc/rtctime_internal.h

@@ -779,6 +779,10 @@ static inline void rtc_time_prepare(void)
   rtc_time_select_frc2_source();
 }
 
+static int32_t rtc_time_adjust_delta_by_rate(int32_t delta) {
+  return (delta * ((1ull << 32) + (int) rtc_usrate)) >> 32;
+}
+
 static uint64_t rtc_time_adjust_us_by_rate(uint64_t us, int force) {
   uint64_t usoff = us - rtc_usatlastrate; 
   uint64_t usadj = (usoff * ((1ull << 32) + (int) rtc_usrate)) >> 32;

+ 140 - 74
app/modules/gpio_pulse.c

@@ -12,12 +12,18 @@
 
 #define TIMER_OWNER 'P'
 
+#define xstr(s) str(s)
+#define str(s) #s
+
+// Maximum delay in microseconds
+#define DELAY_LIMIT 64000000
+
 typedef struct {
   uint32_t gpio_set;
   uint32_t gpio_clr;
-  uint32_t delay;
-  uint32_t delay_min;
-  uint32_t delay_max;
+  int32_t delay;
+  int32_t delay_min;
+  int32_t delay_max;
   uint32_t count;
   uint32_t count_left;
   uint16_t loop;
@@ -27,10 +33,13 @@ typedef struct {
   uint32_t entry_count;
   volatile uint32_t steps;
   volatile uint16_t entry_pos;
+  volatile uint16_t active_pos;
   volatile int16_t stop_pos;  // -1 is stop nowhere, -2 is stop everywhere, otherwise is stop point
   pulse_entry_t *entry;
   volatile uint32_t expected_end_time;
+  volatile uint32_t desired_end_time;
   volatile int32_t next_adjust;
+  volatile int32_t pending_delay;
   volatile int cb_ref;
 } pulse_t;
 
@@ -41,21 +50,21 @@ static task_handle_t tasknumber;
 static int gpio_pulse_push_state(lua_State *L, pulse_t *pulser) {
   uint32_t now;
   uint32_t expected_end_time;
-  uint32_t entry_pos;
+  uint32_t active_pos;
   uint32_t steps;
   do {
     now = 0x7FFFFFFF & system_get_time();
     expected_end_time = pulser->expected_end_time;
-    entry_pos = pulser->entry_pos;
+    active_pos = pulser->active_pos;
     steps = pulser->steps;
   } while (expected_end_time != pulser->expected_end_time ||
-           entry_pos != pulser->entry_pos ||
+           active_pos != pulser->active_pos ||
            steps != pulser->steps);
 
-  if (entry_pos >= pulser->entry_count) {
+  if (active_pos >= pulser->entry_count) {
     lua_pushnil(L);
   } else {
-    lua_pushinteger(L, entry_pos + 1);    // Lua is 1 offset
+    lua_pushinteger(L, active_pos + 1);    // Lua is 1 offset
   }
   lua_pushinteger(L, steps);
 
@@ -68,10 +77,6 @@ static int gpio_pulse_push_state(lua_State *L, pulse_t *pulser) {
 static int gpio_pulse_getstate(lua_State *L) {
   pulse_t *pulser = luaL_checkudata(L, 1, "gpio.pulse");
 
-  if (pulser != active_pulser) {
-    return 0;
-  }
-
   return gpio_pulse_push_state(L, pulser);
 }
 
@@ -128,6 +133,69 @@ static int gpio_pulse_delete(lua_State *L) {
   return 0;
 }
 
+static void fill_entry_from_table(lua_State *L, pulse_entry_t *entry) {
+  if (lua_type(L, -1) != LUA_TTABLE) {
+    luaL_error(L, "All entries must be tables");
+  }
+
+  lua_pushnil(L);             // key position
+  while (lua_next(L, -2)) {
+    // stack now contains: -1 => value; -2 => key; -3 => table
+    if (lua_type(L, -2) == LUA_TNUMBER) {
+      int pin = luaL_checkint(L, -2);
+      int value = luaL_checkint(L, -1);
+
+      if (pin < 0 || pin >= GPIO_PIN_NUM) {
+        luaL_error(L, "pin number %d must be in range 0 .. %d", pin, GPIO_PIN_NUM - 1);
+      }
+
+      if (value) {
+        entry->gpio_set |= BIT(pin_num[pin]);
+      } else {
+        entry->gpio_clr |= BIT(pin_num[pin]);
+      }
+    } else {
+      const char *str = luaL_checkstring(L, -2);
+
+      if (strcmp(str, "delay") == 0) {
+        entry->delay = luaL_checkint(L, -1);
+        if (entry->delay < 0 || entry->delay > DELAY_LIMIT) {
+          luaL_error(L, "delay of %d must be in the range 0 .. " xstr(DELAY_LIMIT) " microseconds", entry->delay);
+        }
+      } else if (strcmp(str, "min") == 0) {
+        entry->delay_min = luaL_checkint(L, -1);
+        if (entry->delay_min < 0 || entry->delay_min > DELAY_LIMIT) {
+          luaL_error(L, "delay minimum of %d must be in the range 0 .. " xstr(DELAY_LIMIT) " microseconds", entry->delay_min);
+        }
+      } else if (strcmp(str, "max") == 0) {
+        entry->delay_max = luaL_checkint(L, -1);
+        if (entry->delay_max < 0 || entry->delay_max > DELAY_LIMIT) {
+          luaL_error(L, "delay maximum of %d must be in the range 0 .. " xstr(DELAY_LIMIT) " microseconds", entry->delay_max);
+        }
+      } else if (strcmp(str, "count") == 0) {
+        entry->count = luaL_checkint(L, -1);
+      } else if (strcmp(str, "loop") == 0) {
+        entry->loop = luaL_checkint(L, -1);
+      } else {
+        luaL_error(L, "Unrecognized key found: %s", str);
+      }
+    }
+    lua_pop(L, 1);
+  }
+
+  if (entry->delay_min != -1 || entry->delay_max != -1) {
+    if (entry->delay_min == -1) {
+      entry->delay_min = 0;
+    }
+    if (entry->delay_min > entry->delay ||
+        entry->delay_max < entry->delay) {
+      luaL_error(L, "Delay of %d must be between min and max", entry->delay);
+    }
+  }
+
+  lua_pop(L, 1);
+}
+
 static int gpio_pulse_build(lua_State *L) {
   // Take a table argument
   luaL_checktype(L, 1, LUA_TTABLE);
@@ -159,69 +227,34 @@ static int gpio_pulse_build(lua_State *L) {
 
     lua_rawgeti(L, 1, i + 1);
 
-    if (lua_type(L, -1) != LUA_TTABLE) {
-      return luaL_error(L, "All entries must be tables");
-    }
+    fill_entry_from_table(L, entry);
+  }
 
-    lua_pushnil(L);             // key position
-    while (lua_next(L, -2)) {
-      // stack now contains: -1 => value; -2 => key; -3 => table
-      if (lua_type(L, -2) == LUA_TNUMBER) {
-        int pin = lua_tonumber(L, -2);
-        int value = lua_tonumber(L, -1);
+  return 1;
+}
 
-        if (pin < 0 || pin >= GPIO_PIN_NUM) {
-          luaL_error(L, "pin number %d must be in range 0 .. %d", pin, GPIO_PIN_NUM - 1);
-        }
+static int gpio_pulse_update(lua_State *L) {
+  pulse_t *pulser = luaL_checkudata(L, 1, "gpio.pulse");
+  int entry_pos = luaL_checkinteger(L, 2);  
 
-        if (value) {
-          entry->gpio_set |= BIT(pin_num[pin]);
-        } else {
-          entry->gpio_clr |= BIT(pin_num[pin]);
-        }
-      } else {
-        const char *str = lua_tostring(L, -2);
-
-        if (strcmp(str, "delay") == 0) {
-          entry->delay = lua_tonumber(L, -1);
-          if (entry->delay < 0 || entry->delay > 1000000) {
-            return luaL_error(L, "delay of %d must be in the range 0 .. 1000000 microseconds", entry->delay);
-          }
-        } else if (strcmp(str, "min") == 0) {
-          entry->delay_min = lua_tonumber(L, -1);
-          if (entry->delay_min < 0 || entry->delay_min > 1000000) {
-            return luaL_error(L, "delay minimum of %d must be in the range 0 .. 1000000 microseconds", entry->delay_min);
-          }
-        } else if (strcmp(str, "max") == 0) {
-          entry->delay_max = lua_tonumber(L, -1);
-          if (entry->delay_max < 0 || entry->delay_max > 1000000) {
-            return luaL_error(L, "delay maximum of %d must be in the range 0 .. 1000000 microseconds", entry->delay_max);
-          }
-        } else if (strcmp(str, "count") == 0) {
-          entry->count = lua_tonumber(L, -1);
-        } else if (strcmp(str, "loop") == 0) {
-          entry->loop = lua_tonumber(L, -1);
-        } else {
-          return luaL_error(L, "Unrecognized key found: %s", str);
-        }
-      }
-      lua_pop(L, 1);
-    }
+  if (entry_pos < 1 || entry_pos > pulser->entry_count) {
+    return luaL_error(L, "entry number must be in range 1 .. %d", pulser->entry_count);
+  }
 
-    if (entry->delay_min != -1 || entry->delay_max != -1) {
-      if (entry->delay_min == -1) {
-        entry->delay_min = 0;
-      }
-      if (entry->delay_min > entry->delay ||
-          entry->delay_max < entry->delay) {
-        return luaL_error(L, "Delay of %d must be between min and max", entry->delay);
-      }
-    }
+  pulse_entry_t *entry = pulser->entry + entry_pos - 1;
 
-    lua_pop(L, 1);
-  }
+  pulse_entry_t new_entry = *entry;
 
-  return 1;
+  lua_pushvalue(L, 3);
+
+  fill_entry_from_table(L, &new_entry);
+
+  // Now do the update
+  ETS_FRC1_INTR_DISABLE();
+  *entry = new_entry;
+  ETS_FRC1_INTR_ENABLE();
+
+  return 0;
 }
 
 static int gpio_pulse_adjust(lua_State *L) {
@@ -264,9 +297,25 @@ static int gpio_pulse_cancel(lua_State *L) {
 static void ICACHE_RAM_ATTR gpio_pulse_timeout(os_param_t p) {
   (void) p;
 
+  uint32_t now = system_get_time();
+
   int delay;
 
+  if (active_pulser) {
+    delay = active_pulser->pending_delay;
+    if (delay > 0) {
+      if (delay > 1200000) {
+        delay = 1000000;
+      }
+      active_pulser->pending_delay -= delay;
+      platform_hw_timer_arm_us(TIMER_OWNER, delay);
+      return;
+    }
+  }
+
   do {
+    active_pulser->active_pos = active_pulser->entry_pos;
+
     if (!active_pulser || active_pulser->entry_pos >= active_pulser->entry_count) {
       if (active_pulser) {
         active_pulser->steps++;
@@ -311,23 +360,38 @@ static void ICACHE_RAM_ATTR gpio_pulse_timeout(os_param_t p) {
 
     delay = entry->delay;
 
+    int delay_offset = 0;
+
     if (entry->delay_min != -1) {
       int offset = active_pulser->next_adjust;
       active_pulser->next_adjust = 0;
-      int delay_offset = 0x7fffffff & (system_get_time() - (active_pulser->expected_end_time + offset));
-      delay -= (delay_offset << 1) >> 1;
+      delay_offset = ((0x7fffffff & (now - active_pulser->desired_end_time)) << 1) >> 1;
+      delay -= delay_offset;
+      delay += offset;
+      //dbg_printf("%d(et %d diff %d): Delay was %d us, offset = %d, delay_offset = %d, new delay = %d, range=%d..%d\n", 
+      //           now, active_pulser->desired_end_time, now - active_pulser->desired_end_time, 
+      //           entry->delay, offset, delay_offset, delay, entry->delay_min, entry->delay_max);
       if (delay < entry->delay_min) {
-        active_pulser->next_adjust = entry->delay_min - delay;
+        // we can't delay as little as 'delay', so we need to adjust
+        // the next period as well.
+        active_pulser->next_adjust = (entry->delay - entry->delay_min) + offset;
         delay = entry->delay_min;
       } else if (delay > entry->delay_max) {
-        active_pulser->next_adjust = entry->delay_max - delay;
+        // we can't delay as much as 'delay', so we need to adjust
+        // the next period as well.
+        active_pulser->next_adjust = (entry->delay - entry->delay_max) + offset;
         delay = entry->delay_max;
       }
     }
 
-    active_pulser->expected_end_time += delay;
+    active_pulser->desired_end_time += delay + delay_offset;
+    active_pulser->expected_end_time = system_get_time() + delay;
   } while (delay < 3);
   
+  if (delay > 1200000) {
+    active_pulser->pending_delay = delay - 1000000;
+    delay = 1000000;
+  }
   platform_hw_timer_arm_us(TIMER_OWNER, delay);
 }
 
@@ -364,6 +428,7 @@ static int gpio_pulse_start(lua_State *L) {
     pulser->entry[i].count_left = 0;
   }
   pulser->entry_pos = 0;
+  pulser->active_pos = 0;
   pulser->steps = 0;
   pulser->stop_pos = -1;
   pulser->next_adjust = initial_adjust;
@@ -408,6 +473,7 @@ static const LUA_REG_TYPE pulse_map[] = {
   { LSTRKEY( "cancel" ),              LFUNCVAL( gpio_pulse_cancel ) },
   { LSTRKEY( "start" ),               LFUNCVAL( gpio_pulse_start ) },
   { LSTRKEY( "adjust" ),              LFUNCVAL( gpio_pulse_adjust ) },
+  { LSTRKEY( "update" ),              LFUNCVAL( gpio_pulse_update ) },
   { LSTRKEY( "__gc" ),                LFUNCVAL( gpio_pulse_delete ) },
   { LSTRKEY( "__index" ),             LROVAL( pulse_map ) },
   { LNILKEY, LNILVAL }

+ 10 - 0
app/modules/rtctime.c

@@ -167,6 +167,15 @@ static void do_sleep_opt (lua_State *L, int idx)
   }
 }
 
+// rtctime.adjust_delta (usec)
+static int rtctime_adjust_delta (lua_State *L)
+{
+  uint32_t us = luaL_checknumber (L, 1);
+  lua_pushnumber(L, rtc_time_adjust_delta_by_rate(us));
+  return 1;
+}
+
+
 // rtctime.dsleep (usec, option)
 static int rtctime_dsleep (lua_State *L)
 {
@@ -222,6 +231,7 @@ static int rtctime_epoch2cal (lua_State *L)
 static const LUA_REG_TYPE rtctime_map[] = {
   { LSTRKEY("set"),            LFUNCVAL(rtctime_set) },
   { LSTRKEY("get"),            LFUNCVAL(rtctime_get) },
+  { LSTRKEY("adjust_delta"),   LFUNCVAL(rtctime_adjust_delta) },
   { LSTRKEY("dsleep"),         LFUNCVAL(rtctime_dsleep)  },
   { LSTRKEY("dsleep_aligned"), LFUNCVAL(rtctime_dsleep_aligned) },
   { LSTRKEY("epoch2cal"),      LFUNCVAL(rtctime_epoch2cal) },

+ 2 - 0
app/platform/platform.c

@@ -105,6 +105,8 @@ static void NO_INTR_CODE set_gpio_no_interrupt(uint8 pin, uint8_t push_pull) {
                    GPIO_REG_READ(GPIO_PIN_ADDR(GPIO_ID_PIN(pnum))) |
                    GPIO_PIN_PAD_DRIVER_SET(GPIO_PAD_DRIVER_ENABLE));      //enable open drain;
   }
+
+  GPIO_REG_WRITE(GPIO_ENABLE_W1TS_ADDRESS, BIT(pnum));
   ETS_GPIO_INTR_ENABLE();
 }
 

+ 64 - 10
docs/en/modules/gpio.md

@@ -191,7 +191,7 @@ timing control.
 
 The basic idea is to build a `gpio.pulse` object and then control it with methods on that object. Only one `gpio.pulse`
 object can be active at a time. The object is built from an array of tables where each inner table represents
-an action to take and the time to delay before moving to the next action.
+an action to take and the time to delay before moving to the next action. 
 
 One of the uses for this is to generate bipolar impulse for driving clock movements where you want (say) a pulse on Pin 1 on the even
 second, and a pulse on Pin 2 on the odd second. `:getstate` and `:adjust` can be used to keep the pulse synchronized to the
@@ -201,6 +201,32 @@ RTC clock (that is itself synchronized with NTP).
 
 	This sub module is disabled by default. Uncomment `LUA_USE_MODULES_GPIO_PULSE` in `app/include/user_modules.h` before building the firmware to enable it.
 
+To make use of this feature, decide on the sort of pulse train that you need to generate -- hopefully it repeats a number of times.
+Decide on the number of GPIO pins that you will be using. Then draw up a chart of what you want to happen, and in what order. Then
+you can construct the table struct that you pass into `gpio.pulse.build`. For example, for the two out of phase square waves, you might do:
+
+Step | Pin 1 | Pin 2 | Duration (&#956;S) | Next Step
+---:|---|---|---:| --:
+1 | High | Low | 100,000 | 2
+2 | Low | High | 100,000 | **1**
+
+This would (when built and started) just runs step 1 (by setting the output pins as specified), and then after 100,000&#956;S, it changes to step 2i. This
+alters the output pins
+and then waits for 100,000&#956;S before going back to step 1. This has the effect of outputting to Pin 1 and Pin 2 a 5Hz square wave with the pins being out of phase. The frequency will be
+slightly lower than 5Hz as this is software generated and interrupt masking can delay the move to the next step. To get much closer to 5Hz,
+you want to allow the duration of each step to vary slightly. This will then adjust the length of each step so that, overall, the output is
+at 5Hz.
+
+Step | Pin 1 | Pin 2 | Duration (&#956;S) | Range | Next Step
+---:|---|---|---:|:---:| --:
+1 | High | Low | 100,000 | 90,000 - 110,000 | 2
+2 | Low | High | 100,000 | 90,000 - 110,000 | **1**
+
+When turning this into the table structure as described below, you don't need to specify anything
+special when the number of the next step is one more than the current step. When specifying an out of order
+step, you must specify how often you want this to be performed. A very large value can be used for roughly infinite.
+
+
 ## gpio.pulse.build
 
 This builds the `gpio.pulse` object from the supplied argument (a table as described below).
@@ -211,27 +237,34 @@ This builds the `gpio.pulse` object from the supplied argument (a table as descr
 #### Parameter
 `table` this is view as an array of instructions. Each instruction is represented by a table as follows:
 
-- All numeric keys are considered to be pin numbers. The values of each are the value to be set onto the respective GPIO line. For example `{ [1] = gpio.HIGH }` would set pin 1 to be high. Note this that is the pin number and *not* the GPIO number. Multiple pins can be
-set at the same time.
-- `delay` specifies the number of microseconds after setting the pin values to wait until moving to the next state. The actual delay may be longer than this value depending on whether interrupts are enabled at the end time.
+- All numeric keys are considered to be pin numbers. The values of each are the value to be set onto the respective GPIO line. 
+For example `{ [1] = gpio.HIGH }` would set pin 1 to be high. 
+Note this that is the NodeMCU pin number and *not* the ESP8266 GPIO number. Multiple pins can be
+set at the same time. Note that any valid GPIO pin can be used, including pin 0.
+- `delay` specifies the number of microseconds after setting the pin values to wait until moving to the next state. The actual delay may be longer than this value depending on whether interrupts are enabled at the end time. The maximum value is 64,000,000 -- i.e. a bit more than a minute.
 - `min` and `max` can be used to specify (along with `delay`) that this time can be varied. If one time interval overruns, then the extra time will be deducted from a time period which has a `min` or `max` specified. The actual time can also be adjusted with the `:adjust` API below.
-- `count` and `loop` allow simple looping. When a state with `count` and `loop` is completed, the next state is at `loop` (provided that `count` has not decremented to zero). The first state is state 1.
+- `count` and `loop` allow simple looping. When a state with `count` and `loop` is completed, the next state is at `loop` (provided that `count` has not decremented to zero). The first state is state 1. The `loop` is rather like a goto instruction as it specifies the next instruction to be executed.
 
 #### Returns
 `gpio.pulse` object.
 
 #### Example
 ```lua
-gpio.mode(1,gpio.OUTPUT,gpio.PULLUP)
-gpio.mode(2,gpio.OUTPUT,gpio.PULLUP)
+gpio.mode(1, gpio.OUTPUT)
+gpio.mode(2, gpio.OUTPUT)
 
 pulser = gpio.pulse.build( {
-  { [1] = gpio.HIGH, [2] = gpio.LOW, delay=100000 },
-  { [1] = gpio.LOW, [2] = gpio.HIGH, delay=100000, loop=1, count=100, min=90000, max=110000 }
+  { [1] = gpio.HIGH, [2] = gpio.LOW, delay=250000 },
+  { [1] = gpio.LOW, [2] = gpio.HIGH, delay=250000, loop=1, count=20, min=240000, max=260000 }
 })
 
+pulser:start(function() print ('done') end)
+
 ```
+This will generate a square wave on pins 1 and 2, but they will be exactly out of phase. After 10 seconds, the sequence will end, with pin 2 being high.
 
+Note that you *must* set the pins into output mode (either gpio.OUTPUT or gpio.OPENDRAIN) before starting the output sequence, otherwise
+nothing will appear to happen.
 
 ## gpio.pulse:start
 
@@ -274,7 +307,7 @@ loops.
 
 #### Example
 ```lua
-pos, steps, offset, new = pulser:getstate()
+pos, steps, offset, now = pulser:getstate()
 print (pos, steps, offset, now)
 ```
 
@@ -347,3 +380,24 @@ loops.
 pulser:adjust(177)
 ```
 
+## gpio.pulse:update
+
+This can change the contents of a particular step in the output program. This can be used to adjust the delay times, or even the pin changes. This cannot
+be used to remove entries or add new entries.
+
+#### Syntax
+`pulser:update(entrynum, entrytable)`
+
+#### Parameters
+- `entrynum` is the number of the entry in the original pulse sequence definition. The first entry is numbered 1.
+- `entrytable` is a table containing the same keys as for `gpio.pulse.build`
+
+#### Returns
+Nothing
+
+
+ Example
+```lua
+pulser:update(1, { delay=1000 })
+```
+

+ 27 - 0
docs/en/modules/rtctime.md

@@ -157,3 +157,30 @@ rtctime.set(1436430589, 0)
 ```
 #### See also
 [`sntp.sync()`](sntp.md#sntpsync)
+
+## rtctime.adjust_delta()
+
+This takes a time interval in 'system clock microseconds' based on the timestamps returned by `tmr.now` and returns 
+an adjusted time interval in 'wall clock microseconds'. The size of the adjustment is typically pretty small as it (roughly) the error in the
+crystal clock rate. This function is useful in some precision timing applications.
+
+#### Syntax
+`rtctime.adjust_delta(microseconds)`
+
+#### Parameters
+- `microseconds` a time interval measured in system clock microseconds.
+
+#### Returns
+The same interval but measured in wall clock microseconds
+
+#### Example
+```lua
+local start = tmr.now()
+-- do something
+local end = tmr.now()
+print ('Duration', rtctime.adjust_delta(end - start))
+
+-- You can also go in the other direction (roughly)
+local one_second = 1000000
+local ticks_in_one_second = one_second - (rtctime.adjust_delta(one_second) - one_second)
+```