Question on how to condense code

[bsnut]

I was looking at Jon's and your program BigRez and I was thinking their is another way to do write it. This is a great challenge, which I love and will continue work with Jack's program that he posted, since Jack liked the timing.

As, I stated this is a great learning experience for me and everyone else.

[JackMan]

Well, I came up with this version which uses 42% of the EEPROM. I know, I know,......It doesn't refresh the servo, but it works for my application as the servo doesn't have a load when paused.

Code Select Expand

Main:
  PAUSE 29500
  FOR counter = 1 TO 4
  GOSUB Servo_Move
  NEXT

  PAUSE 13900
  FOR counter = 1 TO 4
  GOSUB Servo_Move
  NEXT

  PAUSE 27750
  FOR counter = 1 TO 4
  GOSUB Servo_Move
  NEXT

  PAUSE 14000
  FOR counter = 1 TO 4
  GOSUB Servo_Move
  NEXT

  PAUSE 14000
  FOR counter = 1 TO 4
  GOSUB Servo_Move
  NEXT

  END

  Servo_Move:
  FOR idx = 1 TO 4
   FOR pos = 150 TO 125 STEP -4
   PULSOUT Servo, pos
   PAUSE 19
   NEXT
   FOR pos = 125 TO 150 STEP 4
   PULSOUT Servo, pos
   PAUSE 19
   NEXT
  NEXT
  PAUSE 2200
   RETURN

[JonnyMac]

Coding style become habit.... if you break the "no PAUSE greater than 20ms" rule now, you probably will in the future when it then bites you on the backside.  It really is better to do it right the first time.

Forgive me for being selfish, but I'm also concerned that newcomers will look at this and emulate it.

[bsnut]

I agree with Jon on his point that "Coding style become habit". I do not want a newcomers to see programs were servos are used to have PAUSE greater than 20ms in them. We don't want promote bad habits ;D on this site.

I am condensing your program right now with the "20ms" rule and your timing in it. I have one question...Do you any other code that needs to be in this other than servo code?

[JackMan]

Coding style become habit.... if you break the "no PAUSE greater than 20ms" rule now, you probably will in the future when it then bites you on the backside.  It really is better to do it right the first time.

Forgive me for being selfish, but I'm also concerned that newcomers will look at this and emulate it.

Agreed. Feel free to remove that post if you like, you won't hurt my feelings.  ;D I just know that for this particular application it won't be a problem.

I am condensing your program right now with the "20ms" rule and your timing in it. I have one question...Do you any other code that needs to be in this other than servo code?

Bill,
    I appreciate it but really, there's no need to waste your time on this. I had intentions of using a Prop-1 to drive the jaws of a pair of background singers in part of my show but I'm not sure I'm gonna fit everything in the eeprom, hence the reason for trying to condense the first half of it. If I can fit it in the Prop-1 it will be an easier route than creating another sound track and using an MP3 player and audio/servo driver.

[JonnyMac]

No need to remove it; there are plenty of warnings and admonitions in this thread.

[bsnut]

Jack you are not wasting my time and love the challenge and I have no problem doing it for you or someone else. One thing it shows that a program like this can be condense into a small program, for a smaller EEPROM. I only made one minor change to your code and that is the value for the PAUSE, which was this 277750 to this 277700. I hope it doesn't put your timing off.

Here is, what I came up with and it uses only 36% of the EEPROM. I hope you have a wig wag time with this one and hope I didn't upset anyone :).

'   File.......Servo Test.BS1
'   Purpose....
'   Author.....W. Stefan
'   E-mail.....
'   Started....
'   Updated....
' {$STAMP BS1}
' {$PBASIC 1.0}
'INPUTS
'--------------------------------------------------------------------


'OUTPUTS
'--------------------------------------------------------------------
SYMBOL Servo  = 0

'RC-4/AP-8 SERIAL I/O
'--------------------------------------------------------------------

'CONSTANTS
'--------------------------------------------------------------------
SYMBOL delay0   = 29500
SYMBOL delay1   = 13900
SYMBOL delay2   = 27700   'New value. Change this 27750 for 20ms to work out
SYMBOL delay3   = 14000
SYMBOL delay4   = 14000
SYMBOL delay5   = 2200

'VARIABLES
'--------------------------------------------------------------------
SYMBOL counter  = B2
SYMBOL counter1 = B3
SYMBOL pos      = B4
SYMBOL idx      = B5
SYMBOL delay    = W3
'EEPROM DATA
'--------------------------------------------------------------------

'INITIALIZATION
'--------------------------------------------------------------------


'--------------------------------------------------------------------
'PROGRAM CODE


Main:
 LOOKUP idx, (delay0, delay1, delay2, delay3, delay4, delay5), delay

Servo_Delay:
 PULSOUT Servo, 150                   'Holds servo centered
 PAUSE 20                             'Servo update pulse and "delay" pause
 delay = delay - 20                   'Count down "delay"
 IF idx = 5 AND delay = 0 THEN Done
 IF delay = 0 THEN Servo_Count
 GOTO Servo_Delay

Servo_Count:
 FOR counter = 1 TO 4
   GOSUB Servo_Move
 NEXT
 GOTO Main

Done:
 idx = 0
 END

'--------------------------------------------------------------------

'SUBROUTINES
'--------------------------------------------------------------------
Servo_Move:
 FOR counter1 = 1 TO 4
   FOR pos = 150 TO 125 STEP -4
     PULSOUT Servo, pos
     PAUSE 19
   NEXT
   FOR pos = 125 TO 150 STEP 4
     PULSOUT Servo, pos
     PAUSE 19
   NEXT
 NEXT
 idx = idx + 1 MAX 5                 'Use MAX to clamp "idx" at 5
 RETURN    

[JonnyMac]

Bill,

You have to change the definition of Servo to 0 instead of PIN0.  When you do this,

 PULSOUT PIN0, timing

... the Prop-1 reads the value of PIN0 (0 or 1) and the sends the pulse to that pin -- this can lead to errors.  Since you mean 0 you need to tell the controller that in the pin definition.

It should be:

SYMBOL Servo  = 0

[BigRez]

Bill - there are a few other things wrong/incorrect in the above program...

1) LOOKUP is trying to put a word value into a byte variable.  The symbols delay0-4 all contain values that are WORD sizes, but delay is only a byte.  This means data values are truncated.  For example, on the first run, 29500 has high and low byte values of 115 and 60. So the first time through delay is set to only 60.  Change delay to W5.

2) You are decrementing delay by only 1 but shouldn't it really be at least 20 to account for the 20ms PAUSE and PULSOUT commands?

3) The Servo_Count routine will execute ONLY ONCE even though it's in a FOR...NEXT loop defining four iterations.  This is because the control variable counter is changed in the Servo_Move routine and when this routine exits, counter is set to four which causes the Servo_Count loop to exit. Did you mean to use counter1 for one of the two loops?

4) The mechanism for producing the 2.2 second delay after each set of four "wags" isn't correct in that after the first set, idx will be four. This will cause Main to determine the next pause to be the fifth element in the array (delay4) and it'll do that wag and pause, but then in the second set of wags it'll stop incrementing at five. This will cause the program to pause 2.2 seconds the next time in Main and then end.  It'll never do the delay1, delay2, delay3 steps and won't do the 2.2 second delay (delay5) in between each set. idx should be incremented after the loop in Servo_Count, but that still won't properly produce the 2.2 second delay after each wag set.

(Edited #4 above to be more accurate.)

[bsnut]

Jon,

Thanks for catching that mistake. It edited it for this PIN0 to this 0. Sometimes I get a head of myself and should have caught it on the second edit that I did.

BigRez,

Hey-anyone seen my mistake in my source code ;D? I was fixing my first mistake when you were posting your post. I should have caught it and I thank you.