Better ways to write repeated code

g3cwi · 2012-06-04 02:14

Hi

I am working on a barometric trend display Object. It works well but I can't help thinking that the code is way too long. Can anyone help on ways to streamline some of the lengthy tests etc?

Thanks

Richard

CON

  _clkmode      = xtal1 + pll16x  
  _xinfreq      = 5_000_000       

  ALTITUDE      = 145 'Your altitude in metres ASL
  
  'EEPROM locations for hourly observations
  R0   = $9003
  R1   = $9007
  R2   = $900b
  R3   = $900f
  R4   = $9013
  R5   = $9017
  R6   = $901b
  R7   = $901f
  R8   = $9023
  R9   = $9027
  R10  = $902b
  R11  = $902f
  R12  = $9033
  R13  = $9037
  R14  = $903b
  R15  = $903f  
  R16  = $9043
  R17  = $9047
  R18  = $904b
  R19  = $904f 
  R20  = $9053
  R21  = $9057
  R22  = $905b
  R23  = $905f
  Time = $9063
  
  'Flag for initial boot detection
  Flag = $9067
  
  'Storage for trend values
  T3   = $906b
  T1   = $906f
  Ti   = $9073
  
'By Richard Newstead, G3CWI, June 2012
OBJ

  bar   : "29124_altimeter"
  rtc   : "DS1302_full"
  SN    : "Simple_Numbers" 
  DEBUG : "Debug_LCD03"
  EEPROM: "I2C_ROMEngine"

Var

  byte hour, minute, second
  Long time_long,Trend3,Trend1,Trendi

PUB Start 'Start everything running
  
  rtc.init( 4, 5, 6 ) 'define ports for Real Time Clock
  
  bar.start_explicit( 0, 1, true )
  bar.set_resolution( bar#highest )  'Set to highest resolution. 

  DEBUG.start( 15, 9600, 4 ) '4 lines serial LCD  
  
  EEPROM.ROMEngineStart( 29, 28, 0 )            
  Initialise_EEPROM

  Main
     
Pub Main | p

  DEBUG.CLS 
  DEBUG.Backlight(true)
  DEBUG.Cursor(0)
 
  Display
  
  repeat

    rtc.readTime( @hour, @minute, @second )  'read time from DS1302

    'Update everything once a minute
    IF minute<>EEPROM.ReadLong(Time)
    
      p  := bar.average_press 'Get the average pressure.
       
      EEPROM.WriteLong(Time, minute) 'Store time
   
      time_long := hour * 10000 + minute * 100 + second 'Concatenate
 
        'Store hourly readings
        Case time_long
          000000:EEPROM.WriteLong(R0,  p)
          010000:EEPROM.WriteLong(R1,  p)
          020000:EEPROM.WriteLong(R2,  p)
          030000:EEPROM.WriteLong(R3,  p)
          040000:EEPROM.WriteLong(R4,  p)
          050000:EEPROM.WriteLong(R5,  p)
          060000:EEPROM.WriteLong(R6,  p)
          070000:EEPROM.WriteLong(R7,  p)
          080000:EEPROM.WriteLong(R8,  p)
          090000:EEPROM.WriteLong(R9,  p)
          100000:EEPROM.WriteLong(R10, p)
          110000:EEPROM.WriteLong(R11, p)
          120000:EEPROM.WriteLong(R12, p)
          130000:EEPROM.WriteLong(R13, p)
          140000:EEPROM.WriteLong(R14, p)
          150000:EEPROM.WriteLong(R15, p)
          160000:EEPROM.WriteLong(R16, p)
          170000:EEPROM.WriteLong(R17, p)
          180000:EEPROM.WriteLong(R18, p)
          190000:EEPROM.WriteLong(R19, p)
          200000:EEPROM.WriteLong(R20, p)
          210000:EEPROM.WriteLong(R21, p)
          220000:EEPROM.WriteLong(R22, p)
          230000:EEPROM.WriteLong(R23, p)
          
        'Using obs, calculate 3 hour trend   
        Case time_long
          000001..010000:Trend3:=EEProm.Readlong(R0)- EEProm.Readlong(R21)
          010001..020000:Trend3:=EEProm.Readlong(R1)- EEProm.Readlong(R22)
          020001..030000:Trend3:=EEProm.Readlong(R2)- EEProm.Readlong(R23)
          030001..040000:Trend3:=EEProm.Readlong(R3)- EEProm.Readlong(R0)
          040001..050000:Trend3:=EEProm.Readlong(R4)- EEProm.Readlong(R1)  
          050001..060000:Trend3:=EEProm.Readlong(R5)- EEProm.Readlong(R2)
          060001..070000:Trend3:=EEProm.Readlong(R6)- EEProm.Readlong(R3)
          070001..080000:Trend3:=EEProm.Readlong(R7)- EEProm.Readlong(R4)
          080001..090000:Trend3:=EEProm.Readlong(R8)- EEProm.Readlong(R5)
          090001..100000:Trend3:=EEProm.Readlong(R9)- EEProm.Readlong(R6)
          100001..110000:Trend3:=EEProm.Readlong(R10)-EEProm.Readlong(R7)
          110001..120000:Trend3:=EEProm.Readlong(R11)-EEProm.Readlong(R8)
          120001..130000:Trend3:=EEProm.Readlong(R12)-EEProm.Readlong(R9)  
          130001..140000:Trend3:=EEProm.Readlong(R13)-EEProm.Readlong(R10)
          140001..150000:Trend3:=EEProm.Readlong(R14)-EEProm.Readlong(R11)
          150001..160000:Trend3:=EEProm.Readlong(R15)-EEProm.Readlong(R12)
          160001..170000:Trend3:=EEProm.Readlong(R16)-EEProm.Readlong(R13)
          170001..180000:Trend3:=EEProm.Readlong(R17)-EEProm.Readlong(R14)
          180001..190000:Trend3:=EEProm.Readlong(R18)-EEProm.Readlong(R15)
          190001..200000:Trend3:=EEProm.Readlong(R19)-EEProm.Readlong(R16)
          200001..210000:Trend3:=EEProm.Readlong(R20)-EEProm.Readlong(R17)  
          210001..220000:Trend3:=EEProm.Readlong(R21)-EEProm.Readlong(R18)
          220001..230000:Trend3:=EEProm.Readlong(R22)-EEProm.Readlong(R19)
          230001..000000:Trend3:=EEProm.Readlong(R23)-EEProm.Readlong(R20)

        'Using obs, calculate 1 hour trend   
        Case time_long
          000001..010000:Trend1:=EEProm.Readlong(R0)- EEProm.Readlong(R23)
          010001..020000:Trend1:=EEProm.Readlong(R1)- EEProm.Readlong(R0)
          020001..030000:Trend1:=EEProm.Readlong(R2)- EEProm.Readlong(R1)
          030001..040000:Trend1:=EEProm.Readlong(R3)- EEProm.Readlong(R2)
          040001..050000:Trend1:=EEProm.Readlong(R4)- EEProm.Readlong(R3)  
          050001..060000:Trend1:=EEProm.Readlong(R5)- EEProm.Readlong(R4)
          060001..070000:Trend1:=EEProm.Readlong(R6)- EEProm.Readlong(R5)
          070001..080000:Trend1:=EEProm.Readlong(R7)- EEProm.Readlong(R6)
          080001..090000:Trend1:=EEProm.Readlong(R8)- EEProm.Readlong(R7)
          090001..100000:Trend1:=EEProm.Readlong(R9)- EEProm.Readlong(R8)
          100001..110000:Trend1:=EEProm.Readlong(R10)-EEProm.Readlong(R9)
          110001..120000:Trend1:=EEProm.Readlong(R11)-EEProm.Readlong(R10)
          120001..130000:Trend1:=EEProm.Readlong(R12)-EEProm.Readlong(R11)  
          130001..140000:Trend1:=EEProm.Readlong(R13)-EEProm.Readlong(R12)
          140001..150000:Trend1:=EEProm.Readlong(R14)-EEProm.Readlong(R13)
          150001..160000:Trend1:=EEProm.Readlong(R15)-EEProm.Readlong(R14)
          160001..170000:Trend1:=EEProm.Readlong(R16)-EEProm.Readlong(R15)
          170001..180000:Trend1:=EEProm.Readlong(R17)-EEProm.Readlong(R16)
          180001..190000:Trend1:=EEProm.Readlong(R18)-EEProm.Readlong(R17)
          190001..200000:Trend1:=EEProm.Readlong(R19)-EEProm.Readlong(R18)
          200001..210000:Trend1:=EEProm.Readlong(R20)-EEProm.Readlong(R19)  
          210001..220000:Trend1:=EEProm.Readlong(R21)-EEProm.Readlong(R20)
          220001..230000:Trend1:=EEProm.Readlong(R22)-EEProm.Readlong(R21)
          230001..000000:Trend1:=EEProm.Readlong(R23)-EEProm.Readlong(R22)
   
        Case time_long
          000001..010000:Trendi:=p-EEProm.Readlong(R23)
          010001..020000:Trendi:=p-EEProm.Readlong(R0 )
          020001..030000:Trendi:=p-EEProm.Readlong(R1 )
          030001..040000:Trendi:=p-EEProm.Readlong(R2 )
          040001..050000:Trendi:=p-EEProm.Readlong(R3 )  
          050001..060000:Trendi:=p-EEProm.Readlong(R4 )
          060001..070000:Trendi:=p-EEProm.Readlong(R5 )
          070001..080000:Trendi:=p-EEProm.Readlong(R6 )
          080001..090000:Trendi:=p-EEProm.Readlong(R7 )
          090001..100000:Trendi:=p-EEProm.Readlong(R8 )
          100001..110000:Trendi:=p-EEProm.Readlong(R9 )
          110001..120000:Trendi:=p-EEProm.Readlong(R10)
          120001..130000:Trendi:=p-EEProm.Readlong(R11)  
          130001..140000:Trendi:=p-EEProm.Readlong(R12)
          140001..150000:Trendi:=p-EEProm.Readlong(R13)
          150001..160000:Trendi:=p-EEProm.Readlong(R14)
          160001..170000:Trendi:=p-EEProm.Readlong(R15)
          170001..180000:Trendi:=p-EEProm.Readlong(R16)
          180001..190000:Trendi:=p-EEProm.Readlong(R17)
          190001..200000:Trendi:=p-EEProm.Readlong(R18)
          200001..210000:Trendi:=p-EEProm.Readlong(R19)  
          210001..220000:Trendi:=p-EEProm.Readlong(R20)
          220001..230000:Trendi:=p-EEProm.Readlong(R21)
          230001..000000:Trendi:=p-EEProm.Readlong(R22)
          
    'Write trends to EEPROM so preserved in case of power down      
      EEPROM.WriteLong(T3,Trend3)
      EEPROM.WriteLong(T1,Trend1)
      EEPROM.WriteLong(Ti,Trendi)

      Display    

Pub Display | sp, cm, p

      p  := bar.average_press
      
      cm := ALTITUDE * 100
             
      sp := bar.sealevel_press(p, cm)

      DEBUG.cls
                              
      DEBUG.str(string("QNH :")) 'Print sea-level pressure heading.

      DEBUG.str(bar.formatn(sp, bar#MILLIBARS | bar#CECR, 6))   ' Print sea-level pressure in millibars, clear-to-end, and CR.

      DEBUG.nl
      
         'Print medium-term trend narrative
        Debug.Str(string("3hr :"))
        
        Case EEPROM.Readlong(T3)
          -9999..-600:Debug.Str(string("Very rapid fall"))
          -599..-360 :Debug.Str(string("Falling rapidly"))
          -359..-160 :Debug.Str(string("Falling"))
          -159..-10  :Debug.Str(string("Falling slowly"))
          -9..9      :Debug.Str(string("Steady"))
          10..159    :Debug.Str(string("Rising slowly"))
          160..359   :Debug.Str(string("Rising"))
          360..600   :Debug.Str(string("Rising rapidly"))
          600..9999  :Debug.Str(string("Very rapid rise"))
          Other      :Debug.Str(string("Error"))
          
        DEBUG.nl
        
         'Print short-term trend narrative
        Debug.Str(string("1hr :"))
        
        Case EEPROM.Readlong(T1)
          -9999..-600:Debug.Str(string("Very rapid fall"))
          -599..-360 :Debug.Str(string("Falling rapidly"))
          -359..-160 :Debug.Str(string("Falling"))
          -159..-10  :Debug.Str(string("Falling slowly"))
          -9..9      :Debug.Str(string("Steady"))
          10..159    :Debug.Str(string("Rising slowly"))
          160..359   :Debug.Str(string("Rising"))
          360..600   :Debug.Str(string("Rising rapidly"))
          600..9999  :Debug.Str(string("Very rapid rise"))
          Other      :Debug.Str(string("Error"))
          
        DEBUG.nl
        
          'Print instantaneous trend narrative
        Debug.Str(string("inst:"))
        
        Case EEPROM.Readlong(Ti)
          -9999..-600:Debug.Str(string("Very rapid fall"))
          -599..-360 :Debug.Str(string("Falling rapidly"))
          -359..-160 :Debug.Str(string("Falling"))
          -159..-10  :Debug.Str(string("Falling slowly"))
          -9..9      :Debug.Str(string("Steady"))
          10..159    :Debug.Str(string("Rising slowly"))
          160..359   :Debug.Str(string("Rising"))
          360..600   :Debug.Str(string("Rising rapidly"))
          600..9999  :Debug.Str(string("Very rapid rise"))
          Other      :Debug.Str(string("Error"))
          
Pri Initialise_EEPROM | p

 'Fill EEPROM with default data
  'if not already filled
  
  p := bar.average_press

  If EEPROM.Readlong(Flag)<>$5555 'check fill flag
  
    EEPROM.WriteLong(R0,  p) 
    EEPROM.WriteLong(R1,  p)
    EEPROM.WriteLong(R2,  p)
    EEPROM.WriteLong(R3,  p)
    EEPROM.WriteLong(R4,  p)
    EEPROM.WriteLong(R5,  p)
    EEPROM.WriteLong(R6,  p)
    EEPROM.WriteLong(R7,  p)
    EEPROM.WriteLong(R8,  p) 
    EEPROM.WriteLong(R9,  p)
    EEPROM.WriteLong(R10, p)
    EEPROM.WriteLong(R11, p)
    EEPROM.WriteLong(R12, p)
    EEPROM.WriteLong(R13, p)
    EEPROM.WriteLong(R14, p)
    EEPROM.WriteLong(R15, p)
    EEPROM.WriteLong(R16, p) 
    EEPROM.WriteLong(R17, p)
    EEPROM.WriteLong(R18, p)
    EEPROM.WriteLong(R19, p)
    EEPROM.WriteLong(R20, p)
    EEPROM.WriteLong(R21, p)
    EEPROM.WriteLong(R22, p)
    EEPROM.WriteLong(R23, p)
    EEPROM.WriteLong(Flag,$5555)  
  
  'get starting minutes and store
  rtc.readTime( @hour, @minute, @second )
  
  EEPROM.WriteLong(Time, minute)

Erik Friesen · 2012-06-04 05:18

It would be shorter to use loops. For example,

EEPROM.WriteLong(R0, p)
EEPROM.WriteLong(R1, p)
EEPROM.WriteLong(R2, p)

could be something like

repeat a from 0 to ?? 'spin syntax a little rusty here
EEPROM.WriteLong(($9003+a), p)

rosco_pc · 2012-06-04 06:18

Just use, also untested, but I think I have the logic right

EEPROM.WriteLoNg(R0+hour*4)
Trend3 := EEPROM.ReadLong(R0+hour*4) - EEPROM.ReadLong(R0 +((hour+21)//24)*4)
Trend1 := EEPROM.ReadLong(R0+hour*4) - EEPROM.ReadLong(R0 +((hour+23)//24)*4)
Trendi := EEPROM.ReadLong(R0 +((hour+23)//24)*4)

And just for argument sake I would have longs start at long boundaries, so

R0=$9000
...

Not sure if it is needed, but old habits die hard

Duane Degn · 2012-06-04 07:55

I think rosco has the right idea of using an algorithm to determine the EEPROM location.

Your algorithm ought to be able to handle any number of hours. From two to thousands (assuming you have the memeory to store the data).

You might want to include some out of bound checks if you make the number of samples easy to change (by changing a constant at the beginning of the program).

g3cwi · 2012-06-07 06:07

Hi Rosco and Duane

Just got around to implementing these changes. Thanks for the ideas.

Cheers

Richard

g3cwi · 2012-06-07 06:33

Okay, Rosco - your ideas worked fine (just one small changed needed to the Ti test). I have now used the same idea to streamline the code and there are no repeated blocks or lengthy tests now. It is running under testing now and I will see how it works over the next few hours. There is just one further part that I could do with advice on:

This is in the CON method: is there a more elegant way of defining these locations?

  'EEPROM locations for hourly observations
  R0   = $9000
  R1   = $9004
  R2   = $9008
  R3   = $900c
  R4   = $9010
  R5   = $9014
  R6   = $9018
  R7   = $901c
  R8   = $9020
  R9   = $9024
  R10  = $9028
  R11  = $902c
  R12  = $9030
  R13  = $9034
  R14  = $9038
  R15  = $903c  
  R16  = $9040
  R17  = $9044
  R18  = $9048
  R19  = $904c 
  R20  = $9050
  R21  = $9054
  R22  = $9058
  R23  = $905c
  Time = $9060

Cheers

Richard

Mike G · 2012-06-07 06:38

How about using pointer math.

GetEepromAddr(idx)
  return $9000 + (idx * 4)

g3cwi · 2012-06-07 06:55

Hi Mike

Remember that I am very dim. I guess that is a method? i.e.

Pub GetEepromAddr(idx)   
return $9000 + (idx * 4)

If so, I am guessing that you don't think that I need to define the locations in the Con block at all?

Or have I missed the point entirely?

Cheers

Richard

[Edit - I am begining to think that is exactly what you mean. There is really no need to define the locations]

g3cwi · 2012-06-07 07:10

New and more elegant code:

{A trending barometer using the Parallax
Barometer modules and a generic Real Time
Clock module.

Uses a serial 4 x 20 serial LCD to display:

- Current Pressure
- 3 hour tend (narrative)
- 1 hour tred (narrative)
- instantaneous trend (narrative)

By Richard Newstead, G3CWI, June 2012}


CON

  _clkmode      = xtal1 + pll16x  
  _xinfreq      = 5_000_000       

  ALTITUDE      = 145 'Your altitude in metres ASL
  
  'Define some EEPROM storage locations
  'Note $9000 to $905C reserved
  Time = $9060

  'Storage for trend values
  T3   = $9064
  T1   = $9068
  Ti   = $906c
  
  'Flag for initial boot detection
  Flag = $9070
 

OBJ

  bar   : "29124_altimeter"
  rtc   : "DS1302_full"
  SN    : "Simple_Numbers" 
  DEBUG : "Debug_LCD03"
  EEPROM: "I2C_ROMEngine"

Var

  byte hour, minute, second
  Long time_long,Trend3,Trend1,Trendi

PUB Start | temp 'Start everything running

  
  rtc.init( 4, 5, 6 ) 'define ports for Real Time Clock
  
  bar.start_explicit( 0, 1, true )
  bar.set_resolution( bar#highest )  'Set to highest resolution. 

  DEBUG.start( 15, 9600, 4 ) '4 lines serial LCD  
  
  EEPROM.ROMEngineStart( 29, 28, 0 )            
  Initialise_EEPROM
 
  Main
     
Pub Main | p

  DEBUG.CLS 
  DEBUG.Backlight(true)
  DEBUG.Cursor(0)
 
  Display
  
  repeat

    rtc.readTime( @hour, @minute, @second )  'read time from DS1302

    'Update everything once a minute
    IF minute<>EEPROM.ReadLong(Time)
    
      p  := bar.average_press 'Get the average pressure.
       
      EEPROM.WriteLong(Time, minute)
   
      time_long := hour * 10000 + minute * 100 + second 'Concatenate
 
        'Store hourly readings
        If minute := 0 and second :=0
          EEPROM.WriteLong($9000+hour*4,  p)
          
        'Using obs, calculate 3 hour trend and store
        Trend3 := EEPROM.ReadLong($9000+hour*4) - EEPROM.ReadLong($9000 +((hour+21)//24)*4)
        EEPROM.WriteLong(T3,Trend3)
        
        'Using obs, calculate 1 hour trend and store
        Trend1 := EEPROM.ReadLong($9000+hour*4) - EEPROM.ReadLong($9000 +((hour+23)//24)*4)   
        EEPROM.WriteLong(T1,Trend1)
        
        'Using obs, calculate instantaneous trend and store
        Trendi := p - EEPROM.ReadLong($9000 +((hour+23)//24)*4)
        EEPROM.WriteLong(Ti,Trendi)

      Display    

Pub Display | sp, cm, p

      p  := bar.average_press
      
      cm := ALTITUDE * 100
             
      sp := bar.sealevel_press(p, cm)

      DEBUG.cls
                              
      DEBUG.str(string("QNH :")) 'Print sea-level pressure heading.

      DEBUG.str(bar.formatn(sp, bar#MILLIBARS | bar#CECR, 6))   ' Print sea-level pressure in millibars, clear-to-end, and CR.

      DEBUG.nl
      
         'Print medium-term trend narrative
        Debug.Str(string("3hr :"))
        
        Trend_Test(EEPROM.ReadLong(T3))
        
         'Print short-term trend narrative
        Debug.Str(string("1hr :"))
        
        Trend_Test(EEPROM.ReadLong(T1)) 
        
          'Print instantaneous trend narrative
        Debug.Str(string("inst:"))
        
        Trend_Test(EEPROM.ReadLong(Ti)) 
          
Pri Initialise_EEPROM | p, Loc

 'Fill EEPROM with default data
  'if not already filled
  
  p := bar.average_press

  If EEPROM.Readlong(Flag)<>$5555 'check fill flag
  
     Repeat Loc from $9000 to $905c step 4
        EEPROM.WriteLong (Loc, p)
  
    EEPROM.WriteLong(Flag,$5555)  
  
  'get starting minutes and store
  rtc.readTime( @hour, @minute, @second )
  EEPROM.WriteLong(Time, minute)

Pri Trend_Test (Trend)

  Case Trend
      -9999..-600:Debug.Str(string("Very rapid fall"))
      -599..-360 :Debug.Str(string("Falling rapidly"))
      -359..-160 :Debug.Str(string("Falling"))
      -159..-10  :Debug.Str(string("Falling slowly"))
      -9..9      :Debug.Str(string("Steady"))
      10..159    :Debug.Str(string("Rising slowly"))
      160..359   :Debug.Str(string("Rising"))
      360..600   :Debug.Str(string("Rising rapidly"))
      600..9999  :Debug.Str(string("Very rapid rise"))
      Other      :Debug.Str(string("Error"))

  Debug.nl

Thanks for all the help - this has been a very useful learning exercise.

Added to OBEX:http://obex.parallax.com/objects/868/[URL="http://obex.parallax.com/objects/868/]"]
[/URL]

Cheers

Richard

rosco_pc · 2012-06-07 14:19

Good that it worked

You can make some optimizations, limiting the number of reads/writes from/to the EEPROM:
1) Read the needed values first, and then calculate the trends.
2) Trend1 and Trend3 only need to be re-calculated and written when you write the new hourly value.

like this:

        'Using obs, calculate instantaneous trend and store
	hour1 := EEPROM.ReadLong($9000 +((hour+23)//24)*4)   
        EEPROM.WriteLong(Ti,p-hour1)
        If minute := 0 and second :=0
		EEPROM.WriteLong($9000+hour*4,  p)
          
		'Using obs, calculate 3 hour trend and store
		hour3  := EEPROM.ReadLong($9000 +((hour+21)//24)*4)
		EEPROM.WriteLong(T3,p-hour3)
		
		'Using obs, calculate 1 hour trend and store
		EEPROM.WriteLong(T1,p-hour1)

Better ways to write repeated code

Comments