Question on how to condense code

[JackMan]

Looking for advice from the experts on any way to use less eeprom for this code.

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 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
   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
   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
   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
   PAUSE 2200
   RETURN

[bsnut]

JackMan,

I was looking at your code and here's what you can do. You can use extra FOR/NEXT in "Servo_Move" label and it will look like this. 


Servo_Move:
 FOR counter = 0 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

Leave the rest of your code alone under the label "Main" and use the Syntax Check feature to check how much space is used. I just did this myself and noticed that you will save 27%. Now the code is taking up 44% of the EEPROM.


     

[JonnyMac]

Since you have a lot of redundant code you can do it with loops.  The only thing you have to be mindful of is not having more than 16 GOSUBs in your Prop-1 program.  My version uses more variable space than yours, but consumes only 46% of the program space (yours takes 70%).

Important Note: It is a very bad thing to have a PAUSE more than 20ms when servos are involved -- they need to be refreshed all the time.

' =========================================================================
'
'   File......
'   Purpose...
'   Author....
'   E-mail....
'   Started...
'   Updated...
'
'   {$STAMP BS1}
'   {$PBASIC 1.0}
'
' =========================================================================


' -----[ Program Description ]---------------------------------------------


' -----[ Revision History ]------------------------------------------------


' -----[ I/O Definitions ]-------------------------------------------------

SYMBOL  Sio             = 7                     ' SETUP = UP; no ULN
SYMBOL  Trigger         = PIN6                  ' SETUP = DN

SYMBOL  Servo           = 0


' -----[ Constants ]-------------------------------------------------------

SYMBOL  IsOn            = 1                     ' for active-high in/out
SYMBOL  IsOff           = 0

SYMBOL  Baud            = OT2400


' -----[ Variables ]-------------------------------------------------------

SYMBOL  pos             = B2                    ' servo position
SYMBOL  ofs             = B3                    ' offset in LOOKUP table
SYMBOL  idx             = B4
SYMBOL  counter         = B5

SYMBOL  delay           = W5                    ' holds ms for Servo_Pause


' -----[ Initialization ]--------------------------------------------------

Reset:
 PINS = %00000000                              ' clear all outputs
 DIRS = %00111111                              ' make P0-P5 outputs

 pos = 150


' -----[ Program Code ]----------------------------------------------------

Main:
 FOR ofs = 0 TO 4
   LOOKUP ofs, (29500, 13900, 27750, 14000, 14000), delay
   GOSUB Servo_Pause
   FOR counter = 1 TO 4
     GOSUB Servo_Move
   NEXT
 NEXT

The_End:
 pos = 150
 GOSUB Update_Servo
 GOTO The_End


' -----[ Subroutines ]-----------------------------------------------------

Servo_Move:
 FOR idx = 1 TO 4
   FOR pos = 150 TO 125 STEP -4
     GOSUB Update_Servo
   NEXT
   FOR pos = 125 TO 150 STEP 4
     GOSUB Update_Servo
   NEXT
 NEXT
  delay = 2200
  GOSUB Servo_Pause
 RETURN

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

Update_Servo:
 PULSOUT Servo, pos
 PAUSE 19
 RETURN

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

Servo_Pause:
 IF delay < 20 THEN SP_Exit
   PULSOUT Servo, pos
   PAUSE 19
   delay = delay - 20
   GOTO Servo_Pause

SP_Exit:
 PAUSE delay
 RETURN


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


' -----[ User Data ]-------------------------------------------------------

[bsnut]

I forgot that servos need to be refreshed all the time and Jon found the way to do it.

[JackMan]

Thanks for the info guys but I just loaded both programs and unfortunately niether one worked. My original has the servo moving back and forth 4 times rather quickly each time GOSUB is commanded. Bsnut, your change looked good, but only moved the servo 4 times on the first mark and that was it. Jon, yours only moved the servo slow one rep at a time with a long pause between. My timing for the sets of 4 is critical. I know about the refresh but I've been running two other servo programs on my props for quite some time with lengthy pauses and no issues. No need to spend a lot of time on this, it's not that important. Thanks.

[JonnyMac]

Jack,

I accidentally stuck the 2.2s delay that comes at the end of the four wags inside the loop (between wags) -- see the code above (orange lines) that are now moved to where they belong.

Is there a compelling reason to embed that 2.2s delay in the subroutine when you're inserting delays between the calls to it, anyway?

[bsnut]

I understand were you are coming from Jack and I don't like give up :). So, I took your little program condense it into two subroutines. This program only takes up 42% of the space on the prop1.

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


'OUTPUTS
'--------------------------------------------------------------------
SYMBOL Servo  = PIN0

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

'CONSTANTS
'--------------------------------------------------------------------


'VARIABLES
'--------------------------------------------------------------------
SYMBOL counter = B2
SYMBOL pos     = B3
'EEPROM DATA
'--------------------------------------------------------------------

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


'PROGRAM CODE


Main:
  PAUSE 29500
  GOSUB Servo_Count
  GOSUB Servo_Move


  PAUSE 13900
  GOSUB Servo_Count
  GOSUB Servo_Move


  PAUSE 27750
  GOSUB Servo_Count
  GOSUB Servo_Move


  PAUSE 14000
  GOSUB Servo_Count
  GOSUB Servo_Move


  PAUSE 14000
  GOSUB Servo_Count
  GOSUB Servo_Move
  END

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

'SUBROUTINES
'--------------------------------------------------------------------
Servo_Count:
  FOR counter = 1 TO 4
    GOSUB Servo_Move
  NEXT
  return

Servo_Move:
  FOR counter = 0 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
   PAUSE 2200
   RETURN

[BigRez]

I wish I had my boards (and a servo) with me to test this out...  this has an even smaller footprint and is clean.  (Hopefully I'll get back to my dungeon tomorrow and can test it.)

BSNut - you have a few issues in your program; First, if the servos have any load and they're not refreshed during the pauses, the servos could move.  Second, you've got a double-pause at the end.  Lastly, I believe the value of counter in Servo_Count is changed when you use the same variable in the Servo_Move routine, which will cause Servo_Count to execute only once.  (I haven't tested it so I could be wrong.)

I was interested to see if I could shrink it even more and came up with this version based heavily off of JonnyMac's program, but adds a different technique for looping.  Shrinks it by 14 more bytes.  (Yeah - it's probably missing something.)

Last Update: 9/29/2010 22:32 PDT

Code Select Expand

' =========================================================================
'
'   File......
'   Purpose...
'   Author....
'   E-mail....
'   Started...
'   Updated...    9/29/2010 22:32
'
'   {$STAMP BS1}
'   {$PBASIC 1.0}
'
' =========================================================================

' -----[ Program Description ]---------------------------------------------

' -----[ Revision History ]------------------------------------------------

' -----[ I/O Definitions ]-------------------------------------------------
SYMBOL Servo  = PIN0

' -----[ Constants ]-------------------------------------------------------

' -----[ Variables ]-------------------------------------------------------
SYMBOL pauser  = W1         ' full pause value
SYMBOL pauserLo= B2          ' low bit read from eeprom
SYMBOL pauserHi= B3          ' high bit read from eeprom
SYMBOL pntr    = B8
SYMBOL looper1 = B9
SYMBOL looper2 = B10
SYMBOL pos     = B11

' -----[ Initialization ]--------------------------------------------------

Reset:
  pos = 150

' -----[ Program Code ]----------------------------------------------------
Main:
  READ pntr, pauserHi                           ' read pause value
  pntr = pntr + 1                               ' increment the memory location
  READ pntr, pauserLo                           ' read pause value
  pntr = pntr + 1                               ' increment the memory location
  IF pauser = 0 THEN WeAreDone                  ' zero indicates we're done
  GOSUB ServoPauser                             ' do the required pause

  GOSUB MoveTheServo                            ' Do the movement routine
  GOTO Main                                     ' go back and do again until we read a zero

WeAreDone:
  ' assumes the value of pos remains at 150
  GOSUB UpdateServo
  GOTO WeAreDone


' -----[ Subroutines ]-----------------------------------------------------
MoveTheServo:
 FOR looper1 = 1 TO 4                           '   We'll loop 4 times (outer loop). Each time,
    FOR looper2 = 1 TO 4                           '   We'll loop 4 times (inner). Each time,
      FOR pos = 150 TO 125 STEP -4                 '   Wind the servo counter-clockwise...
         GOSUB UpdateServo
      NEXT
      FOR pos = 125 TO 150 STEP 4                  '   then wind it clockwise...
        GOSUB UpdateServo
      NEXT
   NEXT
   pauser = 2200                                   ' pause 2.2 seconds while refreshing the servo
   GOSUB ServoPauser
 NEXT
 RETURN

ServoPauser:                                        ' Assumes first entry here is >22
 GOSUB UpdateServo
 pauser = pauser - 25
 IF pauser > 22 THEN ServoPauser
 PAUSE pauser                                      ' pause remainder of amount
 RETURN

UpdateServo:
 PULSOUT Servo, pos
 PAUSE 19
 RETURN

' -----[ User Data ]-------------------------------------------------------
PauseData:
 EEPROM (115,60)     'Hi and lo value for 29500
 EEPROM (54,76)      '                    13900
 EEPROM (108,102)    '                    27750
 EEPROM (54,176)     '                    14000
 EEPROM (54,176)     '                    14000
 EEPROM (0)


[JackMan]

Is there a compelling reason to embed that 2.2s delay in the subroutine when you're inserting delays between the calls to it, anyway?

Yeah, I need a 2.2s delay between each set of 4 servo repetitions. For each GOSUB in my original code there are 4 sets of 4 servo wags. The various delays between GOSUB's are critical to the timing of the sets. I'll give everyone's new version a try today and let ya know. Thanks guys, I appreciate your time.  ;)

[JackMan]

Well, none of these work like my original but Jon's was the closest.

Jon's version:
    For some reason the timing between sets is way off. It's giving me (roughly) 33s to the 1st set, 17s to the 2nd, 33s to the 3rd, 17s to the 4th, and 17s to the final.

Bsnut's version:
    Good on the timing bewteen sets but I'm only getting 2 sets of 4 servo wags for each GOSUB of my original (should be 4 sets of 4 wags)

bigrez's version:
    A+ for effort, but it doesn't move the servo at all.

Again, I appreciate everyone's effort but don't knock yourself out on this. At this time of the season I'm sure you have better things to do!

[JonnyMac]

For some reason the timing between sets is way off. It's giving me (roughly) 33s to the 1st set, 17s to the 2nd, 33s to the 3rd, 17s to the 4th, and 17s to the final.

Remember... you shouldn't use a PAUSE of more than 20ms when you have servos involved, otherwise they may start to drift for not being refreshed.  I changed:

 PAUSE 14000

to:

 delay = 14000
 GOSUB Servo_Pause

Now, let's look at Servo_Pause -- when the servo is centered (150, or 1.5ms)

Servo_Pause:
 IF delay < 20 THEN SP_Exit
   PULSOUT Servo, pos
   PAUSE 19
   delay = delay - 20
   GOTO Servo_Pause

SP_Exit:
 PAUSE delay
 RETURN

What's happening is that the actual loop delay is 1.5ms (servo position value) plus 19, [theoretical] total of 20.5ms -- plus some call/loop overhead.  With a Prop-1 you're never going to get it perfect.

What to do?  Well, when the going gets tough, the tough get out the oscilloscope.  I setup a little test code:

Main:
 PINS = %00000010
 'delay = 20
 'GOSUB Servo_Pause
 PINS = %00000000
 PAUSE 50
 GOTO Main

Note that I've commented out the working part -- I need a baseline for the scope indicator (using PIN1) so that I know how much time that consumes. In the first image you can see that it takes ~430us (0.43ms) to flip a pin from on to off.

Now the working lines get un-commented and the program runs again.  In a perfect world, the pulse width would bet 20.4ms -- but it's not, its actually 25.9.

Why?  The Prop-1 is not very fast and the overhead running that loop takes extra time.  Knowing what you know you can change the delay update within the loop to 25 or 26 and that will get you much closer to your target delay.  That said, you'll sill need to do a bit of empirical fine-tuning.

Since I had the 'scope hooked up I measure the delay for the routine at these standard servo points (60 and 240 are endpoints for 180-degree servos)

60 = 24.8ms
100 = 25.1ms
150 = 25.4ms
200 = 26.1ms
240 = 26.6ms

Now I'm getting obsessed, so I ditch the 'scope (Parallax PropScope) and hook-up a Propeller board.  The Propeller runs at 80 MHz and has configurable counters that can do very precise pulse measurments -- convenitenly, one of the modes meausres the duration of a high-going pulse.  So I wrote a simple test program (attached for those that want to have a look at how the Propeller [brain of the AP-16+] is programmed).

Here are the results:

no pulse (test setup) = 430us
60 = 24935 - 430 = 24.505ms
100 = 25334 - 430 = 24.904ms
150 = 25832 - 430 = 25.402ms
200 = 26331 - 430 = 25.901ms
240 = 26730 - 430 = 26.300ms

The average, using the range of servo values, is 25.4 (no surprise, that was the value at the center position).  So... change the subroutine to this:

Servo_Pause:
 IF delay < 20 THEN SP_Exit
   PULSOUT Servo, pos
   PAUSE 19
   delay = delay - 25
   GOTO Servo_Pause

SP_Exit:
 PAUSE delay
 RETURN

... and you'll be close with just minor fine tuning required.

Is there a way to get more precise, no matter what the position value?  Yes, but it takes a spare pin and a word variable so I'm not going to get into it unless someone really needs it.

In the end, the Prop-1 is a great little controller, but in some cases it seems like we're asking a go-kart to win the Indy 500.  That's not to say that we can't run the race, but we have to work within our limits and, as I have demonstrated in this post, to a little code tuning to get the results we want.

[BigRez]

My version didn't work because the value being read into pauser was a word-size value rather than a byte-size.  I've corrected that above along with Jon's tuning findings. Again since I don't have a Prop-1 with me (and not sure where I left my spare) I can't test till tonight.  

I gotta get some more prop-1's... Hey, maybe I'll swing by JonnyMac's place on my way home and pick up another (or just buy him a Starbucks), or wait for a nice special to pick up a few of them  ;)

[bsnut]

Let me answer your question Bigrez.

BSNut - you have a few issues in your program; First, if the servos have any load and they're not refreshed during the pauses, the servos could move.  Second, you've got a double-pause at the end.  Lastly, I believe the value of counter in Servo_Count is changed when you use the same variable in the Servo_Move routine, which will cause Servo_Count to execute only once.  (I haven't tested it so I could be wrong.)

I know, I need to refresh the servos in my program, but I was only condensing Jack's code on the second try and Jack wasn't to worried about the refreshing the servos. That should answer one of your questions.

Now for your other question about the FOR/NEXT loop. The one cool thing about the FOR/NEXT loop is that, the variable starts with a "0" each time the FOR/NEXT is called.

Jon did the must work, by showing us that servos do need to be refreshed and also condensed his program. You stated that, they need to be refreshed under load which is right.

The goal is to learn from each other. That is why we post are puzzles "code" here ; :).      

[JackMan]

I'm really sorry bigrez, but the servo still doesn't move. Thanks again for the effort though.  ;)

Jon's got it now, I just need to play around with the numbers in the LOOKUP line. Once again, he is the program Guru. Thanks to all that pitched in, hopefully I learned something here.  ???

[BigRez]

OK Jack - found the problem.  (I considered it a challenge!)   When using the EEPROM area and READ, we're dealing with bytes not words.  I had to store the HI and LO values of the pause values and then read both hi and lo into the variable.

I was able to load this modified program (above) into a prop-1 with a servo connected and it works fine although you may have to fine tune some of the pausing.  And of course, it's now about the same size as Jon's (119 vs 120 bytes).

It was a good exercise though because I learned something new and hopefully so will others.

BSNut -

The one cool thing about the FOR/NEXT loop is that, the variable starts with a "0" each time the FOR/NEXT is called.

But what you may not be seeing is that the FOR loop in the Servo_Count routine is executed ONLY ONE TIME.  This is due to variable scope; both routines use the same memory location. So although counter is set to one the first time through the loop in Servo_Count, after it returns from the Servo_Move routine the value of counter is now 4.  The FOR loop in Servo_Count then exists.  Additionally, the 0 to 4 parameter in the Servo_Move "wags" the servos five times but only four "wags" are wanted.