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

reyalp

13878

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #60 on: 16 / January / 2012, 00:53:03 »

Advertisements

Quote from: philmoz on 16 / January / 2012, 00:24:22

CHDK should only ever modify bits in physw_status that are defined in the KEYS_MASKx values.
Initialising extra bits in the CHDK variables outside what is defined in KEYS_MASKx should have no effect on the firmware (unless KEYS_MASKx are wrong).

That's how I thought it was supposed to work, but I wasn't sure I could square it with the reported effects.

Quote

The problem is in what CHDK does with the kbd state variables and stems from the fact that the state variables are active low. If you don't initialise these they start out as all 0 until the first call to kbd_key_release_all (which also looks dodgy on the A495). There are cases when CHDK calls the other kbd functions (kbd_key_press, etc) first and so gets phantom key presses.

If this is correct, then wouldn't just calling kbd_key_release_all somewhere early (when kbd_task starts for example) should solve the problem ? I was confused by the fact that kbd_new_state and kbd_prev_state were also set as part of this fix.

If you are confident that it is OK as is (and a495 problem is just bugs in the port), I'm fine checking in the current patch. There still isn't a trunk autobuild, so exposure for end users should be fairly low anyway.

Logged

Don't forget what the H stands for.

philmoz

3445

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #61 on: 16 / January / 2012, 01:06:39 »

Quote from: reyalp on 16 / January / 2012, 00:53:03

Quote from: philmoz on 16 / January / 2012, 00:24:22
CHDK should only ever modify bits in physw_status that are defined in the KEYS_MASKx values.
Initialising extra bits in the CHDK variables outside what is defined in KEYS_MASKx should have no effect on the firmware (unless KEYS_MASKx are wrong).
That's how I thought it was supposed to work, but I wasn't sure I could square it with the reported effects.
Quote
The problem is in what CHDK does with the kbd state variables and stems from the fact that the state variables are active low. If you don't initialise these they start out as all 0 until the first call to kbd_key_release_all (which also looks dodgy on the A495). There are cases when CHDK calls the other kbd functions (kbd_key_press, etc) first and so gets phantom key presses.
If this is correct, then wouldn't just calling kbd_key_release_all somewhere early (when kbd_task starts for example) should solve the problem ? I was confused by the fact that kbd_new_state and kbd_prev_state were also set as part of this fix.

If you are confident that it is OK as is (and a495 problem is just bugs in the port), I'm fine checking in the current patch. There still isn't a trunk autobuild, so exposure for end users should be fairly low anyway.

Since they all have the same active low logic it makes sense to me to intialise everything to the off state in all three arrays. Otherwise it takes a call to kbd_key_release_all and two calls to my_kbd_read_keys before every used bit of off/high (assuming no button presses in the meantime).

As to where they are initialised, why do something in the run-time code that the compiler can do for you?

I would be pretty confident that the current method in the patch will not cause any problems unless the port itself is buggy - and I'd rather fix any of these as they arise than try and code something to work around the bugs.

Now we just need to find someone with an A490 / A495 to do some testing

Phil.

Update: I stand corrected. It seems the A490, A495 & A800 do actually have inverted logic in the physw_status variables for some keys. The solution proposed by srsc_4a is quite elegant - inverting the bits as they copied to/from the physy_status variables in to the CHDK variables.

« Last Edit: 22 / January / 2012, 22:28:32 by philmoz »

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)

reyalp

13878

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #62 on: 16 / January / 2012, 01:55:07 »

OK, I've checked this in. I updated sx40 to use the SD_READONLY_IDX (untested, obviously)

I did not update the ixus230.

Logged

Don't forget what the H stands for.

Microfunguy

4037

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #63 on: 16 / January / 2012, 08:20:18 »

Be aware that enabling remote switch functionality is not the same as precision synch.

For example, the remote switch works with the SX230HS but the synch is extremely poor .. unusually bad in fact.

Logged

waterwingz

12132

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #64 on: 16 / January / 2012, 09:25:47 »

Quote from: Microfunguy on 16 / January / 2012, 08:20:18

Be aware that enabling remote switch functionality is not the same as precision synch.

I think that I documented that correctly ? : Multicamera Syncronization

Quote

For example, the remote switch works with the SX230HS but the synch is extremely poor .. unusually bad in fact.

This is not surprising. The release-1_0 CHDK code in the autobuild has (and has always had) a very broken wait_until_remote_button_is_released() function. Bugs vary with different cameras too. I have a version that has the various bugs fixed but iin talking to the devs, its not clear that we want to make the change to the stable v1.0 autobuild.

Logged

Ported : A1200 SD940 G10 Powershot N G16

Microfunguy

4037

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #65 on: 16 / January / 2012, 09:32:25 »

Quote from: waterwingz on 16 / January / 2012, 09:25:47

This is not surprising. The release-1_0 CHDK code in the autobuild has (and has always had) a very broken wait_until_remote_button_is_released() function.

I have ported the SX230HS to SDM and included a very minimal wait_until_remote_button_is_released() function.

I know the switch release is detected with just 0.5 msec difference between two cameras.
I have also 'dug-down' further into the code to insert that function as late as possible.
The result is always the same.

All 'special' camera functions are disabled.

Difficult to believe the firmware has much more to do at that point.

The SX130is code looks similar, its synch is probably poor.

Logged

waterwingz

12132

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #66 on: 16 / January / 2012, 11:10:28 »

Quote from: Microfunguy on 16 / January / 2012, 09:32:25

The SX130is code looks similar, its synch is probably poor.

In your experience, how close does the actual exposure have to be sync'd ? What applications need really precise sync ? I assume its things that are moving quickly enough that the difference in sync distorts the 3D effect ?

Logged

Ported : A1200 SD940 G10 Powershot N G16

Microfunguy

4037

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #67 on: 16 / January / 2012, 11:22:13 »

Quote from: waterwingz on 16 / January / 2012, 11:10:28

how close does the actual exposure have to be sync'd ?

fraction of a millisecond.

Quote

What applications need really precise sync ?

Any multi-camera application (stereo, bullet sequences) that involves 'moderate' movement.

That can be people, animals, vehicles, water, foliage, etc.

The same degree of synch is required for movies.

Not a problem as synchronised movies are not supported and never will be with current hardware design.

Pity, one of the great unsolved problems of amateur stereo video (unless GoPro's are acceptable).

Logged

srsa_4c

4451

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #68 on: 16 / January / 2012, 22:39:56 »

[OFFTOPIC]
An update for the problem with the A495: the false keypress(es) seem to be the ones that are handled "special": up/down/left/menu (link in my related previous post).
[/OFFTOPIC]

Logged

philmoz

3445

Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code

« Reply #69 on: 17 / January / 2012, 00:18:28 »

Quote from: srsa_4c on 16 / January / 2012, 22:39:56

[OFFTOPIC]
An update for the problem with the A495: the false keypress(es) seem to be the ones that are handled "special": up/down/left/menu (link in my related previous post).
[/OFFTOPIC]

Try removing the special case code for these keys in the 'kbd_is_key_pressed' function in kbd.c.
My guess is that this is what is causing the problem after changing the initialisation code - if you change the kbd_is_key_pressed code to match other ports I suspect it will sort out the problem.

Phil.

Logged