clumsy code

toxicmouse

Senior Member
I am trying to clean up the code below to make it less clumsy, any ideas appreciated. I am using a 40x2.


symbol PIR1= B.1
symbol PIR2= B.2
symbol PIR3= B.3
symbol PIR4= B.4
symbol PIR5= B.5
symbol PIR6= B.6
symbol PIR7= B.7

main:
gosub check_pir

{some other code in here to process the results}
goto main


check_pir: 'if PIR activated then pirflag = 1 and pir_activated = the id number of the activated PIR
b7 = PIR1
if b7 = 1 then goto check_pir_PIR1
b7 = PIR2
if b7 = 1 then goto check_pir_PIR2
b7 = PIR3
if b7 = 1 then goto check_pir_PIR3
b7 = PIR4
if b7 = 1 then goto check_pir_PIR4
b7 = PIR5
if b7 = 1 then goto check_pir_PIR5
b7 = PIR6
if b7 = 1 then goto check_pir_PIR6
b7 = PIR7
if b7 = 1 then goto check_pir_PIR7
return 'this is only if none of the PIRs have been activated.

check_pir_PIR1:
pirflag = 1
pir_activated =1
return 'return to main programme
check_pir_PIR2:
pirflag = 1
pir_activated = 2
return 'return to main programme
check_pir_PIR3:
pirflag = 1
pir_activated = 3
return 'return to main programme
check_pir_PIR4:
pirflag = 1
pir_activated = 4
return 'return to main programme
check_pir_PIR5:
pirflag = 1
pir_activated =5
return 'return to main programme
check_pir_PIR6:
pirflag = 1
pir_activated = 6
return 'return to main programme
check_pir_PIR7:
pirflag = 1
pir_activated = 7
return 'return to main programme
 

nick12ab

Senior Member
When checking the PIRs, you don't need to use that variable. Simply use the pin variable (or symbol definition for a pin variable) in an IF statement like this:

Code:
if pir1 = 1 then
.....
You also need to define PIR1..7 as pin variables, not pin constants. Instead of using 'symbol PIR1 = B.1', you should use 'symbol PIR1 = pinB.1'.

When are the 'check_pir_PIRX' subroutines called? It's likely that they could be replaced by a very small bit of code but that can't be confirmed without knowing this.
 

oracacle

Senior Member
it would help if the code had some form of formating, something like this, some more info about what you are trying to achieve may also help

Code:
[color=Blue]symbol [/color][color=Black]PIR1[/color][color=DarkCyan]= [/color][color=Blue]B.1
 symbol [/color][color=Black]PIR2[/color][color=DarkCyan]= [/color][color=Blue]B.2
 symbol [/color][color=Black]PIR3[/color][color=DarkCyan]= [/color][color=Blue]B.3
 symbol [/color][color=Black]PIR4[/color][color=DarkCyan]= [/color][color=Blue]B.4
 symbol [/color][color=Black]PIR5[/color][color=DarkCyan]= [/color][color=Blue]B.5
 symbol [/color][color=Black]PIR6[/color][color=DarkCyan]= [/color][color=Blue]B.6
 symbol [/color][color=Black]PIR7[/color][color=DarkCyan]= [/color][color=Blue]B.7
 [/color]
[color=Black]main:
      [/color][color=Blue]gosub [/color][color=Black]check_pir
 [/color]
[color=Gray]{[/color][color=Black]some other code in here [/color][color=Blue]to [/color][color=Black]process the results[/color][color=Gray]}
      [/color][color=Blue]goto [/color][color=Black]main
 

check_pir:              [/color][color=Green]'if PIR activated then pirflag = 1 and pir_activated = the id number of the activated PIR
      [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Black]PIR1
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR1
      [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Black]PIR2
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR2
      [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Black]PIR3
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR3
      [/color][color=Navy]7 [/color][color=DarkCyan]= [/color][color=Black]PIR4
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR4
      [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Black]PIR5
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR5
      [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Black]PIR6
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR6
      [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Black]PIR7
      [/color][color=Blue]if [/color][color=Purple]b7 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then goto [/color][color=Black]check_pir_PIR7
      [/color][color=Blue]return            [/color][color=Green]'this is only if none of the PIRs have been activated.
 [/color]
[color=Black]check_pir_PIR1: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]=[/color][color=Navy]1 
      [/color][color=Blue]return            [/color][color=Green]'return to main programme[/color]
[color=Black]check_pir_PIR2: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]= [/color][color=Navy]2
      [/color][color=Blue]return            [/color][color=Green]'return to main programme[/color]
[color=Black]check_pir_PIR3: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]= [/color][color=Navy]3
      [/color][color=Blue]return            [/color][color=Green]'return to main programme[/color]
[color=Black]check_pir_PIR4: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]= [/color][color=Navy]4
      [/color][color=Blue]return            [/color][color=Green]'return to main programme[/color]
[color=Black]check_pir_PIR5: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]=[/color][color=Navy]5 
      [/color][color=Blue]return            [/color][color=Green]'return to main programme[/color]
[color=Black]check_pir_PIR6: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]= [/color][color=Navy]6
      [/color][color=Blue]return            [/color][color=Green]'return to main programme[/color]
[color=Black]check_pir_PIR7: 
      pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]= [/color][color=Navy]7
      [/color][color=Blue]return            [/color][color=Green]'return to main programme [/color]
 

oracacle

Senior Member
had a quick think, you dont really need to call another sub to set the flags, i havent been able to test this in simulator as yet but a cant see any reason why it shouldn't unless it will goes and if-statement stack error (it willl return to main programe before the end if is called)

Code:
[color=Black]main:

      [/color][color=Blue]gosub [/color][color=Black]check_pir
      [/color][color=Green]'do stuff
      [/color][color=Blue]goto [/color][color=Black]main
      
      
check_pir:              [/color][color=Green]'if PIR activated then pirflag = 1 and pir_activated = the id number of the activated PIR
      [/color][color=Blue]let [/color][color=Purple]b0 [/color][color=DarkCyan]= [/color][color=Purple]pinsb
      [/color][color=Blue]if [/color][color=Purple]bit0 [/color][color=DarkCyan]= [/color][color=Navy]1 [/color][color=Blue]then
            [/color][color=Black]pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Black]pir_activated [/color][color=DarkCyan]=[/color][color=Navy]1 
      [/color][color=Blue]return 
      end if[/color]
dunno what you are doing with b.0, but this could work

Code:
[color=Blue]symbol [/color][color=Purple]pir_activeted    [/color][color=DarkCyan]= [/color][color=Purple]b1[/color]
[color=Blue]symbol [/color][color=Purple]pirflag          [/color][color=DarkCyan]= [/color][color=Purple]b2[/color]
[color=Black]main:

      [/color][color=Blue]gosub [/color][color=Black]check_pir
      [/color][color=Green]'do stuff
      [/color][color=Blue]goto [/color][color=Black]main
      
      
check_pir:              [/color][color=Green]'if PIR activated then pirflag = 1 and pir_activated = the id number of the activated PIR
      [/color][color=Blue]let [/color][color=Purple]b0 [/color][color=DarkCyan]= [/color][color=Purple]pinsb    [/color][color=Green]'read value of portb into variable
      
      [/color][color=Blue]lookup [/color][color=Purple]b0[/color][color=Black], [/color][color=Blue]([/color][color=Navy]0[/color][color=Black],[/color][color=Navy]1[/color][color=Black],[/color][color=Navy]2[/color][color=Black],[/color][color=Navy]4[/color][color=Black],[/color][color=Navy]8[/color][color=Black],[/color][color=Navy]16[/color][color=Black],[/color][color=Navy]32[/color][color=Black],[/color][color=Navy]64[/color][color=Black],[/color][color=Navy]255[/color][color=Blue])[/color][color=Black],[/color][color=Purple]pir_activeted
      [/color][color=Green]'now match number and put postition into pir activated, value will be 0 - 7
      'if b.0 is presume as something else, 0 in pir_activated will indacte no activation
      
      [/color][color=Blue]if [/color][color=Purple]pir_activeted [/color][color=DarkCyan]> [/color][color=Navy]0 [/color][color=Blue]then
            [/color][color=Purple]pirflag [/color][color=DarkCyan]= [/color][color=Navy]1
      [/color][color=Blue]end if 
      return[/color]
 
Last edited:

hippy

Technical Support
Staff member
There are a couple of minor typo bugs in oracacle's code and a potential issue if more than one PIR activates at the same time. That can however be fixed by doing a LOOKDOWN to find which is the first bit set in the 'b0' variable ...

Code:
#Picaxe 40X2

Symbol pirflag       = b1
Symbol pir_activated = b2

Do           ;  76543210
  b0 = pinsB & %11111110
  If b0 <> 0 Then
    pirflag = 1
    LookDown 1, (bit0,bit1,bit2,bit3,bit4,bit5,bit6,bit7), pir_activated
  End If
Loop
The 40X2 also has an NCD unary operator which can replace the LOOKDOWN ...

Code:
#Picaxe 40X2

Symbol pirflag       = b1
Symbol pir_activated = b2

Do           ;  76543210
  b0 = pinsB & %11111110
  If b0 <> 0 Then
    pirflag = 1
    pir_activated = NCD b0 - 1
  End If
Loop
The only issue with that is NCD will detect the PIR on B.7 first, detect B.1 last. That probably is not a problem and the LOOKDOWN code can be used if it is.
 

oracacle

Senior Member
glad you spotted those hippy, i wasnt sure how to over come them after i notced about 5 minuites after i posted, then hte phone wrang i i got distracted
 

toxicmouse

Senior Member
Thanks all. That helps a lot.

@hippy - that is very compact and logical, thank you

@nick12ab- I must have missed the part in the user manual where it mentions pin variables. Thanks for that.
 
Top