1. Welcome to our site! Electro Tech is an online community (with over 170,000 members) who enjoy talking about and building electronic circuits, projects and gadgets. To participate you need to register. Registration is free. Click here to register now.
    Dismiss Notice

unhappy with code I wrote. It works BUT??

Discussion in 'Microcontrollers' started by MrDEB, Jan 9, 2017.

  1. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19
    IMO I have tooo many IF THEN statements but this code works as is.
    can it be done better or??
    Always room for improvement.


    // My objective is to monitor 10 switches using an 18F4520 pic. This is just
    // a testing routine to determine if the sequence of events will work
    // leds are OFF until the designated switch is depressed then stay ON until the designated switch is depressed again.
    // can't help it but I feel this code could be better?? tooo many if thens??
    //my plan is to designate each switch and led (swt1 and led1)
    DEVICE = 18F2420
    CLOCK = 8
    INCLUDE"InternalOscillator.bas"
    INCLUDE "SetDigitalIO.bas"
    // alias to port pin...
    DIM LED AS PORTc.4
    dim swt as portc.3

    low (led)

    // main program...
    while true
    if swt = 1 and led = 0
    then
    led = 0
    end if

    if swt = 0 and led = 0 // button press
    then
    led = 1
    delayms(500) // switch debounce
    end if

    if led = 1 and swt = 1
    then led =1
    end if

    if led = 1 and swt = 0
    then led = 0
    delayms(500)
    led = 0
    end if

    wend
     
  2. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    8,684
    Likes:
    828
    Location:
    Rochdale UK
    There is only two conditions.... If the switch is off who cares... You only need to know the "pressed" condition.

    switch pressed and light is on > switch it off
    switch pressed and light is off > switch it on
     
  3. Dr_Doggy

    Dr_Doggy Well-Known Member

    Joined:
    Aug 11, 2007
    Messages:
    1,714
    Likes:
    37
    unfortunately if statements occur alot.. so it is common to see lots of them...
    sometimes i use if-statements and global variables to recover from problems like not enough stack space...

    something i note in the code is that if we go in to it, these if statements are redundant since led is already 0 when we go in to it so there is no point to set it to 0 when it already is:
    if swt = 1 and led = 0
    then
    led = 0
    end if

    same with this chunk....
    if swt = 1 and led = 0
    then
    led = 0
    end if


    which changes your code to:

    while true
    if swt = 0 and led = 0 // button press
    then
    led = 1
    delayms(500) // switch debounce
    end if

    if led = 1 and swt = 0
    then led = 0
    delayms(500)
    led = 0
    end if
    wend

    for this NEW example, we could also change to this next one, which would be faster since it only checks swt condition once, but requires an additional stack :

    if swt = 0 then
    if led = 0 then
    led = 1
    delayms(500)
    else
    led = 0
    delayms(500)
    endif
    endif


    any other ideas id be interested to hear as I commonly ponder similar problems...
     
  4. dave

    Dave New Member

    Joined:
    Jan 12, 1997
    Messages:
    -
    Likes:
    0


     
  5. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19

    Thanks for suggestions. I don't think I have ever seen 2 THEN statements together lie that.
    NOTE maybe I need to clarify a small bit.
    It stars out the LED is off, press the mon switch and LED comes on and stays on UNTIL switch is pressed again.
    I tried using the REPEAT UNTIL statement but got no where fast.
    This is to replace the decade counter circuit I built. Reason for change is the on/off switches were too large. Made the board profile too tall to allow seeing the dominoes from across the table playing Mexican Train.
    The new circuit is to utilize an 18F4520 (need 20 pins)10 LEDs and 10 mon tactile push-buttons.
     
  6. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    8,684
    Likes:
    828
    Location:
    Rochdale UK
    The other way is to create a state machine

    Code (psuedo):

    dim condition as byte

    condition = 0
    if switch then condition = 1
    if led then condition = condition + 2
    debounce!!!
    if condition = 1 then LED = off
    if condition = 3 then LED = on
     
     
    Last edited: Jan 9, 2017
  7. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    8,684
    Likes:
    828
    Location:
    Rochdale UK
    if you create an old and new combination then debouncing isn't necessary..

    upload_2017-1-9_23-18-9.png
    Code (psuedo):


    dim condition as byte

    loop
      condition = condition * 4 ' shift two to the left
      condition = condition and 12  ' mask the old condition
      if switch then condition = 1  ' add new switch
      if led then condition = condition + 2  ' add new led

    'now we have

      if condition = 2 then LED = on
      if condition = 13 then LED = off

     
     
    • Like Like x 1
  8. Dr_Doggy

    Dr_Doggy Well-Known Member

    Joined:
    Aug 11, 2007
    Messages:
    1,714
    Likes:
    37
    got some reading to do about this machine(another thanks!)!

    here is the same code even more simplified... it should be working as you describe... not 100% sure if its swordfish compatible(with the not "!" statement)

    if swt = 0 then
    led = !led
    delayms(500)
    endif
     
    Last edited: Jan 10, 2017
  9. JonSea

    JonSea Active Member

    Joined:
    Oct 1, 2012
    Messages:
    976
    Likes:
    74
    Location:
    Seattle, WA
    Swordfish supports Not.

    Led = Not (Led)

    I believe the parentheses are needed.

    Swordfish also supports Toggle.

    Toggle(Led)

    will change its state.
     
  10. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19
    CONDITION appears to look like the FUNCTION command??
    I considered the TOGGLE statement but it never made it into the code.
    I still like the REPEAT UNTII, seems easier.
    working on adding a FOR NEXT loop to cycle all 10 inputs.
    Am still looking at how to reduce the number of wires interconnecting the circuit board to each LED and switch.
    At present I need 4 per switch/led combo(grd, Vcc, switch input, led enable.
     
  11. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19
    I went back and reread Ians post #5 and started as an experiment using an index counter.
    LED = 1 then index = 1
    if swt = 0 then index = 0
    continue on if index = 1 then the LED is enabled. Add in the swt so if index = 2 then the led AND the swt are enabled
    basically generate a truth table
    INDEX = 0 (both led and swt are off
    INDEX = 1 (swt is closed and Led is off
    INDEX = 2 (Led on and swt is open
    INDEX = 3 (Led on and swt is closed(on)
    maybe not correct but along those lines of thought.
     
  12. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    8,684
    Likes:
    828
    Location:
    Rochdale UK
    Trouble here is... If you still have your finger on the switch, it will flash. With a state machine it will wait until you let go..

    The help I'm trying to convey is if you have 10 of these then your going to run out of lines... It'll take and age to cycle though..
     
  13. Dr_Doggy

    Dr_Doggy Well-Known Member

    Joined:
    Aug 11, 2007
    Messages:
    1,714
    Likes:
    37
    sry got carried away elaborating how to simplify if's!
     
  14. Pommie

    Pommie Well-Known Member Most Helpful Member

    Joined:
    Mar 18, 2005
    Messages:
    9,726
    Likes:
    283
    Location:
    Brisbane Australia
    Get all your switches into one 16 bit variable (current).
    Keep a copy of the previous state of the switches (previous).
    If you xor the two together it has a 1 for each switch that has changed state. (Current xor previous)
    If you AND the result of last xor with current key variable it will have a 1 for each new key press. (current xor previous) and current
    If you xor this with a variable representing the LEDs you will have the new state for the LEDs.
    Do this every 20mS for perfectly debounced switches.

    Mike.
     
    • Like Like x 2
  15. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19
    Pommie post sounds interesting but I want to keep it simple and not complicated at all.
    Plan to use a FOR NEXT loop. There is no speed involved in how fast switches are pressed. Plus I need to utilize all or part of 3 different ports.
    10 port pins for Leds and 10 port pins for switches.
    I think my code here should address both items. NOTE code is not complete. Need to add additional port bits etc.

    while true
    for x = 0 to 10

    swt(x) =portc.bits(x)
    led(x) = portb.bits(x)
    if swt(x) = 1 and led(x) = 0
    then
    led(x) = 0
    end if

    if swt(x) = 0 and led(x) = 0 // button press
    then
    led(x) = 1
    delayms(500) // switch debounce
    end if

    if led(x) = 1 and swt(x) = 1
    then led(x) =1
    end if

    if led(x) = 1 and swt(x) = 0
    then led(x) = 0
    delayms(500)
    led(x) = 0
    end if
    next
    wend
     
  16. Dr_Doggy

    Dr_Doggy Well-Known Member

    Joined:
    Aug 11, 2007
    Messages:
    1,714
    Likes:
    37
    again... these code chunks literally do nothing :

    if led(x) = 1 and swt(x) = 1
    then
    led(x) =1 /// led(x) already = 1 before this line
    end if

    and this one too:
    if swt(x) = 1 and led(x) = 0
    then
    led(x) = 0 // led(x) already = 0 before this line
    end if
     
  17. JonSea

    JonSea Active Member

    Joined:
    Oct 1, 2012
    Messages:
    976
    Likes:
    74
    Location:
    Seattle, WA
    Sigh. Mike gave you a super easy, eloquent way of doing this. Perhaps he knows a thing or two....
     
  18. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19
    I need to do a recheck of what and why I have

    if led(x) = 1 and swt(x) = 1
    then
    led(x) =1 /// led(x) already = 1 before this line
    end if

    and this one too:
    if swt(x) = 1 and led(x) = 0
    then
    led(x) = 0 // led(x) already = 0 before this line
    end if

    I recall I wanted to verify and make sure the Led stays on and recheck if it is still on before closing the switch to shut off the led.
    Will re write as per suggestion to see what happens.
    Who is Mike?? Dr doggy??
     
  19. Dr_Doggy

    Dr_Doggy Well-Known Member

    Joined:
    Aug 11, 2007
    Messages:
    1,714
    Likes:
    37
    Mike = Pommie
     
    • Informative Informative x 1
  20. Ian Rogers

    Ian Rogers Super Moderator Most Helpful Member

    Joined:
    Mar 28, 2011
    Messages:
    8,684
    Likes:
    828
    Location:
    Rochdale UK
    Yeah.... See post #13
     
  21. MrDEB

    MrDEB Active Member

    Joined:
    Apr 16, 2007
    Messages:
    4,282
    Likes:
    19
    it sounds easy enough but not familiar with XOR or how to save and deal with a 16 bit variable.
    need to do some research
     

Share This Page