USB Remote Switch in CHDK - version 2 implementation thread - page 2 - General Discussion and Assistance - CHDK Forum

USB Remote Switch in CHDK - version 2 implementation thread

  • 220 Replies
  • 73446 Views
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #10 on: 23 / November / 2011, 12:10:24 »
Advertisements
Also, hard coded thing like set_config_value(121, 1) would break.  Again, something we have seen before I guess.
I don't know a published script that uses specially this ID. The ID remains the the same, only gets more arguments.
That particular function lets you set / clear a menu item selection - in the case of 121 its currently used to enable / disable the USB Remote option from a script.

So if we redo the menu for USB Remote,  we should leave the current menu numbers unassigned or reallocate them ? 
Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline msl

  • *****
  • 1280
  • A720 IS, SX220 HS 1.01a
    • CHDK-DE links
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #11 on: 23 / November / 2011, 13:18:23 »
That particular function lets you set / clear a menu item selection - in the case of 121 its currently used to enable / disable the USB Remote option from a script. ...
Yes, I know. But this is only the configuration ID in conf.c and stands for conf.remote_enable. It does not matter whether there are 2, 3 or more states. The ID remains. The menu entry remains, wherever.

msl
CHDK-DE:  CHDK-DE links

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #12 on: 23 / November / 2011, 13:27:20 »
The ID remains. The menu entry remains, wherever.
So if we have a new USB Remote Menu,  we create new ID's ?  The old one - 121 is not re-assigned ? It just becomes like a zombie ID # - never used in future ?
Ported :   A1200    SD940   G10    Powershot N    G16

Re: USB Remote Switch in CHDK - wait_until_remote_button_is_released()
« Reply #13 on: 26 / November / 2011, 15:18:37 »
I've extracted the wait_until_remote_button_is_released() from every platform/kbd.c file.

With a little perl scripting and some keyboarding, I compared each by deleting whitespace, comments and formatting.  The code seems to break down into four groups - each group implementing slightly different  logic from the rest.  There are also small differences between most files that I've documented but ignored - unused long x[3]; in many files,  the use of debug_led(); for example.


= Group 1 ======= reference G7  ======================================
 
  a430  a450  a460  a470   a490  a495  a590  a550  a560  a570 
  a580  a650  a720  a3000  d10  g7  g9  g11  ixus100_sd780 
  ixus70_sd1000 ixus90_sd790  ixus970_sd890  ixus850_sd800 
  ixus1000_sd4500  ixus300_sd4000  ixus900_sd900  ixus860_sd870
  ixus85_sd770 ixus95_sd1200  ixus980_sd990 s90  s95  sx130is   
  sx220hs  sx230hs   tx1*   
 
    * uses ((usb_physw[2] & USB_MASK)==USB_MASK) vs (usb_physw[2]&USB_MASK)

= Group 2 ===== missing (shooting_get_drive_mode()==1) =================

  ixus120_sd940 ixus870_sd880 ixus950_sd850  ixus960_sd950
  a2000 ixus200_sd980  ixus310_elph500hs  s5is  sx10  sx1 
  sx200is  sx20 g12 sx30 g10 ixus870_sd880 sx110is
  a1100** a480**

      ** adds calls to _platformsub_kbd_fetch_data(x);

= Group 3 ===== comments out all code - just pushes & pops ==============

  s2is s3is   

= Group 4 === old cameras using x=get_mmio() ================================

  generic (includes a410 a530 a540 a610 a620 a630
  a640 a700 a710 ixus800_sd700)
  ixus55_sd450_kbd



Ignoring group 3 & 4 for now,  the code in groups 1 & 2 both have sections that can never be executed.  Group one has one zombie section while group 2 has two.  This needs to be fixed as part of this USB Remote Switch cleanup effort.

Here's the reference code (cut from the G7 but common to all)

Code: [Select]
1   if (    conf.synch_enable
2      && conf.ricoh_ca1_mode
3      && conf.remote_enable
4      && (   !shooting_get_drive_mode()
5           || (shooting_get_drive_mode()==1)
6           || (  (shooting_get_drive_mode()==2)
7                  &&  state_shooting_progress != SHOOTING_PROGRESS_PROCESSING ) ))
8  {
9        nMode=0; usb_physw[2] = 0;   _kbd_read_keys_r2(usb_physw);
10       if((usb_physw[2] & USB_MASK)==USB_MASK) nMode=1;
11
12        if( conf.ricoh_ca1_mode && conf.remote_enable )
13       {
14            if(shooting_get_drive_mode()==1 && state_shooting_progress == SHOOTING_PROGRESS_PROCESSING)
15           {

Notice that it the conditional on line 12 will always evaluate at TRUE due to lines 2 & 3 needing to be true before line 12 can be reached.  The "else" condition of line 12 is never executed.

In addition,  group 2 camera do not have line 5 and so line 14 can never evaluate as TRUE.

Next Steps in Clean-up Plan
  • Reach consensis on how this logic is supposed to work.
  • Decide if we want debug_led() code embedded (tells user that camera is waiting for shooting to complete but interfers with other debug_led() usage)
  • Delete unused long x[3] from all version ( easy decision ).
  • Do we then put the resulting two versions of wait_until_remote_button_is_released() in generic.c  (old cameras version and new camera version) and delete from all platform/kbd.c files ?

Sorry for another tl;dr post - hard to get this info out in a shorter format.
Ported :   A1200    SD940   G10    Powershot N    G16


Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #14 on: 26 / November / 2011, 16:48:06 »
Extracted  wait_until_remote_button_is_released()  code with comments - a work in progress.

wait_until_remote_button_is_released() is called every time a picture is taken and CHDK is loaded in the camera, regardless of the state of USB Remote mode. That means that for camera models with debug_led() in the code,  the LED is blinked every time.  It also means that the need for sync delay is evaluated every time.

The function of wait_until_remote_button_is_released()  is to delay the capt_seq task when USB remote sync is enabled to ensure that shooting does not happen until exactly when the USB remote releases the 5V signal.   This sequence can be extended to support sync_delay - allowing a precise additional delay to allow adjustment between different cameras.   The routine handles different shooting drive modes to allow for precise synchronization of bracketing exposures.

Note : this routine has no effect if the USB remote is operating in "Alex Scriptless Remote" mode.


« Last Edit: 27 / November / 2011, 13:51:53 by waterwingz »
Ported :   A1200    SD940   G10    Powershot N    G16

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #15 on: 27 / November / 2011, 12:01:18 »
Looks like the zombie code mentioned above has been with us for a while.

wait_until_remote_button_is_released() was added to the build when the juciphox branch was merged back in at r511.   The bugs creep into the juiciphox build at r474 ( kdb.c was at r463 ).

Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline vnd

  • *
  • 36
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #16 on: 04 / December / 2011, 12:18:11 »
I have spent the weekend with debugging the remote trigger code, first in SDM, then in CHDK. Then I have been pointed to this thread.

My current impression is that the CHDK is more broken but with higher chance for fixing than SDM ;-)

Some problems not yet mentioned here:

- Ricoh CA-1 mode was apparently never working in CHDK. I have put the fix is here:
http://chdk.setepontos.com/index.php?topic=650.msg77324#msg77324

- CA-1 has problems even in SDM, at certain times the code ignores the pulses:
http://tech.groups.yahoo.com/group/StereoDataMaker/message/12009

- the synch_delay code is compiler dependent:
http://chdk.setepontos.com/index.php?topic=6824.msg77325#msg77325

If the code is going to be rewritten, I'd suggest implementing a low-level "driver" for each remote, with these functions:

get_remote_state()  - called periodically from kbd_process(), return one of HALF-PRESS, FULL_PRESS, RELEASE
wait_for_synch() - optional function for high-precision synchronization, called from wait_until_remote_button_is_released()

The code in kbd_process() should not query the usb directly, but use this interface.

*

Offline vnd

  • *
  • 36
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #17 on: 06 / December / 2011, 03:26:42 »
BTW, can I help with anything?


Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #18 on: 06 / December / 2011, 06:33:25 »
BTW, can I help with anything?
Having another pair of eyes interested in that particular section of the code would be a huge help.   My biggest slowdown so far is trying to understand what the original authors were trying to do. Not sure the best way to compare notes on that ?  This forum,  PM,  email or IRC all seem like viable options if you are interested.  I could send you my working copies of my comments onm the  code - although the danger there is that were my comments are wrong it could be misleading.


Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline vnd

  • *
  • 36
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #19 on: 10 / December / 2011, 06:55:14 »
I have sent some code comments to waterwingz.

In the attachment are some of my thoughts on how the synchable remote currently works and what are the problems with CA-1.

 

Related Topics