Better way to do this code function?

xanatos

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

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

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

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

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