Better ways to write repeated code

Comments

• Erik Friesen Posts: 1,071

  2012-06-04 05:18 edited 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 Posts: 472

  2012-06-04 06:18 edited 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 Posts: 10,588

  2012-06-04 07:55 edited 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 Posts: 262

  2012-06-07 06:07 edited 2012-06-07 06:07

  Hi Rosco and Duane
  
  Just got around to implementing these changes. Thanks for the ideas.
  
  Cheers
  
  Richard
• g3cwi Posts: 262

  2012-06-07 06:33 edited 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 Posts: 2,702

  2012-06-07 06:38 edited 2012-06-07 06:38

  How about using pointer math.
  

  GetEepromAddr(idx)
    return $9000 + (idx * 4)
  
• g3cwi Posts: 262

  2012-06-07 06:55 edited 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 Posts: 262

  2012-06-07 07:10 edited 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 Posts: 472

  2012-06-07 14:19 edited 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)