Making kbd.c code generic - page 4 - General Discussion and Assistance - CHDK Forum supplierdeeply
« previous next »
Pages: 1 2 3 [4] 5 6   Go Down

Making kbd.c code generic

  • 51 Replies
  • 20175 Views
*

Offline msl

  • *****
  • 1280
  • A720 IS, SX220 HS 1.01a
    • CHDK-DE links
Re: Making kbd.c code generic
« Reply #30 on: 20 / March / 2015, 07:45:46 »
Advertisements
SX220 works fine.

- Cam boots (short press => play mode, long press => record mode) OK
- All native buttons & jogdial function OK.
- Switch to rec mode and back OK
- CHDK menu navigation and shortcuts OK
- Long and short press (native and CHDK function) OK
- Scripted key presses & jogdial  OK
- USB remote OK
- optional battery door open hack OK

msl
Logged
CHDK-DE:  CHDK-DE links
*

Offline reyalp

  • ******
  • 14080
Re: Making kbd.c code generic
« Reply #31 on: 20 / March / 2015, 17:23:45 »
Moving ixus120_sd940 discussion from the patch thread:
Quote from: waterwingz on 20 / March / 2015, 08:16:46
My patch did not do that so I assume you made that choice?
Your patch removed it, I only added the code to turn the rest off if the OPT_ define isn't set.
Code: [Select]
- kbd_new_state[2] |=0x00008000;  /// disable the battery door switch
The equivalent common code only masks the battery door if the OPT_ is defined.

You could include the above in your implementation of  my_kbd_read_keys after kbd_update_physw_bits (remember to take out the ifdefs I added in the code-gen files if you do that)

Personally, I would prefer to have behavior consistent across ports where it isn't dictated by some quirk of the actual camera.

I also prefer to have the default stay as close to the Canon behavior as possible.

Quote
Not sure how to handle this (or how many other cameras could use this)? Build as a custom kbd.c or modify the standard my_kbd_read_keys() to allow this as an option?
You could do this with KBD_CUSTOM_UPDATE_KEY_STATE, implementing all of kbd_update_key_state in kbd.c. Since this code only exists for one camera AFAIK, I'm not sure if this is worth adding as general ifdef.

With the alt button now standardized on PLAY, maybe this doesn't matter so much?

OTOH, there are other cases where the inability to send a long press to the canon firmware is a drawback, this affects D10 and some other cameras that can assign functions to the print button. So if you can figure out a general way of doing it, it would be useful to have in the common code.
Logged
Don't forget what the H stands for.
*

Offline waterwingz

  • ******
  • 12138
    • Waterwingz
Re: Making kbd.c code generic
« Reply #32 on: 20 / March / 2015, 17:42:36 »
Quote from: reyalp on 20 / March / 2015, 17:23:45
Personally, I would prefer to have behavior consistent across ports where it isn't dictated by some quirk of the actual camera. I also prefer to have the default stay as close to the Canon behavior as possible.
I can live with that.   If only because it seems unlikely that we are going to attract a scripting fanatic anytime soon who will be using an ixus120 as his/her camera of choice  :P

Quote
You could do this with KBD_CUSTOM_UPDATE_KEY_STATE, implementing all of kbd_update_key_state in kbd.c. Since this code only exists for one camera AFAIK, I'm not sure if this is worth adding as general ifdef. With the alt button now standardized on PLAY, maybe this doesn't matter so much?  OTOH, there are other cases where the inability to send a long press to the canon firmware is a drawback, this affects D10 and some other cameras that can assign functions to the print button. So if you can figure out a general way of doing it, it would be useful to have in the common code.
Found the original forum reference to this : http://chdk.setepontos.com/index.php?topic=5855.msg58646#msg58646. I thought I got the idea from another camera but apparently not.  I assume the ixus100_sd780 could use the same patch and maybe a lot of the other "button limited" ixus cameras. 

For now, I guess this is not very high on my "roundtuit" list - probably just something to remember if the generailze "long press" button press becomes relevant again.
Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline reyalp

  • ******
  • 14080
Re: Making kbd.c code generic
« Reply #33 on: 21 / March / 2015, 15:52:24 »
I finished the first pass through all the platforms last night.  Over 100 ports are converted, and ~30 have been tested.

Thanks to everyone for testing. While I'm sure a few ports will end up broken in this process, the low number of failures so far gives me hope that it won't be too many. I've also fixed a few bugs in the process, so it may be a net gain ;)

There are ~20 cams I skipped over because they were "weird" in some way that I will revisit next.

The majority of these use a canon firmware function (_GetKbdState equivalent) to read directly into physw_status rather than reading into kbd_new_state and setting the modified values in physw_status later. I think most of these can be converted to use the normal code, but I need to look at the disassembly to be sure. It would be helpful to have some examples tested, so please let me know if you have one of these.

These are
A2000
A3100
A490 (also has inverted key logic)
A800 (also has inverted key logic)
IXUS1000_SD4500
IXUS300_SD4000
IXUS60_SD600 (also has feather code, but manual + google suggests it doesn't have hardware)
IXUS65_SD630 (also has "touch dial" per manual)
IXUS900_SD900 (also has "touch dial" per manual)
IXUSIZOOM_SD30 (has other weirdness)
S2IS (has other weirdness)
S5IS (has comments about regular key input code not being found)
SX100IS
SX110IS
SX120IS
SX210IS

There are also some old vxworks cams like which operate differently from both modern cams and the ixus30/ixus40
IXUS50
IXUS700_SD500
S2IS

There are also some with a "feather" or "touch control dial", as well as some that have this code, but (by looking the manual) do not appear to have the hardware. I don't have any of these cameras and I'm not entirely sure how the related CHDK code is supposed to work. The cameras with CAM_FEATURE_FEATHER defined are
IXUS110_SD960
IXUS300_SD4000
IXUS60_SD600 (but as noted above, I don't think it really has the hardware)
IXUS65_SD630
IXUS900_SD900

I also skipped over G9 since nafraf is has been working on other areas of the port.
Logged
Don't forget what the H stands for.
*

Offline waterwingz

  • ******
  • 12138
    • Waterwingz
Re: Making kbd.c code generic
« Reply #34 on: 21 / March / 2015, 16:07:52 »
Quote from: reyalp on 21 / March / 2015, 15:52:24
I finished the first pass through all the platforms last night.  Over 100 ports are converted, and ~30 have been tested.
Nice!

Quote
The majority of these use a canon firmware function (_GetKbdState equivalent) to read directly into physw_status rather than reading into kbd_new_state and setting the modified values in physw_status later. I think most of these can be converted to use the normal code, but I need to look at the disassembly to be sure. It would be helpful to have some examples tested, so please let me know if you have one of these.
I'd bet my next pay cheque that almost all of these are C&P'd from whatever the porter was using for a prototype rather than something that was tested and found to need to be that way.  If checking all the dissassemblies proves to be a big job, converting them blind and fixing any complaints later might be an option?
Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline c_joerg

  • *****
  • 1248
Re: Making kbd.c code generic
« Reply #35 on: 21 / March / 2015, 16:36:50 »
S110 and G1X works fine.

- Camera boots  OK
- All native buttons and jog dial function OK.
- Switch to record mode and back OK
- CHDK menu navigation and shortcuts OK
- Scripted key presses and jog dial  OK

I have not tested USB remote…
Logged
M100 100a, M3 121a, G9x II (1.00c), 2*G1x (101a,100e), S110 (103a), SX50 (100c), SX230 (101a), S45,
Flickr https://www.flickr.com/photos/136329431@N06/albums
YouTube https://www.youtube.com/channel/UCrTH0tHy9OYTVDzWIvXEMlw/videos?shelf_id=0&view=0&sort=dd
*

Offline srsa_4c

  • ******
  • 4451
Re: Making kbd.c code generic
« Reply #36 on: 21 / March / 2015, 17:06:29 »
Quote from: reyalp on 21 / March / 2015, 15:52:24
It would be helpful to have some examples tested, so please let me know if you have one of these.
I have these:
- IXUS65_SD630, it does have the "feather"
- SX100IS.
The IXUS110_SD960 also has the "feather".
Logged
*

Offline reyalp

  • ******
  • 14080
Re: Making kbd.c code generic
« Reply #37 on: 21 / March / 2015, 18:36:43 »
I started on these:

A2000, A3100 have kbd_pwr_off functions that weren't found correctly by finsig. I've converted them to use functions to use manually found functions. I noticed some of the other A series cams only called pwr_on even though there appeared to be a pwr_off equivalent in the firmware.

SX100 doesn't have pwr_on/off equivalents, but GetKbdState is more than just simple MMIO reads. I converted it to use GetKbdState/read_keys_r2.
Logged
Don't forget what the H stands for.
*

Offline reyalp

  • ******
  • 14080
Re: Making kbd.c code generic
« Reply #38 on: 21 / March / 2015, 18:56:44 »
Quote from: srsa_4c on 21 / March / 2015, 17:06:29
The IXUS110_SD960 also has the "feather".
Thanks for the info. This seems to just have the normal jogdial stuff in the code. If it works OK, this is fine.

It looks like the KEY_*_SOFT bits are masked out in the key mask, which I guess corresponds to the comment seen in other cameras

// override key and feather bits to avoid feather osd messing up chdk display in ALT mode

CAM_FEATURE_FEATHER appears to only be used in kbd.c files, so can become a platform_kbd.h define once those cameras are converted. I have also been trying to restrict defines that are only relevant to the keyboard code to platform_kbd.h rather than platform_camera.h, so they don't end up being used outside of kbd.c / kdb_common.c with a conscious decision.
Logged
Don't forget what the H stands for.
*

Offline srsa_4c

  • ******
  • 4451
Re: Making kbd.c code generic
« Reply #39 on: 21 / March / 2015, 19:34:17 »
Quote from: reyalp on 21 / March / 2015, 18:36:43
SX100 doesn't have pwr_on/off equivalents, but GetKbdState is more than just simple MMIO reads. I converted it to use GetKbdState/read_keys_r2.
The cam appears to work fine with r4107. This camera's button handling is a bit unusual: most buttons don't appear to toggle bits when viewing the usual MMIO range (0xc0220200, +4, +8) via my memory viewer. It could be that those buttons are only powered when being read out (kbd_pwr_on, etc.).

Quote from: reyalp on 21 / March / 2015, 18:56:44
It looks like the KEY_*_SOFT bits are masked out in the key mask, which I guess corresponds to the comment seen in other cameras

// override key and feather bits to avoid feather osd messing up chdk display in ALT mode
Perhaps these could be turned into alternative input sometime in the future...

edit:
almost forgot: thanks for doing the conversion!
« Last Edit: 21 / March / 2015, 19:44:41 by srsa_4c »
Logged
Pages: 1 2 3 [4] 5 6   Go Up
« previous next »
 

Related Topics