Making kbd.c code generic - page 2 - General Discussion and Assistance - CHDK Forum supplierdeeply

Making kbd.c code generic

  • 51 Replies
  • 13257 Views
*

Offline reyalp

  • ******
  • 13291
Re: Making kbd.c code generic
« Reply #10 on: 23 / February / 2015, 23:27:19 »
Advertisements
This wasn't my invention, it's just how the ixus40 port was done. The firmware code uses system timers, and is quite different from anything that came later, so it's impossible to make it look/work like the usual keyboard code.
Yes, I think ixus30 and ixus40 are going to need pretty much stay this way and not use the common code, except maybe the press/release etc.

Quote
Quote
a470 has a weird hack involving the power button
+ same hack in ixus110, I'm the one who's responsible... Needed the extra button for CHDK.
Perhaps it can be replaced by something generic? Note that this is using an inverted button on the a470.
Thanks for pointing out the ixus110.

The trend seems to be toward less buttons so it might be nice to have generic, but in the meantime these can with KBD_CUSTOM_UPDATE_KEY_STATE

Having a handful of unique kbd.c files is still going to be a lot better than ~130.
Don't forget what the H stands for.

*

Offline reyalp

  • ******
  • 13291
Re: Making kbd.c code generic
« Reply #11 on: 28 / February / 2015, 19:49:45 »
I think this could start to be integrated in the trunk. If anyone thinks this isn't a good idea, or that major changes in the approach are needed, please let me know.

My approach would be:
For the initial checkin, all unconverted cameras would be set to use KBD_CUSTOM_ALL i.e. use their own kbd.c and ignore the current generic code. This would be temporary except for the ones (like ixus30) that need radically different kbd.c code.

Start converting the don't have any special weirdness blind, as well as cameras that have testers available. A list of which cameras haven't been tested should be kept.

Additional generic code for e.g. inverted key logic, the extra button hack, can be added later.


some other random notes:
* GetKbdState is found by the sig finder on many cams now, and can be used instead of reading MMIOs directly.
* KEY_DUMMY will be removed
* MALLOCD_STACK for vxworks kbd task was never used and can be removed
* Using C code in the vxworks naked mykbd_task is suspect
* DryOS on cams that start kbd_task directly can merge mykbd_task and mykbd_task_proceed (see D10 etc in patch)
* alt_mode_key_mask can probably be removed at some point
* Many kbd.c assume USB_IDX == SD_READONLY_IDX
Don't forget what the H stands for.

*

Offline nafraf

  • *****
  • 1308
Re: Making kbd.c code generic
« Reply #12 on: 28 / February / 2015, 21:33:50 »
kbd-common-work-4.patch tested on A495. Keys tested with ubtest.bas, keys works without problems.

*

Offline philmoz

  • *****
  • 3325
    • Photos
Re: Making kbd.c code generic
« Reply #13 on: 02 / March / 2015, 16:12:32 »
Quick test on the G12, no issues.

When I get a few minutes I'll do the IXUS310 touchscreen version.

Phil.
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)


*

Offline reyalp

  • ******
  • 13291
Re: Making kbd.c code generic
« Reply #14 on: 02 / March / 2015, 16:50:06 »
Quick test on the G12, no issues.

When I get a few minutes I'll do the IXUS310 touchscreen version.
Thanks.
Eventually, I'd like make more of the touch stuff common, but it doesn't have to happen in the first pass.

There are two common styles of using _GetKbdState (aka  kbd_fetch_data) and _kbd_read_keys_r2.

On cameras like d10:
kbd_fetch_data into new_state
run kbd_process (all chdk kbd task code)
copy into physw_status, fiddling key bits as required
_kbd_read_keys_r2, into physw_status
fiddle USB and SD read-only bits.

On cameras like sx160:
kbd_fetch_data into new_state
_kbd_read_keys_r2, into new_state
run kbd_process (all chdk kbd task code)
copy into physw_status, fiddling key bits as required
fiddle USB and SD read-only bits.
FWIW, I converted ixus140 and d10 to the same style as sx160, using the sigfinder _GetKbdState and saw no issues or change in behavior.

I see a comment in g12 which probably explains the origin of this variant.
Code: [Select]
//_kbd_read_keys_r2(physw_status); // re-reads physw_status[0] from 0x2DE4 at start (so above doesn't work properly) !!!!!
Don't forget what the H stands for.

*

Offline philmoz

  • *****
  • 3325
    • Photos
Re: Making kbd.c code generic
« Reply #15 on: 03 / March / 2015, 03:34:47 »
Updated patch with G12 and IXUS310 added.

I have added an extra short to KeyMap for the IXUS310 that I use to set the font scale factor - I use the symbols from the font for up/down/left/right instead of text; but scaled up to fill the button better.

I got this from waterwingz original N port; but this seems to have been removed in that version now.

Phil.
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)

*

Offline srsa_4c

  • ******
  • 4416
Re: Making kbd.c code generic
« Reply #16 on: 03 / March / 2015, 19:37:41 »
Thanks for pointing out the ixus110.

The trend seems to be toward less buttons so it might be nice to have generic, but in the meantime these can with KBD_CUSTOM_UPDATE_KEY_STATE
I managed to convert this one without KBD_CUSTOM_UPDATE_KEY_STATE (attached, I've been using the v4 patch). The "fake key" functionality is still working.

A side note: no matter what I try, the toolchain just doesn't want to emit correct code for mykbd_task(). Adding/removing the naked and noinline attributes makes no difference.
I know that this task is unlikely to ever exit and even if it did, ExitTask would probably not return, but it just isn't right.

edit: disregard the above, just checked, _ExitTask is declared as noreturn in lolevel.h
Code: [Select]
03b9b7cc <mykbd_task>:
 3b9b7cc: e59f3028 ldr r3, [pc, #40] ; 3b9b7fc <mykbd_task+0x30>
 3b9b7d0: e5933000 ldr r3, [r3]
 3b9b7d4: e3530000 cmp r3, #0
 3b9b7d8: 0a000006 beq 3b9b7f8 <mykbd_task+0x2c>
 3b9b7dc: e3a0000a mov r0, #10
 3b9b7e0: ebffb1a6 bl 3b87e80 <_SleepTask>
 3b9b7e4: ebfffff3 bl 3b9b7b8 <wrap_kbd_p1_f>
 3b9b7e8: e3500001 cmp r0, #1
 3b9b7ec: 1afffff6 bne 3b9b7cc <mykbd_task>
 3b9b7f0: ebffb1be bl 3b87ef0 <_kbd_p2_f>
 3b9b7f4: eafffff4 b 3b9b7cc <mykbd_task>
 3b9b7f8: ebffb161 bl 3b87d84 <_ExitTask>
 3b9b7fc: 00001c34 .word 0x00001c34
« Last Edit: 03 / March / 2015, 19:53:35 by srsa_4c »

*

Offline reyalp

  • ******
  • 13291
Re: Making kbd.c code generic
« Reply #17 on: 07 / March / 2015, 18:10:25 »
Thanks for both of those. I've attached an updated combined patch.

Regarding CAM_TOUCHSCREEN_SYMBOLS, all the touch cameras should probably be converted to use symbols, but this doesn't need to be done right away.

Regarding the ixus110, I have reservations about having a variable in the mask define, but again this can be changed later if needed.

Unless there are objections, I will start putting this in the trunk over the weekend.
Don't forget what the H stands for.


*

Offline reyalp

  • ******
  • 13291
Re: Making kbd.c code generic
« Reply #18 on: 08 / March / 2015, 17:42:29 »
I've checked in the initial code in trunk 4046

I have also started a wiki page for requests to test blind changes: http://chdk.wikia.com/wiki/Testing_Needed
I'll add a section for kbd.c once I start converting more cameras.
Don't forget what the H stands for.

Re: Making kbd.c code generic
« Reply #19 on: 11 / March / 2015, 20:37:00 »
A560 tested successfully.   Wiki page updated.
Ported :   A1200    SD940   G10    Powershot N    G16

 

Related Topics