Allowing scripts to trap full & half press shutter button activations

lapser

1093

My preferred solution

« Reply #40 on: 26 / August / 2013, 12:14:19 »

Advertisements

Here's one more stab at getting to the best solution, incorporating everything so far:

The Problems

1. Full shutter press (shoot_full) aborts scripts. We want to make it available in scripts.
2. <alt> key stops/starts scripts. This is confusing, undesirable, and not used or useful.

Proposed Solution
1. Remove <shoot_full> script abort completely. Make <shoot_full> a normal key for scripts.
2. Make <alt> key abort scripts, and remove current behavior.
3. Change bottom screen line script message to "Press <ALT> to abort"

This should be simple to implement, and consistent and easy to use. Changing the bottom screen line for scripts was a great idea, and it can be used to educate current users that the script abort key has changed. It keeps a permanent, single abort key, and corrects the current <alt> key behavior that doesn't work right and is confusing (script still running).

One thing I think is important, whatever the solution. The abort key is not an "exit" key. It abnormally terminates the script, which should be used only if the script hangs. My current script says "Press <menu> to exit." It would be confusing for the bottom line to suggest a different "exit."

Logged

EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos

lapser

1093

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #41 on: 26 / August / 2013, 22:28:15 »

Attached is my solution to the problem, as described above. It turns "shoot_full" into a normal key visible in scripts, and uses the <alt> key to abort a script. While a script is running, it changes the message to "Press <ALT> to abort"

No script changes are required. Here's a simple test script:

Code: [Select]

--[[
@title FullTest
--]]
n=1
repeat wait_click()
  print(n,is_key("shoot_full"))
  n=n+1
until is_key("menu")

AltAbort_v1.patch (2.04 kB - downloaded 23 times.)

Logged

EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos

waterwingz

12140

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #42 on: 26 / August / 2013, 22:35:06 »

Quote from: lapser on 26 / August / 2013, 22:28:15

Attached is my solution to the problem, as described above. It turns "shoot_full" into a normal key visible in scripts, and uses the <alt> key to abort a script.

http://chdk.setepontos.com/index.php?topic=10580.msg104468#msg104468

Logged

Ported : A1200 SD940 G10 Powershot N G16

lapser

1093

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #43 on: 26 / August / 2013, 23:00:01 »

Quote from: waterwingz on 26 / August / 2013, 22:35:06

http://chdk.setepontos.com/index.php?topic=10580.msg104468#msg104468

I assume you're referring to the huge installed base that's used to aborting scripts with the shutter key?

The new message at the bottom while scripts are running that says "Press <ALT> to abort" should be able to educate the huge installed base how to abort the script. The change is a big improvement, and is the simplest solution to both the <alt> button and shutter button problems in scripts. We're just changing the abort key. It's not like it's complicated to learn.

Anyway, that's my solution and I think it's the best one. I think people should try the patch. The <ALT> button script abort works correctly. If you don't want to use it, so be it.

Logged

EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos

waterwingz

12140

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #44 on: 28 / August / 2013, 16:13:41 »

I think this thread has devolved to a matter of personal taste and a "my way is better" conversation.

So for the record, I have no "show stopper" problem with the <ALT> key only solution. I just also like the features of the other solutions that several people have suggested.

My specific reservations about making the <ALT> key the only way to abort a script continue to be :

We have a user base that expects the shutter button to abort a script and all our documentation supports that. For almost every scripting circumstance, there are no issues with continuing things the way they are. We just need a way to change the abort key for the rare case that it's essential.
The shutter button is ergonomically simple and intuitive for users to use to start and stop scripts. Its easy to find and press and always in a prominent place on every camera. For the most part, its a good solution.
If you start a script with the shutter key, it seems intuitive that you would stop it the same way. Pressing <ALT> enters and exits CHDK ALT mode, pressing Menu opens and closes menus. Pressing the shutter button to start a script should normally stop the script too. Adding a message saying "Press <ALT> to abort" helps a bit if the user notices it. Most new users are confused by the whole <ALT> key concept as there is no button labelled <ALT>. This makes it worse. And does "abort" mean exit the script? Exit CHDK? Leave the script driven menu ? Or close the script's shot meter box on the OSD?
On cameras with a limited number of buttons, it might be preferable to be able to use the button assigned as the <ALT> button as a user input button in your script. Forcing it to be the script abort button removes that choice.
Using the <ALT> key for script abort in addition to its other functions means that if its held down too long, it would then take us to also exiting CHDK and maybe invoking the <ALT> key "long press" functionality and thus the original Canon function. This could be very confusing.
Two of the other solutions to this problem can be used to enable the <ALT> button for program abort at the discretion of the script writer and/or camera owner. You are not forced into that mode.

So I have collected the four different patch file options, renamed them for clarity and attached them in one zip file for convenience. My summary is as follows :

new_menu_key.patch : adds a new menu choice to the Script menu to enable/disable using the shutter button as the abort key. This could be extended to allow the user to pick a different exit key ( including the <ALT> key ) if that seems like the best way to go.
short_and_long_shutter_press.patch : modifies the action of the shutter button so that short presses can be detected by a script and long presses abort the current script
script_selected_abort_key.patch : adds a script command that changes the abort key from the default shutter full press to a key of the user's choice (including the <ALT> key if desired)
alt_key_abort.patch : makes the <ALT> key the permanent and only script abort key.

We could probably merge all four patches into a single patch that lets you pick the script abort button via any of the following methods :

a CHDK menu entry that lists all available keys
a script command that accepts all available keys as a parameter
a CHDK menu entry option to enable abort only via a long shutter press

I suppose that would be a good case of "trying to be all things to all people" but might be the best compromise?

script_abort_patches.zip (4.69 kB - downloaded 25 times.)

« Last Edit: 28 / August / 2013, 18:42:43 by waterwingz »

Logged

Ported : A1200 SD940 G10 Powershot N G16

lapser

1093

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #45 on: 28 / August / 2013, 17:59:36 »

Thanks for all your work on this. My patch evolved entirely from what you started, and what others suggested based on what you started.

My patch is best for me because it is the easiest to understand, use, and code. I'm no longer invested in getting any changes in the trunk. I'll be using my own CHDK mods anyway, so I can always just put this in my version.

One thing that could be done is to activate my patch with a menu option, i.e. "Use <alt> as script abort key." That's all that needs to be done to resolve all the issues that you brought up. I'll be happy to write that in if everyone agrees.

Regarding your concern about pressing the <alt> key in a script having undesired effects, I solved that problem by changing where the <alt> key check goes in kbd.process.c. "key_pressed" is the counter for how long the <alt> key is down. Setting it to 0 when a script is running means the next if test is always false., and the <ALT> key starts out normally after the script is aborted by pressing and releasing the <ALT> key.

if( camera_info.state.state_kbd_script_run != SCRIPT_STATE_INACTIVE )
key_pressed=0; //disables <alt> key in scripts
if ( key_pressed && !usb_remote_active)

This is the entire patch:

Code: [Select]

Index: core/gui.c
=============================================================================
--- core/gui.c	(revision )
+++ core/gui.c	(working copy)
@@ -2292,7 +2292,10 @@
 {
 #ifdef CAM_DISP_ALT_TEXT
     gui_draw_alt_helper();
-    draw_string(((CAM_SCREEN_WIDTH/2)-(FONT_WIDTH*5/2)), (CAM_SCREEN_HEIGHT-FONT_HEIGHT), "<ALT>", MAKE_COLOR(COLOR_RED, COLOR_WHITE));
+    char* salt="Press <ALT> to abort";
+    if( camera_info.state.state_kbd_script_run == SCRIPT_STATE_INACTIVE )
+      salt="<ALT>";
+    draw_string(((CAM_SCREEN_WIDTH/2)-(FONT_WIDTH*5/2)), (CAM_SCREEN_HEIGHT-FONT_HEIGHT), salt, MAKE_COLOR(COLOR_RED, COLOR_WHITE));
 #else
     gui_draw_osd();
 #endif
Index: core/kbd_process.c
=============================================================================
--- core/kbd_process.c	(revision )
+++ core/kbd_process.c	(working copy)
@@ -129,8 +129,10 @@
 	// While running Alt. mode shoot key will start a script execution.
 
 	// alt-mode switch and delay emulation
- 
-	if ( key_pressed  && !usb_remote_active ) 
+
+  if( camera_info.state.state_kbd_script_run != SCRIPT_STATE_INACTIVE )
+    key_pressed=0; //disables <alt> key in scripts
+	if ( key_pressed  && !usb_remote_active)
 	{
         if (kbd_is_key_pressed(conf.alt_mode_button)
                 || ((key_pressed >= CAM_EMUL_KEYPRESS_DELAY)
Index: core/script.c
=============================================================================
--- core/script.c	(revision )
+++ core/script.c	(working copy)
@@ -134,8 +134,10 @@
 // Main button processing for CHDK Script mode
 static int gui_script_kbd_process()
 {
-    // Stop a script if the shutter button pressed in Script mode
-    if (kbd_is_key_clicked(KEY_SHOOT_FULL))
+    // Stop a script if the ALT button pressed in Script mode
+//    if (kbd_is_key_clicked(KEY_SHOOT_FULL))
+
+    if (camera_info.state.kbd_last_clicked==conf.alt_mode_button)
     {
         script_console_add_line(LANG_CONSOLE_TEXT_INTERRUPTED);
         if (camera_info.state.state_kbd_script_run == SCRIPT_STATE_INTERRUPTED)

Logged

EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos

waterwingz

12140

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #46 on: 28 / August / 2013, 22:55:18 »

Quote from: lapser on 28 / August / 2013, 17:59:36

One thing that could be done is to activate my patch with a menu option, i.e. "Use <alt> as script abort key." That's all that needs to be done to resolve all the issues that you brought up.

Well, if you are going to go that far, you might want to also add the ability to choose the key that serves as the exit key so that the script writer has choices. And while you are at that, you could add a script function to enable it as well - make it easier than using set_config_value(). With maybe a time delay to emulate the current short press - long press ALT code. Then guess what ? You really would have answered all the issues I brought up!

Of course, you would also have completely implemented what I suggested prior to your most recent post .....

Logged

Ported : A1200 SD940 G10 Powershot N G16

lapser

1093

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #47 on: 29 / August / 2013, 19:58:11 »

Quote from: waterwingz on 28 / August / 2013, 22:55:18

Well, if you are going to go that far, you might want to also add the ability to choose the key that serves as the exit key so that the script writer has choices. And while you are at that, you could add a script function to enable it as well - make it easier than using set_config_value(). With maybe a time delay to emulate the current short press - long press ALT code. Then guess what ? You really would have answered all the issues I brought up!

I really think that multiple "exit keys" is a mistake. It's not an exit key, it's an abort key. It's analogous to ctrl+alt+delete in Windows. It's not a proper way to stop a program. The "abort" key should be one key, possibly chosen by the user between <alt> and <full_shoot> as a menu option, for backwards compatibility. Think of it as a "panic" button.

The current <alt> key behavior while a script is running is unacceptable. It crashes the script in an unpredictable way. I think the <alt> key should always be the abort key for scripts. The shoot__full key should be an optional, additional abort key which can be disabled in the CHDK menu.

I do think a new script function is a good idea, but not to set a different abort key. It should simply disable or re-enable the abort key(s): <alt> and shoot_full. This way, the script can use all the keys, regardless of how the user sets the abort key. The script would be able to read the status of the <alt> key and implement its own long press function if it was useful. The function would be something like: disable_abort_key(true or false). The abort key would always be enabled at the start of the script.

Going out of <alt> mode while the script is running is dangerous, but letting the user pass keys directly to the camera might be useful under script control. The exit_alt() function might work for that, but a better way would be for the script to be able to read key numbers and send them to the camera. For example, to pass through key clicks, you could return the key number from wait_click() and accept a key number or a name in click(): This would pass through clicks to the camera in a script:

repeat click( wait_click() ) until false

Passing through press and release would be a little trickier, but not that hard. You need a new get_pressed_key() function that returns non-zero when a new key is pressed. Then, when the key is released, it triggers wait_click().

Quote

Of course, you would also have completely implemented what I suggested prior to your most recent post .....

Getting a little teste, eh? Let's keep working together on it and we'll figure out the best solution using everyone's input.

My basic goal is to give the script complete control of the camera and CHDK, and of course, KISS (Keep It Simple Stupid).

Logged

EOS-M3_120f / SX50_100b / SX260_101a / G1X_100g / D20_100b
https://www.youtube.com/user/DrLapser/videos

reyalp

14111

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #48 on: 14 / September / 2013, 22:57:08 »

Quote from: waterwingz on 28 / August / 2013, 16:13:41

So I have collected the four different patch file options, renamed them for clarity and attached them in one zip file for convenience. My summary is as follows :

Thanks for all your work on this. I finally got some time to work on it. I've checked in a somewhat modified version of option 3 in trunk 3106.

Note this isn't necessarily the final implementation, but it seems good enough to play with for a while and see if it meets the main goals.

My reasoning for this choice is below:

Quote

new_menu_key.patch : adds a new menu choice to the Script menu to enable/disable using the shutter button as the abort key. This could be extended to allow the user to pick a different exit key ( including the <ALT> key ) if that seems like the best way to go.

Since the main reason for this feature is to allow specific scripts that want to catch the shutter button, this seems inconvenient. Not only would users of these scripts have to set the option, it would affect all their scripts not just the ones that need it.

Quote

short_and_long_shutter_press.patch : modifies the action of the shutter button so that short presses can be detected by a script and long presses abort the current script

If we are going to go to the trouble to change it, not allowing scripts where you can hold the shutter down seems needlessly inflexible to me. There are definitely use cases where holding down the shutter would be the logical interface.

Quote

script_selected_abort_key.patch : adds a script command that changes the abort key from the default shutter full press to a key of the user's choice (including the <ALT> key if desired)

This seems like the most flexible, which is the main reason I chose it. However there are still some open issues:
It would be easy for a script to set a key that doesn't exist as the exit key. This could happen if someone downloads a script that assumes the camera has different buttons. Allowing script (and other CHDK code) to detect which keys the camera actually supports (from the kbd.c keymap) is on my todo list, so this could be addressed.

You can set the key to the alt key, but it will not interrupt the script in the current code. This is broader problem of what it actually means to press the alt key in script.

The actual code is a bit ugly, partly due to the structure of the script keyboard code.

I'm thinking that the script ALT text should become a script specific status line like
<script title> running exit=<key>
or something like that.

Quote

alt_key_abort.patch : makes the <ALT> key the permanent and only script abort key.

I'd be OK with this, but in addition to the issues waterwingz brought up earlier, I think a broader consideration of what alt mode means is required (ref http://chdk.setepontos.com/index.php?topic=10589.0 )[/list]

Logged

Don't forget what the H stands for.

waterwingz

12140

Re: Allowing scripts to trap full & half press shutter button activations

« Reply #49 on: 15 / September / 2013, 00:01:22 »

Quote from: reyalp on 14 / September / 2013, 22:57:08

Thanks for all your work on this. I finally got some time to work on it. I've checked in a somewhat modified version of option 3 in trunk 3106.

Works for me - any of the options would have been fine.

Thanks!

Quote

I'm thinking that the script ALT text should become a script specific status line like
<script title> running exit=<key> or something like that.

~~I thought I had something like that in one of the patches?~~ Update: Never mind - missed your point - I see it now in the code comments.

« Last Edit: 15 / September / 2013, 00:08:13 by waterwingz »

Logged

Ported : A1200 SD940 G10 Powershot N G16