|< in Spin

Erlend

I thought I had it figured out:

IF INA[PINmiso]== 1   
        Framerx |= |<bitpos

-that this would put a '1' into Framerx at the bit postion given by the number held by the variable 'bitpos'?

Erlend

Phil Pilgrim (PhiPi)

That would be my interpretation. Are you seeing something different?

-Phil

Erlend

Thanks, then I know I can trust that one is correct, so now I can concentrate on debugging elsewhere. So far I have found the classical 'forgot to clear all bits in a fresh variable' mistake. I'll nail it down.

Erlend

Erlend

Nailed down.
This is the core method to do SPI, as part of my SPI protocol object:

PUB Transfer(FRAMEtx) | FRAMErx, bitpos    
                                                                                                                                                                     
   FRAMErx:= 0                                                       'clear all bits in input buffer                                                                 
                                                                                                                                                                     
   REPEAT bitpos FROM iFirstPos TO iLastPos                          'follow the direction set by choice of MSB first or LSB first                                   
     OUTA[PINmosi]:= (FRAMEtx & |<bitpos)>0                          'if the bit at at that postion is one, output 1                                                                
     WAITCNT((iDelay*uSec) + cnt)                                    'allow the slave time to react                                                                  
     OUTA[PINclk]:= iActiveCLK                                       'leading edge of clock (whether Active is 1 or 0 is defined by SPI mode)
     
     IF iPhaseCLK== 0                                                'if acting on leading edge read the input now:
       IF INA[PINmiso]== 1                                           'if incoming data bit is 1                                                                      
         Framerx |= |<bitpos                                         'set bit in the buffer at respective position to 1, otherwise leavi it at 0
         
     WAITCNT((iDelay*uSec) + cnt)                                    'allow the slave time to react    
     OUTA[PINclk]:= iIdleCLK                                         'trailing edge of clock (whether Idle is 1 or 0 is defined by SPI mode)
     
     IF iPhaseCLK== 1                                                'if acting on trailing edge read the input now:
       IF INA[PINmiso]== 1                                           'if incoming data bit is 1                                                                      
         Framerx |= |<bitpos                                         'set bit in the buffer at respective position to 1, otherwise leave it at 0     
       
   RETURN FRAMErx

-and the bug was that

OUTA[PINmosi]:= (FRAMEtx & |<bitpos)>0

at first was written as:

OUTA[PINmosi]:= FRAMEtx & |<bitpos

Now it all works fine.

Erlend

JasonDorie

That code might run faster if it was done like this:

OUTA[PINmosi] := (FRAMEtx >> bitpos) & 1

You can likely even drop the "& 1" from the end.

In your version, you're ANDing FRAMEtx and (1 << bitpos) together, then testing if the result is > 0, and setting a bit in OUTA based on that. (four operations total - the shift, the AND, the test, and the set)

In my version, FRAMEtex is shifted down so the desired bit is now at bit zero, then you're ANDing with a constant (1). The result doesn't have to be tested - you can just set the result. (three operations total - the shift, the AND, then the set). I believe the "& 1" is optional because in Spin, setting bit with a statement like "OUTA[pin] := x" only uses the low bit of x. Test it to be sure, but I think that's correct. If it is, your overall transfer rate will improve.

Jason

Erlend

JasonDorie wrote: »

That code might run faster if it was done like this:

OUTA[PINmosi] := (FRAMEtx >> bitpos) & 1

You can likely even drop the "& 1" from the end.

In your version, you're ANDing FRAMEtx and (1 << bitpos) together, then testing if the result is > 0, and setting a bit in OUTA based on that. (four operations total - the shift, the AND, the test, and the set)

In my version, FRAMEtex is shifted down so the desired bit is now at bit zero, then you're ANDing with a constant (1). The result doesn't have to be tested - you can just set the result. (three operations total - the shift, the AND, then the set). I believe the "& 1" is optional because in Spin, setting bit with a statement like "OUTA[pin] := x" only uses the low bit of x. Test it to be sure, but I think that's correct. If it is, your overall transfer rate will improve.

Jason

Thanks!
I will implement this change.
I am curious if '1<<bitpos' is the exact same execution as '|<bitpos'? I understand the outcome is the same, but is the |< more efficient?

Erlend

Phil Pilgrim (PhiPi)

Erlend wrote:

... If it is more efficient I should use it, right?

Even if there is a difference (which I doubt), at some point readability has to trump efficiency. I never really liked the |< operator and always use 1 << instead. It's just clearer to the non-Spin initiate what's meant by the expression.

-Phil

JonnyMac

I never really liked the |< operator and always use 1 << instead.

Me too. It turns out that the Spin operator is faster than the conventional code, but the conventional code is easier for those that are not yet proficient in Spin.

Erlend

JonnyMac wrote: »

Me too. It turns out that the Spin operator is faster than the conventional code, but the conventional code is easier for those that are not yet proficient in Spin.

That's a good point. I do think readability of the code is important.

Erlend