Better way to do this code function?

xanatos · 2012-03-25 14:35

Hi there,

I have the following code that gets each digit as it is being input to a keypad (via EDE1144). It displays the digits, "growing" the input characters from the left-side of the display as in:

1
12
123
1234
123456 (of course, not on multiple lines... I think you get what I'm saying)

I need to capture the six-digit number that is entered as two Words that I subsequently PUT to SPRAM (and write to EEPROM in another slot, I'm using a BS2px), so I make the two words via positional multiplication.

This all WORKS very well, but it takes up a LOT of lines of code. Every time I look at this, I think to myself that there MUST be a more efficient way of doing the task. I'd like to stuff a few more functions in this program slot, and with this albatross in the middle of it all, I fall short on space.

It's all that d6=d5, d5=d4, d4=d3 stuff that seems just ... lame. And the multiple SEROUT LCDs... Works like a champ... just bloated, I think.

Is this the only way, or is there a better way that does all the same functions (especially capturing the top 3 digits as a word, and the bottom three digits as another word)???

Code below... thanks for any thoughts/ideas...

Dave

PS., the GOSUB just captures the SERIN data from the EDE1144 when the Data Valid line goes low, and stores the output in a var called keyIn. It also checks it for # and *, since I have special functions assigned to those keys.

    GOSUB getKeyIn

    IF keyIn = 14 THEN dispChar = "*"
    IF keyIn = 15 THEN dispChar = "#"

    IF (keyIn < 10) THEN
      IF (keyCount = 1) THEN
        d1 = keyIn
        SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d1, "       "]
      ELSEIF (keyCount = 2) THEN
        d2 = d1
        d1 = keyIn
        SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d2, DEC1 d1]
      ELSEIF (keyCount = 3) THEN
        d3 = d2
        d2 = d1
        d1 = keyIn
        SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d3, DEC1 d2, DEC1 d1]
      ELSEIF (keyCount = 4) THEN
        d4 = d3
        d3 = d2
        d2 = d1
        d1 = keyIn
        SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d4, DEC1 d3, DEC1 d2, DEC1 d1]
      ELSEIF (keyCount = 5) THEN
        d5 = d4
        d4 = d3
        d3 = d2
        d2 = d1
        d1 = keyIn
        SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d5, DEC1 d4, DEC1 d3, DEC1 d2, DEC1 d1]
      ELSEIF (keyCount = 6) THEN
        d6 = d5
        d5 = d4
        d4 = d3
        d3 = d2
        d2 = d1
        d1 = keyIn
        SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d6, DEC1 d5, DEC1 d4, DEC1 d3, DEC1 d2, DEC1 d1]
      ENDIF
    ELSEIF (keyIn = 14) THEN  ' 14 = * (Abort and go to beginning)
      'SEROUT LCD, LcdBaud, [$FE, $45, $40, dispChar, "      "]
      'PAUSE 500
      keyCount = 0
      GOSUB redoReps 'Main
    ELSEIF (keyIn = 15) THEN  '15 = # (Accept entry and go to next op)
      'SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC1 d6, DEC1 d5, DEC1 d4, DEC1 d3, DEC1 d2, DEC1 d1, dispChar]
      'PAUSE 500
      keyCount = 0

      TRKREP = (100 * d6) + (10 * d5) + d4
      TRKREP2 = (100 * d3) + (10 * d2) + d1

      IF whichTrack = 1 THEN
        PUT 16, Word TRKREP  
        WRITE 16, Word TRKREP
        PUT 26, Word TRKREP2
        WRITE 26, Word TRKREP2
      ENDIF

      EXIT
    ENDIF

Catspaw · 2012-03-25 16:12

Why not just use an array. Start with nothing in the array, then as each number is input, send element(0), then (0),(1), then (0),(1),(2)....
instead of shifting each number into a different variable.

xanatos · 2012-03-25 17:51

Hi Catspaw, and thanks. Honestly that one was staring me in the face. Too tired... long day. Thank you for that well needed reminder! :-) It did save me some space, but only about 1/10th of what I actually need to free up. Guess I'll go play with turning the SEROUT LCDs into a gosub for/next loop of some sort that will hopefully be smaller than the individual lines.

Thanks again,

Dave

Tracy Allen · 2012-03-25 22:48

Hi Dave,

I agree that the array would be a good way to go. Another could be to build up the variables and display them in place. The following accumulates 4 decimal digits first into a word and then two more into a byte, and at each step displays the accumulation. (I think it would?!) Chars 14, 15 are special case.

[SIZE=1][FONT=courier new]wx VAR WORD  ' high 4 digits
bx VAR BYTE ' low 2 digits, together 000000 to 999999
idx VAR NIB
buildDouble:
  wx=0
  bx=0
  FOR idx=0 TO 3   ' build a word wx range 0000 to 9999
     GOSUB getkeyin
     IF keyIn>9 THEN specialCase
     wx = wx * 10 + keyIn
     SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC wx]
  NEXT
  FOR idx=4 TO 5   ' build byte bx range 00 to 99
     GOSUB getkeyin
[/FONT][/SIZE][SIZE=1][FONT=courier new]     IF keyIn>9 THEN specialCase
[/FONT][/SIZE][SIZE=1][FONT=courier new]     bx = bx * 10 + keyIn
     SEROUT LCD, LcdBaud, [$FE, $45, $40, DEC wx] ' first 4 digits
     SELECT idx
       CASE 4
         SEROUT LCD, LcdBaud, [DEC wx,DEC1 bx] ' one digit
       CASE 5   
         SEROUT LCD, LcdBaud, [DEC wx,DEC2 bx] ' two digits
      ENDSELECT
  NEXT[/FONT][/SIZE]

xanatos · 2012-03-26 17:54

Hi Tracey,

Thanks everybody, I was successful in reducing that mess down. Between using the array and some artful for/next looping in a gosub, I turned that mess into six lines of main code and a four line gosub. Works like a champ! Thanks for all your help, my code looks much better now! But now I have an even wierder issue - in my next post! :-)

Thanks again,

Dave

Better way to do this code function?

Comments