Valid <ALT> key test at startup

lapser

1093

Valid <ALT> key test at startup

« on: 07 / September / 2013, 11:10:50 »

Advertisements

When testing nafraf's patch to remove the KEY_PRINT to KEY_PLAYBACK alias on the SX260 and D20, I discovered that the test build started with no valid <ALT> key defined. Thus, I couldn't enter <ALT> mode. Deleting the .cfg files didn't help, since the default <ALT> key is still KEY_PRINT. The only way I could get it to work was to set the <ALT> key to "video" on the old build first.

The solution is to check the <ALT> key in conf.c on startup. It looks like the best place to do this is in clear_values(). This is what I added at the end of clear_values()

Code: [Select]

//make sure <ALT> key is a valid option
#if defined(CAM_ALT_BUTTON_OPTIONS)
    int keys[] = CAM_ALT_BUTTON_OPTIONS;
    int i=0;
    while((i < sizeof(keys)/sizeof(keys[0]))&&(conf.alt_mode_button != keys[i]))i++;
    if(i == sizeof(keys)/sizeof(keys[0]))
    {
      conf.alt_mode_button=keys[0];
      kbd_set_alt_mode_key_mask(conf.alt_mode_button);
    }
#endif

"sizeof(keys)/sizeof(keys[0])" is the crazy way you find the number of elements in an array in C. I assume the compiler evaluates it to a constant. Is there a better way to do this?

Testing "i" first in the "while" statement should ensure that the "keys[]" array index stays in bounds. Doing it this way saves adding a "break" and second "if" test. Every byte and microsecond counts!

If CAM_ALT_BUTTON_OPTIONS isn't defined, then it looks like you're stuck with KEY_PRINT as the <ALT> key. All of my cameras have the alt button options, so I'm not sure how to deal with it, or if it needs to be dealt with.

I tested nafraf's patch (attached) on the d20 and sx260, and both cameras were left without an <ALT> key. Both worked normally, using KEY_PLAYBACK as the <ALT> key, with the above changes. I'll attach nafraf's patch, and a patch file for my changes.

d20_sx260_delete_playback_alias_trunk-3070.patch (3.32 kB - downloaded 30 times.)

valid_alt_button_T3081.patch (0.87 kB - downloaded 32 times.)

Logged

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

philmoz

3450

Re: Valid <ALT> key test at startup

« Reply #1 on: 07 / September / 2013, 20:37:36 »

Simpler solution:
- add "#define DEFAULT_ALT_KEY KEY_PRINT" to camera.h (with suitable comments)
- replace KEY_PRINT with DEFAULT_ALT_KEY in conf.c (two locations)
- override DEFAULT_ALT_KEY in platform_camera.h for affected cameras.

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

14079

Re: Valid <ALT> key test at startup

« Reply #2 on: 08 / September / 2013, 01:31:22 »

Quote from: philmoz on 07 / September / 2013, 20:37:36

Simpler solution:
- add "#define DEFAULT_ALT_KEY KEY_PRINT" to camera.h (with suitable comments)
- replace KEY_PRINT with DEFAULT_ALT_KEY in conf.c (two locations)
- override DEFAULT_ALT_KEY in platform_camera.h for affected cameras.

This would work fine for new ports, but for any existing port that previously had the alt=PLAYBACK=PRINT style hack, everyone who had it in their cfg would have delete their old cfg (or use set_config_value), since they wouldn't be able to get into the alt menu.

Making sure the assigned alt key is one of the cameras valid alt keys seems like a good sanity check.

Logged

Don't forget what the H stands for.

philmoz

3450

Re: Valid <ALT> key test at startup

« Reply #3 on: 08 / September / 2013, 03:30:42 »

Quote from: reyalp on 08 / September / 2013, 01:31:22

Making sure the assigned alt key is one of the cameras valid alt keys seems like a good sanity check.

Agreed.
Can be done in clear_values with the following code:

Code: [Select]

#if CAM_ADJUSTABLE_ALT_BUTTON
    gui_alt_mode_button_enum(0,0);
#else
    conf.alt_mode_button = DEFAULT_ALT_KEY;
#endif

This will also handle the cases where DEFAULT_ALT_KEY is changed for cameras that do not have CAM_ADJUSTABLE_ALT_BUTTON defined, or if the SD card is used in another camera.

The 'gui_alt_mode_button' function already checks if conf.alt_mode_button is valid and resets it if it is not. There's no need to add the same code again in clear_values.

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)

lapser

1093

Re: Valid <ALT> key test at startup

« Reply #4 on: 09 / September / 2013, 18:21:43 »

Quote from: philmoz on 08 / September / 2013, 03:30:42

Can be done in clear_values with the following code:

This will also handle the cases where DEFAULT_ALT_KEY is changed for cameras that do not have CAM_ADJUSTABLE_ALT_BUTTON defined, or if the SD card is used in another camera.

Great job as usual, Phil.

From some of the comments in the code, it looks like you might be able to take the .cfg file from one camera and use it in a different camera? Is this what you meant about the SD card being used in another camera? I assume that would be after updating the CHDK build to the new camera.

I have 4 different cameras, so I wonder if I could set up one camera and copy the .cfg files onto the other 3?

Logged

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

reyalp

14079

Re: Valid <ALT> key test at startup

« Reply #5 on: 09 / September / 2013, 21:34:53 »

Quote from: lapser on 09 / September / 2013, 18:21:43

From some of the comments in the code, it looks like you might be able to take the .cfg file from one camera and use it in a different camera? Is this what you meant about the SD card being used in another camera? I assume that would be after updating the CHDK build to the new camera.

I have 4 different cameras, so I wonder if I could set up one camera and copy the .cfg files onto the other 3?

Yes, a few things that are platform specific will get reset, but it should be OK. The osd colors may be off.

Logged

Don't forget what the H stands for.

reyalp

14079

Re: Valid <ALT> key test at startup

« Reply #6 on: 15 / September / 2013, 03:38:34 »

Quote from: philmoz on 08 / September / 2013, 03:30:42

Agreed.
Can be done in clear_values with the following code:
Code: [Select]
#if CAM_ADJUSTABLE_ALT_BUTTON gui_alt_mode_button_enum(0,0); #else conf.alt_mode_button = DEFAULT_ALT_KEY; #endif

I checked this in to the trunk.

Logged

Don't forget what the H stands for.

reyalp

14079

Re: Valid <ALT> key test at startup

« Reply #7 on: 15 / September / 2013, 15:53:41 »

I merged this over to the release branch.

From a compatibility point of view, I think the cameras that already have a KEY_PRINT alias should probably stay that way in 1.2. Some users might expect to be able to send "print" to get video or playback etc. I merged it over mainly to ensure users get a valid alt button. New ports don't need to implement it.

When 1.3 comes out, there are likely to be substantial changes to key handling anyway, so removing the print alias there is fine and the potential incompatibility can be noted in the release notes.

Logged

Don't forget what the H stands for.