Should I depend on Locals? (Spin coding)

Erlend

Often the Objects I write need quite a number of parameters. In the object method itself the values of the parameters from the parent caller need to be distributed to the actual PUBs and PRIVs to be acted upon. This can be done by passing the parameters on (and on, and on...), or by copying the parameter values into Local Variables for 'everyone' to share. I am uncertain to what is the best approach? For the enclosed code I started out with passing on the parameters, but the result was that a lot of the PUBs and PRIVs included more or less the same endless list of parameters, e.g. PUB doit (<endless list of param>). This is what will happen when different methods do different things to more or less the same set of parameters. So, I rewrote the code using the other approach - to copy parameter values into local varables early on, so that the PUBs and PRIVs would not need to have them passed as parameters. The code is easier to read now, but-- Is this a good approach? What's the snag?

----snip----

PUB Start(PINpump, PINignt, PINservo, PINfreqCount{NEW}, ptrTempWater, ptrPressWater, ptrFlameRate, ptrLevelWater, ptrOpmode, iSPtemp, iSPpress, iSPtime, iSPvol): Success
    Stop
    Success:= (cog:= COGNEW(HotWcontrol(PINpump, PINignt, PINservo, PINfreqCount{NEW}, ptrTempWater, ptrPressWater, ptrFlameRate, ptrLevelWater, ptrOpmode, iSPtemp, iSPpress, iSPtime, iSPvol), @HWCstack) +1)


PUB Stop
   IF Cog
      COGSTOP(cog~ - 1)

        
PUB HotWcontrol(PINpump, PINignt, PINservo, PINfreqCount{NEW}, ptrTempWater, ptrPressWater, ptrFlameRate, ptrLevelWater, ptrOpmode, iSPtemp, iSPpress, iSPtime, iSPvol)
  
    LptrTempWater:=   ptrTempWater                'Make life easier by copying into local public variables
    LptrPressWater:=  ptrPressWater
    LptrFlameRate:=   ptrFlameRate 
    LptrLevelWater:=  ptrLevelWater
    LptrOpmode:=      ptrOpmode      
    LPINpump:=        PINpump
    LPINignt:=        PINignt
    LPINservo:=       PINservo
    LPINfreqCount:=   PINfreqCount
    LiSPtemp:=        iSPtemp
    LiSPpress:=       iSPpress               
    LiSPtime:=        iSPtime
    LiSPvol:=         iSPvol

    ReadGlobals
    
    CASE iOpmode
      0: Shutdown
      1: HWcontrol
      2: Time
      3: Warm
      4: Cold
      5: Pon
      6: Poff
      7: Pilot
     OTHER:
         Shutdown
                               

PRI Shutdown                             'Shutdown pump, close servo, shut off igniter float outputs, COGSTOP
    OUTA[LPINpump]:= 0
    DIRA[LPINpump]~
    ServoPos(0)
    OUTA[LPINservo]:= 0
    DIRA[LPINservo]~
    if cog
      COGSTOP(cog~ - 1)


PRI HWcontrol

    'Setting up the first run of 'Milestones' as CNT values
    '------------------------------------------------------------------------------------------------------------------------------------------------
    iServoRate := 20*mSec                               'Servo every 20ms
    iServoPuls := 2200*uSec  {0%}                       'Servo Pos 1000..2000uS nominal pulse length    ** will be recalculated by control algorithm
    iServoPulsStart := CNT + iServoRate                 'Define 'milestone' as CNT value
    iServoPulsEnd :=  iServoPulsStart + iServoPuls      'Define 'milestone' as CNT value

    iPumpRate := 100*mSec    {10Hz}                     'Pump Freq 5..60Hz = 200mS..16mS periode        ** will be recalculated by control algorithm
    iPumpPuls := 10*mSec                                'Pump pulse length 10mS                                                                                     
    iPumpPulsStart := CNT + iPumpRate                   'Define 'milestone' as CNT value
    iPumpPulsEnd := iPumpPulsStart + iPumpPuls               'Define 'milestone' as CNT value

    iFreqCountIval:= 500*mSec                            'Read, store and reset counter every 500mS
    iFreqCountNow:=  CNT + iFreqCountIval               'Define 'milestone' as CNT value

    iAlgorithmIval:= 1000*mSec                           'Run control algorithm /sec and update values
    iAlgoritmNow:= CNT +  iAlgorithmIval                'Define 'milestone' as CNT value
   '-------------------------------------------------------------------------------------------------------------------------------------------------

    DIRA[LPINservo]:= 1                                  'Define as outputs
    DIRA[LPINpump]:= 1

    iPumpStrokes:= 0                                    'Set volume counter to zero

   'Configure the counter and do a first frequency count to measure flame rate 
   '------------------------------------------------------------------------------------------------------------------------------------------------- 
    CTRA:= %01010 << 26 + LPINfreqCount                 'Set up counter to POSEDGE mode (bit26-30, using pin LPINfreqCount (bit 0-5), and
    FRQA:= 1                                            'using one count per edge to accumulate into PHSA
    WAITCNT(clkfreq/2 + cnt)
    FreqCount
    IF iFlameRate < CiMinFlame                          'Light the flame if not burning
      Ignite(1)

   'Wait with starting pump until initial heating of water is finished
   '-------------------------------------------------------------------------------------------------------------------------------------------------
    iTempWater:=  LONG[LptrTempWater]                                         
    REPEAT UNTIL iTempWater > LiSPtemp                  'Wait until water is hot enough for brewing
      iTempWater:=  LONG[LptrTempWater]                 'Update temperature measurements data

   'Now start 'milestone' based hot water control routines   
   '--------------------------------------------------------------------------------------------------------------------------------------------------
    REPEAT UNTIL iPumpStrokes > LiSPvol                 'Continously check CNT to detect 'milestones' and perform timed actions - until setpoint volume

    'Do the Servo
      IF (CNT - iServoPulsStart) > 0                    'If the iServoPulsStart milestone has been reached
        OUTA[LPINservo]:= 1                              'do what has to be done,
        iServoPulsStart+= iServoRate                    'then set up the next time milestone

      IF (CNT - iServoPulsEnd) > 0
        OUTA[LPINservo]:= 0
        iServoPulsEnd:=  iServoPulsStart + iServoPuls

    'Do the pump
      IF (CNT - iPumpPulsStart) > 0
        OUTA[LPINpump]:= 1
        iPumpPulsStart += iPumpRate
        iPumpStrokes += 1                                'Keep tally of total pump strokes (=volume)

      IF (CNT - iPumpPulsEnd) > 0
        OUTA[LPINpump]:= 0
        iPumpPulsEnd:= iPumpPulsStart + iPumpPuls
         
    'Do the frequency counter
      IF (CNT - iFreqCountNow) > 0
        FreqCount                                       
        iFreqCountNow += iFreqCountIval

    'Do the control algorithm
      IF (CNT - iAlgoritmNow) > 0                       '!! PRI needed 
        ControlAlgorithm
        iAlgoritmNow += iAlgorithmIval    

----snip----


'--------------------- supporting subs ----------------------


PRI Ignite(Sec)                    'Method to ignite gas burner
    DIRA[LPINignt]~~                                'Set I/O pin to output direction
    OUTA[LPINignt]:= 1                              'Energize igniter
    WAITCNT(clkfreq + cnt)                          'Warm up igniter tip
    ServoPos(950)                                   'Open gas valve fully
    WAITCNT(Sec*clkfreq + cnt)                      'Let glow for Sec
    OUTA[LPINignt]:= 0                              'Switch off Ignt
    DIRA[LPINignt]~                                 'Set I/O pin to input, ie. float
    ServoPos(1500)                                  'Set gas valve low burn 
    
PRI ServoPos(position)              'Method to run a servo just long enough, then let it free, assuming the friction of the valves prevents movement
    DIRA[LPINservo]~~                               'Set I/O pin to output direction
    repeat 25                                       '1/2 S to allow servo to move to new pos
      WAITCNT(clkfreq/50 + cnt)                     '50 hz update frequency
      OUTA[LPINservo]:= 1                           'Pulse high
      WAITCNT(position*uSec + cnt)                  'Servo pulse duration 0% eq 2220 uS, 100% eq 950mS 
      OUTA[LPINservo]:= 0                           'Puse low
    DIRA[LPINservo]~                                'Set I/O pin to input, ie. float


PRI ReadGlobals                     'Method to copy values over from global variables ptrTempWater, ptrPressWater, ptrFlameRate, ptrLevelWater, ptropmode
    iTempWater:=  LONG[LptrTempWater]
    iPressWater:= LONG[LptrPressWater]
    iFlameRate:=  LONG[LptrFlameRate]
    iLevelWater:= LONG[LptrLevelWater]
    iOpmode:=     LONG[LptrOpmode]    
 

PRI FreqCount
    iFreqUV:= PHSA~                                'Capture counter accumulation and then post-zero
    iFlamerate:= LONG[LptrFlameRate]:= iFreqUV * 2  'Calculate real frequency (read 2/Sec) and store in local and global variable

PRI ControlAlgorithm  
    IF  iOpmode== 0                                   'Check for stop command from parent
       Shutdown
    IF  iFlameRate < CiMinFlame                      'Flameout shutdown
       Shutdown  
 ----snip----

By the way, this is code under construction based on a tip from @Ariba of how to 'multitask' within one Cog.

Erlend

JonnyMac

When I have object methods that require a lot of parameters, I tend to pass a pointer to an array. I put the actual parameters into a DAT list, and then pass that address.

dat

  Setup       long      1, 0, 1


pub main

  myobject.start(@Setup)

Erlend

JonnyMac wrote: »

When I have object methods that require a lot of parameters, I tend to pass a pointer to an array. I put the actual parameters into a DAT list, and then pass that address.

Yes, I have done this on a few occasions (actually, I think I learned it from you), but I prefer to do this only when the parameters belong together as a set, sort of, and do not need to be individually named. One example I have done is a set of RFID access codes that is defined as an array in DAT and then passed as a single (arrray) parameter to the RFID read&check Object method.
Erlend

SRLM

You have an interesting case that is a bit outside of the "normal" object oriented programming paradigm. Traditional coding style will tell you to initialize your object with the parameters needed and add getters/setters if you need to adjust those initial values later. Then the individual class methods will have the call specific values needed, and the method will pull the instance values as needed.

Your case (and much of Spin) is different in that you initialize the object with pointers to other variables, then you throw the object into a parallel cog where it watches those values. After the initialization call other functions never call on this object, which is unique in the programming world.

So, yes, I think you're doing it the best way that you can at this point.

On a personal preference, when copying values from method parameters to object instance values I like to give the parameter name a trailing underscore, and the instance value the root name. Eg ptrTempWater_ and ptrTempWater.

Dave Hein

I have a modified version of FSRW that supports multiple instances by passing a pointer to a file structure. The new methods are prefix with the letter "h", such as hopen, hread and hwrite. These methods use LONGMOVE to copy the contents of the file structure to FSRW's local variables, and then calls FSRW's original method that uses the local variables. As an example, the hread method looks like this:

pub hread(handle, buffer, num)
  loadhandle(handle)
  result := \pread(buffer, num)

The loadhandle method looks like this:

pri loadhandle(handle)
  if curr_handle <> handle
    if curr_handle
      longmove(curr_handle, @fclust, handle_longs)
    curr_handle := handle
    longmov(@fclust, curr_handle, handle_longs)

The local variables are defined in a DAT section instead of VAR section. Using this technique I can access multiple files from any object by passing a file handle, which is a pointer to the file structure.

Dave Hein

Here's my modified FSRW code in case you're interested.

EDIT: The h... methods in fsrwx.spin require a pointer to a file struct, which is 556 bytes in size. The 556 bytes include the 512-byte sector buffer plus 11 longs used to store the file information. The file structure should be initialized to all zeroes.

Erlend

SRLM wrote: »

You have an interesting case that is a bit outside of the "normal" object oriented programming paradigm. Traditional coding style will tell you to initialize your object with the parameters needed and add getters/setters if you need to adjust those initial values later. Then the individual class methods will have the call specific values needed, and the method will pull the instance values as needed.

Your case (and much of Spin) is different in that you initialize the object with pointers to other variables, then you throw the object into a parallel cog where it watches those values. After the initialization call other functions never call on this object, which is unique in the programming world.

So, yes, I think you're doing it the best way that you can at this point.

On a personal preference, when copying values from method parameters to object instance values I like to give the parameter name a trailing underscore, and the instance value the root name. Eg ptrTempWater_ and ptrTempWater.

I guess that they are not merely conventional objects, but parallel processes too, makes it not directly comparable with what's going on in other object oriented languages.
About the underscore_ syntax - is that a widely used convention or your own style only?

Erlend

Erlend

@Dave

Thanks for the code example, but I am too slow to understand how this is a problem-solving technique in my case?
Erlend

localroger

It looks like what you essentially need is a global array. Yeah we know globals are BAAAAAD and Spin tries to make us do the right thing, like forcing us to indent correctly and not offering us GOTO, but sometimes you just need those things.

There is a way around this. Create an object to service your parameters. Within this object define a DAT array which contains all your parameters. Also create a bunch of constant pointers to indexes within this array so you can refer to stuff you want by name. You can then drop this object wherever you need it as a child to other objects, and because of the DAT storage they're all accessing hte same parameter list. With SET and READ methods in the service object, you're then good to go.

I did this to implement a generic string library which doesn't require everyone using it to define buffers; the generic common buffer is of course global but it's DAT and all the instances of the string object have access to the same buffer and pointers.

Heater.

OK, I'll bite, what times do you actually need GOTO and globals?

localroger

Heater. wrote: »

OK, I'll bite, what times do you actually need GOTO and globals?

The OP needs a global array. It's the cleanest solution to his problem, which is to make a parameter list available at several different layers of abstraction within an application.

You need GOTO (and badly) when handling error conditions in a complicated application where they can occur at mutiple levels of abstraction and you want to re-enter the control loop after handling them.

There is Spin code out there riddled with all kinds of dodges dealing with the fact that these features aren't available in the edge cases where they are really needed. My string object is a good example; the PropTool won't even compile it because there are so many instances of the small handler object, almost literally as a child to every other object in the system. (BST compiles it fine.)

It's fine to say that globals and GOTO are dangerous and overused, but it's stupid to pretend that they're never the best solution to a particular problem. There really is such a thing as global data.

potatohead

One GOTO case I miss is math oriented jumps. Typically, there are two use cases:

1. Computation based on input, result of logic, or some combination serves as index into table of addresses or, in the case of PASM, likely just JMP instructions.

2. Computation, based on the same, results in JMP address directly.

These may be GOSUB type routines too, and they may do other things, like fall through, or have common exit point.

In BASIC, these are typically expressed as either: ON X GOTO 100, label, label1, 200, etc... depending on whether or not line numbers are needed

, or

GOTO X, where X references a line number, in number oriented languages.

Where there are a small number of possible branches, the lack of these isn't a big deal. Where there are a large number, such as an interpreter, emulator, etc... or a lot of logic tied to something like a character value, the cost difference is dramatic.

Of course, I think it's perfectly silly we frown on these when the CPU itself is setup to do them nice and quick. Always have.

Like UNIX, where you get a lot of power, and you can make a big mess, and that means you also get a lot of responsibility, having these sorts of constructs available means some messes will be made, but some really awesome things will be made too. Works about the same, IMHO.