USB Remote Switch in CHDK - version 2 implementation thread - page 6 - General Discussion and Assistance - CHDK Forum supplierdeeply
« previous next »
Pages: 1 2 3 4 5 [6] 7 8 9 10 11 ... 23   Go Down

USB Remote Switch in CHDK - version 2 implementation thread

  • 220 Replies
  • 73439 Views
*

Offline waterwingz

  • ******
  • 12138
    • Waterwingz
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #50 on: 15 / January / 2012, 22:54:11 »
Advertisements
Quote from: philmoz on 15 / January / 2012, 22:50:33
Problem on the SX40 -> USB_IDX != SD_READONLY_IDX so the code in my_kbd_read_keys is not clearing the SD read-only flag correctly. In the original kbd.c code I had to seperate the USB & SD read only settings onto two lines.
Not sure if any other cameras may be like this.
Sorry.   I actually saw that and cut&pasted the code thinking to use it in all the ports.  But only 5 cams have SD_READONLY_IDX defined and it looked like they were all "2" - same as the USB_IDX. Not sure how I missed the SX40 difference.

Edit : 8 cameras have it defined in kbd.c.   But then I noticed that its in the stub_entry.S file - just commented out.  Another 3 hour marathon edit session at some point I guess.
« Last Edit: 15 / January / 2012, 23:00:24 by waterwingz »
Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #51 on: 15 / January / 2012, 22:59:52 »
I also added the IXUS 230 this morning so that one will also need updating - sorry :)

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)
*

Offline waterwingz

  • ******
  • 12138
    • Waterwingz
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #52 on: 15 / January / 2012, 23:00:56 »
Quote from: philmoz on 15 / January / 2012, 22:59:52
I also added the IXUS 230 this morning so that one will also need updating - sorry :)
Trust me - I was grinding my teeth as I added the a3300 at the last minute today  ....
Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline waterwingz

  • ******
  • 12138
    • Waterwingz
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #53 on: 15 / January / 2012, 23:17:16 »
Quote from: reyalp on 15 / January / 2012, 22:17:15
I'm inclined to get this in the trunk quickly, this is what we have an unstable branch for, and from a code cleanup point of view alone it looks like a huge step forward.
@reyalp  Let me know if you are going to go for this ?   I'll holding off adding the ixus230 and fixing the SX40 (and maybe going back through all 73 kbd.c files to add Philmoz's generalization ..) until then.

Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline reyalp

  • ******
  • 14079
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #54 on: 15 / January / 2012, 23:25:12 »
Quote from: waterwingz on 15 / January / 2012, 23:17:16
@reyalp  Let me know if you are going to go for this ?   I'll holding off adding the ixus230 and fixing the SX40 (and maybe going back through all 73 kbd.c files to add Philmoz's generalization ..) until then.
If you can do the keyboard fix (assuming it works) and resubmit, I think that would be better since the current code might have have bad effects on some peoples cams.
« Last Edit: 15 / January / 2012, 23:33:07 by reyalp »
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #55 on: 15 / January / 2012, 23:38:06 »
Quote from: reyalp on 15 / January / 2012, 23:25:12
Quote from: waterwingz on 15 / January / 2012, 23:17:16
@reyalp  Let me know if you are going to go for this ?   I'll holding off adding the ixus230 and fixing the SX40 (and maybe going back through all 73 kbd.c files to add Philmoz's generalization ..) until then.
If you can do the keyboard fix (assuming it works) and resubmit, I think that would be better since the current code might have have bad effects on some peoples cams.

I've had this kbd.c variable initialisation code in all my cameras for a long time and never had any problems.

I don't see how initialising it differently will change anything - it sounds more like something in the way the A495 port has been done.

Phil.

Edit: The code in kbd_is_key_pressed() for the A495 (and A490) looks very dodgy to me! Some sort of reverse logic applied to four of the buttons - I wonder if this was some previous attempt to fix the odd behaviour you sometimes get if you don't initialise the variables?

« Last Edit: 15 / January / 2012, 23:53:17 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)
*

Offline waterwingz

  • ******
  • 12138
    • Waterwingz
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #56 on: 15 / January / 2012, 23:47:28 »
Quote from: philmoz on 15 / January / 2012, 23:38:06
I don't see how initialising it differently will change anything - it sounds more like something in the way the A495 port has been done.
Having waded through all the kbd.c files,  I notice a lot of small differences in my_kbd_read_keys().  Even though it seems like they should all be the same.  So a port acting different on startup seems hardly surprising.
Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #57 on: 16 / January / 2012, 00:01:03 »
Quote from: waterwingz on 15 / January / 2012, 23:17:16
Quote from: reyalp on 15 / January / 2012, 22:17:15
I'm inclined to get this in the trunk quickly, this is what we have an unstable branch for, and from a code cleanup point of view alone it looks like a huge step forward.
@reyalp  Let me know if you are going to go for this ?   I'll holding off adding the ixus230 and fixing the SX40 (and maybe going back through all 73 kbd.c files to add Philmoz's generalization ..) until then.


I want to merge the changes from the reyalp-flt branch back to the main trunk; but there are some conflicts with the usb remote changes.

Any thoughts on what order these should be done in? If I merge reyalp-flt in now then the usb remote patch will need re-work.

Or I can wait and manually resolve any issues when I merge - any preferences?

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)
*

Offline reyalp

  • ******
  • 14079
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #58 on: 16 / January / 2012, 00:14:16 »
Quote from: philmoz on 16 / January / 2012, 00:01:03
Any thoughts on what order these should be done in? If I merge reyalp-flt in now then the usb remote patch will need re-work.

Or I can wait and manually resolve any issues when I merge - any preferences?
If we can get the remote stuff in first, I think that would probably better. That leaves you resolving conflicts with changes that you are familiar with, and doing it in svn rather than external patches.

As far as the kbd.c stuff goes, I'm not sure I really understand the issue that initializing to all 1s solves (I understand there are phantom key presses, but I don't see how they get there). It sounds like the initial state of those is somehow leaking out to the canon firmware (but in my quick dig through a few kbd.c's I don't actually see how this is happening). If that's the case, then ones or zeros could equally be troublesome. Initializing everything to the state the canon firmware already has seems like it would be the safest.
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: USB Remote Switch in CHDK - an open discussion about cleaning up the code
« Reply #59 on: 16 / January / 2012, 00:24:22 »
Quote from: reyalp on 16 / January / 2012, 00:14:16
Quote from: philmoz on 16 / January / 2012, 00:01:03
Any thoughts on what order these should be done in? If I merge reyalp-flt in now then the usb remote patch will need re-work.

Or I can wait and manually resolve any issues when I merge - any preferences?
If we can get the remote stuff in first, I think that would probably better. That leaves you resolving conflicts with changes that you are familiar with, and doing it in svn rather than external patches.

Makes sense - I'll hold off until the remote stuff is in then.

Quote
As far as the kbd.c stuff goes, I'm not sure I really understand the issue that initializing to all 1s solves (I understand there are phantom key presses, but I don't see how they get there). It sounds like the initial state of those is somehow leaking out to the canon firmware (but in my quick dig through a few kbd.c's I don't actually see how this is happening). If that's the case, then ones or zeros could equally be troublesome. Initializing everything to the state the canon firmware already has seems like it would be the safest.

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

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.

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)
Pages: 1 2 3 4 5 [6] 7 8 9 10 11 ... 23   Go Up
« previous next »
 

Related Topics