wait_click() with short timeout - General Discussion and Assistance - CHDK Forum  
« previous next »
Pages: [1] 2 3 4 5   Go Down

wait_click() with short timeout

  • 40 Replies
  • 8521 Views
*

Offline lapser

  • *****
  • 1093
wait_click() with short timeout
« on: 23 / February / 2013, 17:53:21 »
Advertisements
I'd like to use wait_click(10) in a wait loop, but the timeout returns 10 msec too late, and it sometimes misses key presses. I was able to fix both problems as follows:

Code: [Select]
//in action.stack.c
static int action_stack_AS_WAIT_CLICK()
{
    long delay = action_top(2);
    camera_info.state.kbd_last_clicked = kbd_get_clicked_key();
    if (action_process_delay(delay) || camera_info.state.kbd_last_clicked)
//    if (action_process_delay(delay) || (camera_info.state.kbd_last_clicked = kbd_get_clicked_key()))


//in luascript.c
/*
static int luaCB_wait_click( lua_State* L )
{
  action_wait_for_click(luaL_optnumber( L, 1, 0 ));
  return lua_yield( L, 0 );
}
*/

static int luaCB_wait_click( lua_State* L )
{
  int delay=luaL_optnumber( L, 1, 0 );
  if(delay>0)
  {
    if(delay<=10)
    {
      camera_info.state.kbd_last_clicked = kbd_get_clicked_key();
      if (!camera_info.state.kbd_last_clicked)
        camera_info.state.kbd_last_clicked=0xFFFF;
      return luaCB_sleep(L);     
    }
    delay-=10;
  }
  action_wait_for_click(delay);
  return lua_yield( L, 0 );
}
The first change in action_stack.c corrects the problem with missed keys. It makes sure that it checks for a key click before timing out.

The change in luascript.c handles delays of 10 msec or less by checking for a key click and then calling the standard sleep(10) function. Phil's recent sleep() changes handle sleep(10) correctly.

I've attached a short lua script that demonstrates the new changes. Try it with the current build and you'll see that the times are 10 msec off, and key strokes are sometimes missed.

In play mode, that measured wait_click() delays are always the same. However, in record mode, the delay is occasionally 10 msec longer than specified. Maybe someone can explain why this happens? I think this may have something to do with the problems I'm having with scripts interrupting in my time lapses.
Logged
EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos
*

Offline philmoz

  • *****
  • 3226
    • Photos
Re: wait_click() with short timeout
« Reply #1 on: 23 / February / 2013, 19:44:01 »
Quote from: lapser on 23 / February / 2013, 17:53:21
I'd like to use wait_click(10) in a wait loop, but the timeout returns 10 msec too late, and it sometimes misses key presses. I was able to fix both problems as follows:

Code: [Select]
//in action.stack.c
static int action_stack_AS_WAIT_CLICK()
{
    long delay = action_top(2);
    camera_info.state.kbd_last_clicked = kbd_get_clicked_key();
    if (action_process_delay(delay) || camera_info.state.kbd_last_clicked)
//    if (action_process_delay(delay) || (camera_info.state.kbd_last_clicked = kbd_get_clicked_key()))


//in luascript.c
/*
static int luaCB_wait_click( lua_State* L )
{
  action_wait_for_click(luaL_optnumber( L, 1, 0 ));
  return lua_yield( L, 0 );
}
*/

static int luaCB_wait_click( lua_State* L )
{
  int delay=luaL_optnumber( L, 1, 0 );
  if(delay>0)
  {
    if(delay<=10)
    {
      camera_info.state.kbd_last_clicked = kbd_get_clicked_key();
      if (!camera_info.state.kbd_last_clicked)
        camera_info.state.kbd_last_clicked=0xFFFF;
      return luaCB_sleep(L);     
    }
    delay-=10;
  }
  action_wait_for_click(delay);
  return lua_yield( L, 0 );
}
The first change in action_stack.c corrects the problem with missed keys. It makes sure that it checks for a key click before timing out.

The change in luascript.c handles delays of 10 msec or less by checking for a key click and then calling the standard sleep(10) function. Phil's recent sleep() changes handle sleep(10) correctly.

I've attached a short lua script that demonstrates the new changes. Try it with the current build and you'll see that the times are 10 msec off, and key strokes are sometimes missed.

In play mode, that measured wait_click() delays are always the same. However, in record mode, the delay is occasionally 10 msec longer than specified. Maybe someone can explain why this happens? I think this may have something to do with the problems I'm having with scripts interrupting in my time lapses.


May be better to move the entire 'action_wait_for_click' logic into the Lua and uBasic modules and remove it from action_stack.c. The code can then use the 'sleep_delay' method added to fix the problem with the script 'sleep' commands.

Don't know exactly why record mode sometimes waits 10ms longer than play mode - 'sleep' has the same problem. I presume it's because the camera is doing more work (live view, etc) so the keyboard task sometimes misses a time slice.

If I get time I'll test more today.

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline lapser

  • *****
  • 1093
Re: wait_click() with short timeout
« Reply #2 on: 23 / February / 2013, 21:38:48 »
Quote from: philmoz on 23 / February / 2013, 19:44:01
May be better to move the entire 'action_wait_for_click' logic into the Lua and uBasic modules and remove it from action_stack.c. The code can then use the 'sleep_delay' method added to fix the problem with the script 'sleep' commands.
That does sound better. My way does the sleep(10) AFTER the key clicked check. The key clicked check should come last. There needs to be a minimum 10 msec (1 time slice) delay for loops like this to work:
Code: (lua) [Select]
bldark=false
repeat
  wait_click(10)
  if(is_key("set"))then
    if(bldark)then set_backlight(1) end
    bldark=not bldark   
  end
  if(bldark)then set_backlight(0) end
until get_shot_ready() or is_key("menu")
Quote
Don't know exactly why record mode sometimes waits 10ms longer than play mode - 'sleep' has the same problem. I presume it's because the camera is doing more work (live view, etc) so the keyboard task sometimes misses a time slice.
That sounds plausible. An msleep(10) or a tight loop waiting for something in the keyboard task would cause a missed action stack time slice too, wouldn't it?
Logged
EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos
*

Offline philmoz

  • *****
  • 3226
    • Photos
Re: wait_click() with short timeout
« Reply #3 on: 23 / February / 2013, 21:54:33 »
The current logic is problematic - even with your patch it can still miss key presses if the wait_click delay is short and there is a long delay between calls to wait_click. In this case the kbd_get_clicked_key function does not get called often enough to register the key.

It isn't really designed for how you are trying to use it :(

You can see this with this version of your test script. By sleeping for a second between calls to wait_click it misses the key presses altogether.
Code: [Select]
--[[
@title Keyboard Test
@param w Timeout
@default w 1
--]]
repeat
  tick0=get_tick_count()
  wait_click(w)
  print(is_key("set"),get_tick_count()-tick0)
  --if is_key("set") then sleep(1000) end
  sleep(1000)
until is_key("menu")

It really needs to call kbd_get_clicked_key and set camera_info.state.kbd_last_clicked in the keyboard task main loop, and then just use the value in the script. It's complex because you need to remember the last click until the script has finished with it; but not keep telling the script the key is still clicked.

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline philmoz

  • *****
  • 3226
    • Photos
Re: wait_click() with short timeout
« Reply #4 on: 23 / February / 2013, 22:09:17 »
Try the attached patch (for current trunk).

Key click is captured in the keyboard task loop so it should not miss them.

Phil.
Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline lapser

  • *****
  • 1093
Re: wait_click() with short timeout
« Reply #5 on: 23 / February / 2013, 22:47:35 »
Quote from: philmoz on 23 / February / 2013, 22:09:17
Try the attached patch (for current trunk).
Key click is captured in the keyboard task loop so it should not miss them.
I added this to script.c to get it to compile:

#include "clock.h"

It works great with my test lua script on the SX260. I noticed that it returns with 0 delay if you press a key outside of wait_click(), i.e. during the sleep(1000). This is the best result, since there's no need to delay 10 msec if the key is already there. If a key is not waiting, it delays at least 10 msec, so it should work in wait loops as a substitute for sleep(10).

I'll add the patch to my time lapse mods and test it on the G1X. Thanks Phil. Great job!
Logged
EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos
*

Offline lapser

  • *****
  • 1093
Re: wait_click() with short timeout
« Reply #6 on: 24 / February / 2013, 09:42:25 »
I found another correction that needs to be made to the patch. In ubasic.c you need to start with:

int delay=-1;     instead of     int delay=0;

A nice feature is that if you call    wait_click(0)    it checks for a click and returns immediately. You could do this:

repeat
  sleep(10)
  wait_click(0)
  if(not iskey("no_key"))then
    --process keys here
  end
until is_key("menu")

In Lua, it would be nice if wait_click(timeout) would return a boolean true if a key is ready, and false if it timed out.

You could also add a boolean function like:     get_key_ready()     Then you wouldn't have to worry about the precise timing of wait_click(timeout), and could leave it in action_stack.c modified to handle your new keyboard polling method. Scripts might be easier to read this way:

repeat sleep(10) until get_key_ready()

You could even eliminate wait_click(timeout) and use get_key_ready() instead (except for old scripts).

[EDIT]
An even better way to do it is to use a keyboard type-ahead buffer. You could eliminate the get_tick_count() saves, and it would work even better. Here's an example of how you could do it:

Code: [Select]
#define KBD_NKBUFFER 16
static int kbuffer[KBD_NKBUFFER],wkbuffer=0,rkbuffer=0;

static void kbd_save_key(int key)
{
  int wnext=(wkbuffer+1)%KBD_NKBUFFER;
  if(wnext==rkbuffer)return; //buffer overflow
  kbuffer[wkbuffer]=key;
  wkbuffer=wnext;
}

int kbd_get_key()
{
  if(rkbuffer==wkbuffer)return 0;
  int key=kbuffer[rkbuffer];
  rkbuffer=(rkbuffer+1)%KBD_NKBUFFER;
  return key;
}

« Last Edit: 24 / February / 2013, 11:44:47 by lapser »
Logged
EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos
*

Offline lapser

  • *****
  • 1093
Re: wait_click() with short timeout
« Reply #7 on: 25 / February / 2013, 15:13:28 »
After a little thought, a keyboard type ahead buffer is not really necessary. Phil's patch seems to work, so it may not be worth the time either. If you think the current patch is ready for the trunk, go ahead and we'll move on to something else. I'll keep testing the patch in the mean time.

If you really like the buffer idea, and get_key_ready(), I'll be glad to keep working on it with you too.
Logged
EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos
*

Offline philmoz

  • *****
  • 3226
    • Photos
Re: wait_click() with short timeout
« Reply #8 on: 25 / February / 2013, 16:43:05 »
Quote from: lapser on 25 / February / 2013, 15:13:28
After a little thought, a keyboard type ahead buffer is not really necessary. Phil's patch seems to work, so it may not be worth the time either. If you think the current patch is ready for the trunk, go ahead and we'll move on to something else. I'll keep testing the patch in the mean time.

If you really like the buffer idea, and get_key_ready(), I'll be glad to keep working on it with you too.

Didn't get time to look at this much yesterday.

At this stage the buffering is probably overkill, and after looking at the current way key presses/clicks are handled in the code, it would probably require major surgery to implement ;)

Returning a boolean from wait_click is easy in Lua; but would change the syntax for uBasic since you would need to supply another variable to store the 'return' value. I'd prefer to keep the two operating in a similar manner to avoid confusing script writers too much. If it can be done without breaking existing scripts then it's a good idea.

I'm not sure about this code:
Code: [Select]
repeat
  sleep(10)
  wait_click(0)
...
until ...

Isn't this exactly the same as:
Code: [Select]
repeat
  wait_click(10)
...
until ...

Since the 'wait_click' code is only used by scripts I think I'll leave it in the script modules. Only one of them can be loaded at a time so duplicating the code in the modules doesn't have a run time cost, there's a small maintenance overhead; but I don't think it's substantial.

I'm not sure about 'repeat sleep(10) until get_key_ready()'. It is essentially the same as 'wait_click(-1)'.

If you really see the need to be able to test if any key has been clicked (rather than a specific one) then I would suggest modifying 'iskey' so that if no parameter is supplied then it tests for any key.
You could use:
        if (iskey()) then
in place of
        if (not iskey("no_key")) then

Phil.

Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline lapser

  • *****
  • 1093
Re: wait_click() with short timeout
« Reply #9 on: 26 / February / 2013, 00:34:27 »
Quote from: philmoz on 25 / February / 2013, 16:43:05
At this stage the buffering is probably overkill, and after looking at the current way key presses/clicks are handled in the code, it would probably require major surgery to implement ;)
That's what I thought, but after exploring a little, it looks like it would be very simple. I added this in kbd_process() where you had your key save code:

    if (kbd_get_clicked_key())lapser5++;

Then I displayed lapser5 from main.c and discovered that it increments exactly once for every key click. So all we need to do to save the key clicks is:

kbd_save_key(kbd_get_clicked_key());

I'll add a test for 0 in kbd_save_key(int key)

To read the key, it seems a simple new script call would be the easiest:

get_key()

It would just return kbd_read_key(), and save the key for subsequent calls to is_key("..."). You could also process the key number instead of using is_key(). Hopefully, each key name has a unique number.

if(get_key()>0) then
  if(is_key("set")then done=true end
end

key=get_key() is much easier to understand than wait_click(0); if(is_key(..)) etc.

In fact, the keyboard buffer eliminates the need for wait_click, since it doesn't miss keystrokes.

Let me try coding it, and see if it works without using wait_click.
Logged
EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos
Pages: [1] 2 3 4 5   Go Up
« previous next »
 

Related Topics