Welcome to the Parallax Discussion Forums, sign-up to participate.

# HELP!! SPIN coding problem with LEDs lighting on down count.

Posts: 525
edited 2019-02-04 - 22:02:44
I feel embarrassed to say this, but for the life of me I can't figure out what it is thats screwed up in this code. All its supposed to do is make a series of 8 LED lights turn on one at a time and go back and forth.

Var
Byte Pin ' I/O pin.

PUB Toggle {Made to toggle 8 LEDs on and off in sequence from low to high and then high to low}

Pin := 16 ' Set value to first pin

dira[Pin]~~

Repeat

Repeat until !outa[Pin]++ == 23 ' Go up one pin at a time to pin 23
waitcnt(clkfreq/4 + cnt) ' Wait for delay cycles

Repeat until !outa[Pin]-- == 16 ' Go down one pin at a time to pin 16
waitcnt(clkfreq/4 + cnt) ' Wait for delay cycles

• Posts: 525
Of course the space indentations didn't show up in the post ...
• Posts: 2,559
use the C to make a code block, then the indentions show up.

You are doing it slightly wrong, here a better version
```Var
Byte Pin ' I/O pin.

PUB Toggle {Made to toggle 8 LEDs on and off in sequence from low to high and then high to low}

Pin := 16 ' Set value to first pin

Repeat

Repeat
dira[pin] := 1
outa[pin] := 1
waitcnt(clkfreq/4 + cnt) ' Wait for delay cycles
until Pin++ == 23 ' Go up one pin at a time to pin 23
Repeat
dira[pin] := 0
outa[pin] := 0
waitcnt(clkfreq/4 + cnt) ' Wait for delay cycles
until Pin-- == 16 ' Go down one pin at a time to pin 16
```

Enjoy!

Mike
I am just another Code Monkey.
A determined coder can write COBOL programs in any language. -- Author unknown.
Press any key to continue, any other key to quit

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this post are to be interpreted as described in RFC 2119.
• Posts: 525
edited 2019-02-04 - 14:37:46
Thanks Mike.

Just one question - if I alter the code slightly to shut off the led once they are turned on but putting !outa[Pin] after waitcnt, why doesn't it work going from 23 to 16? So it would read as

waitcnt(clkfreq/4 + cnt)
!outa[pin]

If I do !dira[Pin] it will light up on the way back to 16 but not shut off the others.
• Posts: 6,212
edited 2019-02-04 - 19:35:58
Here's another approach that may be a bit easier to digest. Look for opportunities to put redundant code into methods, and when you keep using the same methods over and over group them into a library.
```pub main | pin

repeat { forever }
repeat pin from 16 to 23
high(pin)
waitcnt(cnt + (clkfreq >> 2))
repeat pin from 16 to 23
low(pin)
waitcnt(cnt + (clkfreq >> 2))

pub high(pin)

outa[pin] := 1
dira[pin] := 1

pub low(pin)

outa[pin] := 0
dira[pin] := 1
```

For my normal work I have a library called jm_io.spin which handles essential pin IO functions.
Jon McPhalen
Hollywood, CA
It's Jon or JonnyMac -- please do not call me Jonny.
• Posts: 22,341
Just for future reference, AIman, you'll probably get more extensive help if your title includes some information about your issue. "Help!!" is much too vague to elicit the best this forum has to offer.

-Phil
Perfection is achieved not when there is nothing more to add, but when there is nothing left to take away. -Antoine de Saint-Exupery
• Posts: 525
Code still isn't working. I will attach a video of a few seconds so you can see the original code.

The LEDs will light up for 1/4 second and then shut off for 1/4 second. The LEDs move from pin 16 to 17 to 18 etc. However, on the way down from 23 the LEDs will not light up - or not light properly. There should be one light on at a time and the light should move back and forth across string of lights.

For ease of coding I am using the Propeller Demo Board Rev G since it already has LEDs on it.

This code works halfway, but it is bulky and crude. At this point I will take bulky and crude if it will run like its supposed to.
```Var

Byte Pin                {I/O pin to toggle on/off. Good from 0 - 255}

PUB Toggle

Pin := 16                {Set value of pin to starting point}

Repeat

repeat while pin < 24   ' Repeat the following
dira[Pin]~~           ' Set I/O pin to output direction
!outa[Pin]            ' Toggle I/O Pin
waitcnt(clkfreq/4 + cnt)  ' Wait for delay cycles
dira[pin]~
waitcnt(clkfreq/4 + cnt)  ' Wait for delay cycles
Pin += 1              ' Go to next pin. Short form of Pin := Pin + 1

repeat until pin < 16
dira[Pin]~~           ' Set I/O pin to output direction
!outa[Pin]            ' Toggle I/O Pin
waitcnt(clkfreq/4 + cnt)  ' Wait for delay cycles
dira[pin]~
waitcnt(clkfreq/4 + cnt)  ' Wait for delay cycles
Pin -= 1              ' Go to next pin. Short form of Pin := Pin - 1

```
• Posts: 525
So the video I was going to attach apparently isn't allowed. This will require a bit of thought to work around.
• Posts: 13,560
AIman wrote: »
The LEDs will light up for 1/4 second and then shut off for 1/4 second. The LEDs move from pin 16 to 17 to 18 etc. However, on the way down from 23 the LEDs will not light up - or not light properly. There should be one light on at a time and the light should move back and forth across string of lights.
In fault finding, you should note exactly what does happen, and compare that with what you expect to happen.
'not light properly' is too vague to be useful.
Do the leds (all/some/partial?) fail on down-slope only, and work ok on every up slope time window ?

• Posts: 525
The video I was hoping to upload would have shown and clarified most questions but in absence of that -

The LEDs are one to each pin on pins 16 - 23. Pin 16 goes high turning on that LED and after 1/4 second the pin goes low shutting off the LED. This repeats up to pin 23.

BUT

When the pins go from 23 - 16 it doesn't work and only a very faint LED light is visible as the pins count down from 23 to 16.
• Posts: 13,560
AIman wrote: »
When the pins go from 23 - 16 it doesn't work and only a very faint LED light is visible as the pins count down from 23 to 16.
.. and then what happens.. ? Does the next up ramp work ok ?

"doesn't work and only a very faint LED light is visible" is still vague. Does that mean the timing is as-expected, but each single on LEDs is faint, or all leds light, but faint, or something else ?
The small details matter a lot, and help you figure out possible causes.
• Posts: 807
Here's the solution. You need to delay before you toggle the pin when going from P23 to p16.
```repeat until pin < 16
dira[Pin]~~           ' Set I/O pin to output direction
waitcnt(clkfreq/4 + cnt)  ' Wait for delay cycles  - need to delay before turning off the led
!outa[Pin]            ' Toggle I/O Pin
' waitcnt(clkfreq/4 + cnt)  '   This line needs to be before the !outa[pin]
dira[pin]~
waitcnt(clkfreq/4 + cnt)  ' Wait for delay cycles
Pin -= 1              ' Go to next pin. Short form of Pin :=
```
• Posts: 525
The next ramp up works fine, but for whatever reason going down doesn't. The LEDs are very dim, almost not noticeable, but have proper timing.

In other words, it works with timing as it should, but the LEDs won't fully light up when subtracting a pin. I will try to post pictures since the video clip can't post.
• Posts: 525
twm47099 - Thanks!!!
• Posts: 6,212
edited 2019-02-05 - 15:12:41
The next ramp up works fine, but for whatever reason going down doesn't. The LEDs are very dim, almost not noticeable, but have proper timing.
Because you're not manipulating the IO pins correctly.

I don't mean to be blunt, but you're writing code in a bit of an obtuse manner. In my opinion, it's always best to write code that is obvious.

It seems that you now want to create a Larson Scanner (Cylons, Kit car) -- I didn't understand that from your initial post. As I live and work in Hollywood, I've written lots of Larson scanner code. Here's one way that I think is easy to follow.
```pub main | pin

repeat { forever }
repeat pin from 16 to 22
high(pin)
waitcnt(cnt + (clkfreq >> 3))                             ' delay 1/8 second
low(pin)
repeat pin from 23 to 17
high(pin)
waitcnt(cnt + (clkfreq >> 3))
low(pin)

pub high(pin)

outa[pin] := 1
dira[pin] := 1

pub low(pin)

outa[pin] := 0
dira[pin] := 1
```

There are many roads that lead to Rome, and many ways to solve the same programming problem. Here's one that's close to your style -- if a bit more obvious:
```pub main | pin

outa[23..16] := %00000000                                     ' set LEDs to outputs
dira[23..16] := %11111111

pin := 16

repeat { forever }
repeat
outa[pin] := 1
waitcnt(cnt + (clkfreq >> 3))                             ' delay 1/8 second
outa[pin] := 0
while (++pin < 23)
repeat
outa[pin] := 1
waitcnt(cnt + (clkfreq >> 3))
outa[pin] := 0
while (--pin > 16)
```
Jon McPhalen
Hollywood, CA
It's Jon or JonnyMac -- please do not call me Jonny.
• Posts: 525
edited 2019-03-03 - 20:50:58
Ironic you should say that about KITT. My kiddo pointed out to me just a short while ago that my lights looked like the light from KITT.

Here is what I got to work.
```Var

Byte Pin     {I/O pin to toggle on/off. Good from 0 - 255}
Byte Rate    {Number value that controls the speed of the LED Light}

PUB LED_Light

Pin := 16     {Set value of pin to starting point}
' Final Value to start zero

Rate := 11   ' TEST VALUE ONLY. REAL VALUE WILL BE SENT FROM OTHER SOURCES.

Repeat

Repeat while Pin =< 23        ' Repeat the following
dira[Pin]~~                ' Set I/O pin to output direction
!outa[Pin]                 ' Toggle I/O Pin
if Pin <> 16
waitcnt(clkfreq/Rate + cnt)  ' Wait for delay cycles
dira[Pin]~                 ' Set I/O pin to input direction
if Pin <> 23
waitcnt(clkfreq/Rate + cnt)' Wait for delay cycles
Pin += 1                   ' Go to next pin. Short form of Pin := Pin + 1

Repeat while Pin => 16
dira[Pin]~~                  ' Set I/O pin to output direction
if Pin <> 23
waitcnt(clkfreq/Rate + cnt)  ' Wait for delay cycles  - need to delay before turning off the led
!outa[Pin]                 ' Toggle I/O Pin
dira[Pin]~                 ' Set I/O pin to input direction
if Pin <> 16
waitcnt(clkfreq/Rate  + cnt)' Wait for delay cycles
Pin -= 1                   ' Go to next pin. Short form of Pin := Pin - 1

```

• Posts: 807
Glad it worked. When you post code you should post it between code blocks to preserve the indentation. Put the cursor where you ant to start the code; click on the "C" in the icons at the top of your edit window. That will enter [ code][ /code] (without the spaces between the [ and the first character.) Position the cursor between the ][ and paste your code there.

• Posts: 6,212
edited 2019-03-01 - 17:24:58
In my day job I do a bit of Python coding. There is an Easter Egg built into Python that every programmer can benefit from. If you enter "import this" into a Python REPL, you get...
```Beautiful is better than ugly.
Explicit is better than implicit.
Simple is better than complex.
Complex is better than complicated.
Flat is better than nested.
Sparse is better than dense.
Special cases aren't special enough to break the rules.
Although practicality beats purity.
Errors should never pass silently.
Unless explicitly silenced.
In the face of ambiguity, refuse the temptation to guess.
There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
Now is better than never.
Although never is often better than *right* now.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
Namespaces are one honking great idea -- let's do more of those!
```
In point of fact, you spent more time adding comments than had you encapsulated confusing code into named methods (like I did with high() and low()).
Also, I suggest you avoid arcane operators when possible. This:
```  dira[pin]~~
```
...uses an arcane operator and requires a comment. This version, however...
```  dira[pin] := 1
```
...would be understood by a fellow embedded programmer and actually runs more than half a microsecond faster. Yes, microseconds count. Not in this program, but if you do enough coding you will come to a point where you are fighting for them. I wrote a very complex (about 6000 total lines) laser-tag control program and I was scrambling for every bit of time savings I could muster. The great thing about the Propeller is that you can use standard code to do timing tests -- here's what I used:
```  pin := 16

elapsed := -cnt                                               ' start timing
dira[pin]~~                                                   ' code under test
elapsed += cnt - 544                                          ' stop timing
term.dec(elapsed)
term.txn(13, 2)

elapsed := -cnt
dira[pin] := 1
elapsed += cnt - 544
term.dec(elapsed)
term.txn(13, 2)
```
In my program, term is an instance of FullDuplexSerial (my variation). The -544 in the final calculation accounts for the Spin overhead. If you remove code from between the updates of elapsed you should get 0.
Finally, if I had been commissioned by on of the special effects clients to write this code, I would have given them this (archive is attached).
```pub main

setup

' Larson scanner
' -- avoids "sticking" on end boundaries

repeat { forever }
zip(LSB, MSB-1, ON_MS)
zip(MSB, LSB+1, ON_MS)

pub zip(first, last, ms) | idx

'' Sequence outputs from first to last
'' -- output delay in milliseconds

repeat idx from first to last
io.high(idx)
time.pause(ms)
io.low(idx)
```
This follows the rule by one of my colleagues: If you find yourself writing the same block of code more than once, it's time for a function/method. I frequently remind the new programmers in work to break their code into small, atomic functions/methods so that the code the write has more clarity and the ability to be re-used.
You'll get there. I just takes time and practice.
Jon McPhalen
Hollywood, CA
It's Jon or JonnyMac -- please do not call me Jonny.