Making scripting support optional - page 2 - General Discussion and Assistance - CHDK Forum
« previous next »
Pages: 1 [2] 3 4 5 6 7 ... 9   Go Down

Making scripting support optional

  • 83 Replies
  • 30811 Views
*

Offline reyalp

  • ******
  • 14079
Re: Making scripting support optional
« Reply #10 on: 08 / November / 2010, 21:48:47 »
Advertisements
Quote from: ultimA on 08 / November / 2010, 13:37:00
What is the best way to trigger a shoot from C-code and wait for its completion? Or isn't anything like that available?
That's probably why it's in script. The remote code (in core/kbd.c) might give you some ideas, but it's hideous (speaking of things that need cleaning up...)

You could also probably abuse the script stack in some way.

An alternate way to do this would be to make the user do it:
"Please take the first picture"
...idle until a shot has been taken...
"Please take the second picture"
...make badpixel...
Logged
Don't forget what the H stands for.
*

Offline ultimA

  • ***
  • 137
Re: Making scripting support optional
« Reply #11 on: 10 / November / 2010, 03:08:42 »
« Last Edit: 11 / November / 2010, 20:35:59 by ultimA »
Logged
*

Offline whim

  • ******
  • 2046
  • A495/590/620/630 ixus70/115/220/230/300/870 S95
Re: Making scripting support optional
« Reply #12 on: 10 / November / 2010, 06:37:07 »
@ultimA

First of all: thanks ! This must have been LOTS of work ...

About "chdk_scriptless_v1.diff":

- it might be a good idea to somehow refer to the reference trunk build (964) in the name
- my patch.exe (v. 2.5.4, 11-07-2009) choked on the - very simple - patch to ptp.c
  (I'm using 'patch --verbose -p0 < {patchfile}')


DISKBOOT.BIN for ixus870_sd880 101a , compiled with GCC 4.5.1
(all options OFF, except: GAMES, CURVES, TEXTREADER, CALENDAR, DEBUGGING, EDGEOVERLAY)

unpatched 964: 242.249 bytes

   patched 964: 160.033 bytes

Now I'm off to test the binary

cheers,

wim

edit: more feedback on compiling 964 patched (all OPT on)
Quote
motion_detector.c: In function 'md_save_calls_history':
motion_detector.c:272:3: warning: implicit declaration of function 'script_console_add_line'
adding #include "console.h" and changing 'script_console_add_line' -> 'console_add_line'
cured the warning
      
« Last Edit: 10 / November / 2010, 09:45:05 by whim »
Logged
*

Offline ultimA

  • ***
  • 137
Re: Making scripting support optional
« Reply #13 on: 10 / November / 2010, 12:18:22 »
Thx whim! I've incorporated your feedback, but I can do nothing about your patch.exe complaining. I'm just doing an "svn diff", that's all, it must be some incompatibility between the tool versions (I have SVN 1.6.9).

I've updated the patch in the post above (it is now v2). In addition to whim's feedback, I've realized that ptp.c still needed to be made scriptless, so I've done that too. Some tiny refactoring for the console is also present, notably it is not absolutely needed anymore to call init on it, as it will initialize itself when needed. Defined CAM_CHDK_PTP for the SX20 too.

Edit: new update v2->v3: Prevent forwarding keys to the gui while a script is running (a one-liner fix).
« Last Edit: 11 / November / 2010, 20:39:21 by ultimA »
Logged
*

Offline reyalp

  • ******
  • 14079
Re: Making scripting support optional
« Reply #14 on: 13 / November / 2010, 21:31:16 »
Finally got a chance to look at this.

First, thanks a lot for working on it. This is all good stuff to have, and you've obviously put a lot of work into it. One way or another, all or most of this should go into the trunk.

In general, I much prefer multiple patches for things that can be separated out. It makes it much easier to understand whats going on, both while I'm trying to review and merge it, and later down the road if something breaks. One giant patch like this is very hard to review, and requires a single, big chunk of time.

Things that are easy to understand and isolated can be added quickly.  If there's something that I don't like, that one part can be sent back to you without rejecting everything.

From what I've looked at so far, this could be split into
- unrelated changes like adding ptp to sx20, the filereader conf bug etc.
- refactor console out of script
- rename various things ubasic->camera
- rename/refactor script stack
- non-script badpixel
- ifdefs for ubasic/lua

I know this is somewhat annoying to do with SVN, since dependent patches basically require you to wait for the earlier ones to be committed before you can make the later ones, or to do non-svn diffs between trees. For independent ones, you can just use different working copies, or save the changes off in a patch, revert, and do the next one.

I'm not asking you to split everything up for this one. If you can break out the stuff that's really independent, that would help.

On the script stack, I'd suggest not calling it task_stack because task already means something quite different, and OS tasks also have stacks. action_stack maybe ? automation_stack ?

On PTP:
The "minimal proposal" PTP currently in the trunk basically assumes everything will be done in lua. Without lua, you pretty much only have file copy and reboot. Both are still useful, so that's OK, but it's something to think about. Also, the protocol should probably indicate whether lua is available somehow.
Logged
Don't forget what the H stands for.
*

Offline asm1989

  • *****
  • 527
  • SX720, SX260, SX210 & SX200
Re: Making scripting support optional
« Reply #15 on: 14 / November / 2010, 10:16:22 »
non-script badpixel   -> this would be very interesting to have in the next trunk

Logged
*

Offline reyalp

  • ******
  • 14079
Re: Making scripting support optional
« Reply #16 on: 14 / November / 2010, 19:58:22 »
ptp reboot appears to be broken with this patch, even if lua is enabled. I haven't looked in detail yet.

@asm1989
definitely

edit:
@ultimA can you explain what changed with auto_redraw / console_last_drawn ? Splitting the code into a different file and changing how it works makes it quite hard to follow.
« Last Edit: 14 / November / 2010, 20:21:20 by reyalp »
Logged
Don't forget what the H stands for.
*

Offline ultimA

  • ***
  • 137
Re: Making scripting support optional
« Reply #17 on: 14 / November / 2010, 22:04:24 »
I am going to split up the independent parts into separate patches, so I will be posting a new patchset soon. The scripting part of ptp is broken, I've realized that already. The probelm is that ptp.c is using a different entry point for script execution than kbd.c does, and so ptp.c does not start up an action stack required for the scripts. This should be fixed in my next patches. The problem is that I assumed scripts will always be started using the script_start function (was that really an unreasonable assumption?), but ptp.c just neglects that and calls lua_script_exec directly.  BTW, action stack, I liked that name, so I already renamed everything in code according to that.

ad. auto_redraw: The previous console code had this variable that could be set externally and if set to true, it resulted in automatically (1) clearing the screen and (2) redrawing the console with new contents upon adding a line to the console. I think this is not really useful, and now instead I always show the console whithin 3 seconds after adding new contents to it. The way this is done is setting console_last_drawn to the current get_tick_count upon redraw or when adding a new line, and then only drawing the console if three seconds have not yet elapsed (get_tick_count is asked while drawing). If someone wants to show the console even after 3 seconds have elapsed, it is possible to call redraw which will reinitialize console_last_drawn.
Logged
*

Offline reyalp

  • ******
  • 14079
Re: Making scripting support optional
« Reply #18 on: 14 / November / 2010, 22:41:55 »
Sounds good, thanks again for working on this. I checked in the text reader and enable PTP on sx20 changes.

regarding PTP, you want to make sure the script runs in the kbd task, not the PTP task.
Quote from: ultimA on 14 / November / 2010, 22:04:24
The problem is that I assumed scripts will always be started using the script_start function (was that really an unreasonable assumption?)
Making assumptions about CHDK code is usually not safe ;) But if we keep doing cleanup, it will be safer in the future!
Logged
Don't forget what the H stands for.
*

Offline ultimA

  • ***
  • 137
Re: Making scripting support optional
« Reply #19 on: 15 / November / 2010, 16:36:09 »
Here are the first three patches from a longer series. They include renames, decoupling the md from ubasic, and the refactored console. Unfortunately there is some minimal collision, I suggest applying the patches in this order, which will result in just a single hunk for script.c failing that is trivial to resolve by hand. To avoid any more complications, I will wait until these get accepted to the trunk before continuing work on the rest. When these are okay, the rest will follow, probably in the order of (1) action stack, (2) #ifdefs, (3) badpixel and the rest.

edit: please use console patch from a couple of posts further below instead of the one in this zip file.
« Last Edit: 20 / November / 2010, 20:13:16 by ultimA »
Logged
Pages: 1 [2] 3 4 5 6 7 ... 9   Go Up
« previous next »
 

Related Topics