# unhappy with code I wrote. It works BUT??

Status
Not open for further replies.

#### MrDEB

##### Well-Known Member
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

#### Ian Rogers

##### User Extraordinaire
Forum Supporter
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

#### Dr_Doggy

##### Well-Known Member
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

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...

#### MrDEB

##### Well-Known Member
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.

#### Ian Rogers

##### User Extraordinaire
Forum Supporter
The other way is to create a state machine

Code:
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:

#### Ian Rogers

##### User Extraordinaire
Forum Supporter
if you create an old and new combination then debouncing isn't necessary..

Code:
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

#### Dr_Doggy

##### Well-Known Member

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:

#### JonSea

##### Well-Known Member
Swordfish supports Not.

Led = Not (Led)

I believe the parentheses are needed.

Swordfish also supports Toggle.

Toggle(Led)

will change its state.

#### MrDEB

##### Well-Known Member
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.

#### MrDEB

##### Well-Known Member
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.

#### Ian Rogers

##### User Extraordinaire
Forum Supporter
if swt = 0 then
led = !led
delayms(500)
endif
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..

#### Dr_Doggy

##### Well-Known Member
sry got carried away elaborating how to simplify if's!

#### Pommie

##### Well-Known Member
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.

#### MrDEB

##### Well-Known Member
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

#### Dr_Doggy

##### Well-Known Member
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

#### JonSea

##### Well-Known Member
Sigh. Mike gave you a super easy, eloquent way of doing this. Perhaps he knows a thing or two....

#### MrDEB

##### Well-Known Member
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??

Mike = Pommie

#### MrDEB

##### Well-Known Member
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

Status
Not open for further replies.