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

waterwingz

12138

USB Remote Switch in CHDK - version 2 implementation thread

« on: 22 / November / 2011, 18:32:41 »

Advertisements

I've started this developer's thread to hopefully open a discussion about what should be done with the USB remote code in CHDK. This includes the core/kbd.c and platform/model/kbd.c files.

While the first question might be "Why?", one only has to spend a little time looking at the current code to answer that question. Its undocumented, complicated, buggy and in some places broken. Never mind it being difficult to configure from the menus. There is a lot of history behind the current code - some of it here in the forum & wiki and some lost with the passage of time.

This is probably as good as it gets for documentation : USB Remote Cable

I'm part way through the process of documenting the code as it exists now - just trying to figure out what its supposed to do. The next step is probably to clean things up by breaking it into logical functions, changing variable naming to reflect function and fix the parts that are broken. Right now there are also whole sections of code that can never be reached for example.

What to do next is a big question. For argument sake, if there is consensus on what to do, I'll take on the coding (with whatever help I can get). I think we can start the discussion by breaking things down as follows :

Current Capability

Hardware devices.
Built-in Script-less Remote functions.
Scripting support.

Hardware Devices
Current code supports two input devices :

a "hand made switch" that simply turns power to the USB port on & off. There are several sources for these or you can build your own.
the Ricoh CA-1 USB switch, widely avaible on Amazon or eBay. This device generates a specific series of pulses on the USB port power line

There are other devices on the market, for RC airplane, KAP and balloon flights, principally from gentles ( http://www.gentles.ltd.uk/ ). These are currently only supported via scripting.

Built-In Script-less Remote Functions
There are several different interrelated modes currently supported.

Two press mode - click once for "half press", click again for "full press". Currently implemented in "Alex scriptless remote code". Allows you to cancel a shot by not following the exact required timing. Should work with CA-1 but does not with my cameras
One press mode - apply power to USB port to "half press", remove power from USB port for "full press". Works in "syncable remote mode.
Sync functions - waits in a tight loop for USB power to go away and then fires the shutter. A programmable delay is also available. This is mostly useful when coordinating with another camera or device ( i.e. SDM).
Remote zoom - allows you to specify the zoom direction by pressing the right or left keys and then starts zooming on USB pulse. Mostly useful for coordinating zoom between two cameras ( i.e. SDM).

Scripting Support for USB Remote
Currently two scripting methods of determining the USB remote state.

wait_click 1 | is_key k "remote"
get_usb_power - returns pulse width of last USB pulse

Note : its not clear that the scriptless / sync code is currently fully disabled when a script is running.

Finally, here's what I propose should be done (if its agreed to even allow a rewrite) :

Provide built-in scriptless support for both "hand made switches" and the "Ricoh CA1".
Support "one press" and "two press" activation methods.
Other devices supported via scripting only.
Support Sync and Sync delay for "one press" mode. Disabled for two press and script modes.
Add scripting support to buffer up to 10 pulses widths - never miss a command.
Add scripting support for pulse counting - returns number of pulses since last called.
Delete built-in support for syncable zoom (defer to SDM for this).
Rework the Remote Parameters menu to reflect the changes

But that's just my opinion. Other comments ?

« Last Edit: 09 / February / 2012, 20:31:58 by waterwingz »

Logged

Ported : A1200 SD940 G10 Powershot N G16

reyalp

14035

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

« Reply #1 on: 22 / November / 2011, 21:24:05 »

Quote from: waterwingz on 22 / November / 2011, 18:32:41

But that's just my opinion. Other comments ?

Ms. Ripley [noembed]has some advice[/noembed]

Seriously though, this is an area that could definitely use some improvement.

I encourage current remote users to speak up about how they use this feature.

From a code perspective, it would be great to have the copy/paste stuff from each platform/kbd.c refactored into one common function with the any platform specific values being supplied by helper functions or #defines.

Logged

Don't forget what the H stands for.

waterwingz

12138

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

« Reply #2 on: 22 / November / 2011, 21:47:44 »

Quote from: reyalp on 22 / November / 2011, 21:24:05

From a code perspective, it would be great to have the copy/paste stuff from each platform/kbd.c refactored into one common function with the any platform specific values being supplied by helper functions or #defines.

That might be the worst part of attempting the project - the platform/kbd.c stuff. The brief look I took showed most ports to be almost identical - its just the wait_until_remote_button_is_released() routine that matters here.

It the "almost" part that is the killer - changing anything means checking and updating every camera model unless this can be done such that only the core/kbd.c is changed.

Maybe this wasn't such a good idea after-all.

Logged

Ported : A1200 SD940 G10 Powershot N G16

philmoz

3450

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

« Reply #3 on: 22 / November / 2011, 21:50:43 »

Quote from: reyalp on 22 / November / 2011, 21:24:05

Seriously though, this is an area that could definitely use some improvement.

That must count as the greatest understatement of all time

IMHO you're approach makes a lot of sense.
Decide how it should work, keeping it as simple as possible. Then throw away the current mess and build it properly.

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)

reyalp

14035

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

« Reply #4 on: 22 / November / 2011, 21:56:03 »

Quote from: waterwingz on 22 / November / 2011, 21:47:44

That might be the worst part of attempting the project - the platform/kbd.c stuff. The brief look I took showed most ports to be almost identical - its just the wait_until_remote_button_is_released() routine that matters here.

It the "almost" part that is the killer - changing anything means checking and updating every camera model unless this can be done such that only the core/kbd.c is changed.

I think it's safe to assume that most of those copy/pastes were never tested in the first place.

Logged

Don't forget what the H stands for.

waterwingz

12138

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

« Reply #5 on: 22 / November / 2011, 21:56:37 »

I think the biggest decision to be made would be dropping "Remote Zoom". It adds a lot of complexity. With that gone, the whole complicated state machine logic in core/kbd.c could pretty much go. Cleaning up platform/kbd.c becomes the hard task then.

Logged

Ported : A1200 SD940 G10 Powershot N G16

waterwingz

12138

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

« Reply #6 on: 22 / November / 2011, 21:57:33 »

Quote from: reyalp on 22 / November / 2011, 21:56:03

I think it's safe to assume that most of those copy/pastes were never tested in the first place.

Yup - that's where the unreachable sections of code reside.

Logged

Ported : A1200 SD940 G10 Powershot N G16

msl

1280
A720 IS, SX220 HS 1.01a

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

« Reply #7 on: 23 / November / 2011, 08:45:57 »

Quote from: waterwingz on 22 / November / 2011, 18:32:41

Finally, here's what I propose should be done (if its agreed to even allow a rewrite) :

Provide built-in scriptless support for both "hand made switches" and the "Ricoh CA1".
Support "one press" and "two press" activation methods.
Other devices supported via scripting only.
Support Sync and Sync delay for "one press" mode. Disabled for two press and script modes.
Add scripting support to buffer up to 10 pulses widths - never miss a command.
Add scripting support for pulse counting - returns number of pulses since last called.
Delete built-in support for syncable zoom (defer to SDM for this).
Rework the Remote Parameters menu to reflect the changes

But that's just my opinion. Other comments ?

Those are all good suggestions.

1. Agreed

2. Perhaps via a menu selection [Off, oneclick, doubleclick]

3. Agreed

4. All synch function should be only as an compile option in a extra submenu. These functions are the biggest misunderstanding of CHDK.

5. & 6. Agreed

7. See 4.

8.The menu entry for remote control must be more in the foreground. See also 2.

msl

Logged

CHDK-DE: CHDK-DE links

waterwingz

12138

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

« Reply #8 on: 23 / November / 2011, 09:18:25 »

@msl - thanks for sharing your thoughts. I was a little afraid my original post would get a tl;dr response which would have pretty much discouraged me from taking on this project at all. Philmoz & reyalp have commented - it would be nice to hear from "regular users" too but that might be asking too much.

Quote from: msl on 23 / November / 2011, 08:45:57

2. Perhaps via a menu selection [Off, oneclick, doubleclick]

Yes - a clear menu that give you the choices for that and for the hardware device is important.

Right now in the sync modes the code chooses between "hand made switch" and "Ricoh CA-1" automatically. It measures pulse widths and assumes short pulses (less that about 100mSec) can only be made by a CA-1 so it switches how it handles the hardware based on that assumption. The problem with that is obvious I think.

In "Alex scriptless remote" mode (i.e. remote enabled but not sync enabled), it just assumes a "hand made switch" - there is nothing to handle the CA-1's pulses so you have to pound on the CA-1's button several times to trick it into taking a shot in that mode.

Quote

4. All synch function should be only as an compile option in a extra submenu.

So the default autobuild would not include sync options ? Or is putting them in a submenu enough ?

Quote

8.The menu entry for remote control must be more in the foreground. See also 2.

I've always thought so too. And I never understood why it was cross linked from the scripting menu.

Which brings us to yet more issues or questions. These changes would require documentation updates. Managable but hard to get everything I suspect. Can we live with that ?

Also, hard coded thing like set_config_value(121, 1) would break. Again, something we have seen before I guess.

Would the CHDK-DE version track the changes ? Or would this move us farther apart ?

Logged

Ported : A1200 SD940 G10 Powershot N G16

msl

1280
A720 IS, SX220 HS 1.01a

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

« Reply #9 on: 23 / November / 2011, 11:39:45 »

Quote from: waterwingz on 23 / November / 2011, 09:18:25

Quote
4. All synch function should be only as an compile option in a extra submenu.
So the default autobuild would not include sync options ? Or is putting them in a submenu enough ?

I don't know how many people use the synch functions and have a CA-1. But this is more a SDM feature. Most newbies have problems with their first self-built remote control, because they do not understand the synch functions, e.g. a delayed reaction with active synchro or trying zoom functions. I think it would be better to offer these features only optional, for the autobuild optional off.

Quote from: waterwingz on 23 / November / 2011, 09:18:25

Which brings us to yet more issues or questions. These changes would require documentation updates. Managable but hard to get everything I suspect. Can we live with that ?

Yes, I see there no problems. The improvements would be important enough. Other improvements like the newest menu changes needs also documentation updates and nobody asks.

Quote from: waterwingz on 23 / November / 2011, 09:18:25

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.

Quote from: waterwingz on 23 / November / 2011, 09:18:25

Would the CHDK-DE version track the changes ? Or would this move us farther apart ?

No, the opposite would be true. For me it is important that we have an uniform camera-dependent part. The core can be different. platform/ixus100/kbd.c is a good example where we need consistent improvements.

Quote from: waterwingz on 23 / November / 2011, 09:18:25

I've always thought so too. And I never understood why it was cross linked from the scripting menu.

This is a historical thing. In the early years of CHDK there was no scriptless remote control only via script.

msl

Logged

CHDK-DE: CHDK-DE links