 |
 |
| View previous topic :: View next topic |
| Author |
Message |
thefloyd
Joined: 02 Sep 2009
Posts: 46
|
| losing my mind? or can I not figure out a simple for() loop? |
 Posted: Mon Sep 17, 2012 10:30 am |
 |
|
ok, so I'm not the best programmer/hardware guy out there but I can certainly handle a for() loop.. why oh why doesn't this work?
| Code: |
for(bit=23;bit>=0;bit--) {
if(bit_test(this_led, bit) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
}
|
Whereas this works just fine:
| Code: |
if(bit_test(this_led, 23) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
if(bit_test(this_led, 22) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
if(bit_test(this_led, 21) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
if(bit_test(this_led, 20) == 1) {
spi_write(ws2811_one);
}
..... all the way down to zero
|
Obviously the hardware is fine as the latter works.
I've tried all sorts of permutations of my for() loop. I'm sure I'm missing something obvious.. can someone provide me with the electronic slap upside the head? |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon Sep 17, 2012 11:50 am |
|
|
Make the first code into a small but compilable test program, with the
#include for the PIC, #fuses, #use delay(), main(), all variable and
constant declarations, etc.
Also post your compiler version. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19433
|
|
| Posted: Mon Sep 17, 2012 11:50 am |
|
|
Number types...
If you have an unsigned integer, it is _always_ going to be >=0.
When you decrement past 0 it wraps to be 255,
Hence loop can't work.
Best Wishes |
|
|
thefloyd
Joined: 02 Sep 2009
Posts: 46
|
|
| Posted: Mon Sep 17, 2012 12:16 pm |
|
|
Thanks for the quick replies.
ttelmah, nice catch. This explains a different problem I had somewhere else, but not this one as the following don't work:
| Code: |
for(bit=23;bit!=255;bit--)
or
int bit;
for(bit=23;bit>=0;bit--)
|
PCM: my compiler version is 4.134
A quick sample program is below. I've included the broken send_frame() routine as the working one is fairly obvious.
| Code: |
#include <18f46k22.h>
#Fuses INTRC_IO,PLLEN,PRIMARY_ON,NOFCMEN,NOIESO,PUT,
#Fuses BROWNOUT,BORV29,NOWDT,
#Fuses CCP2C1,NOPBADEN,CCP3B5,
#Fuses NOHFOFST,TIMER3B5,NOMCLR,
#Fuses NOSTVREN,NOLVP,NOXINST,NODEBUG
#Fuses NOPROTECT,NOCPB,NOCPD,NOWRT,NOWRTC
#Fuses NOWRTB,NOWRTD,NOEBTR,NOEBTRB
#use delay( clock=64000000,INTERNAL )
#define LED_STRIP_LEN 50
unsigned int32 led_strip_colors[LED_STRIP_LEN];
#define ws2811_zero 0b10000000
#define ws2811_one 0b11110000
void send_frame() {
unsigned int16 i;
unsigned int bit;
unsigned int32 this_led;
for(i=0;i<LED_STRIP_LEN;i++) {
this_led = led_strip_colors[i];
for(bit=23;bit!=255;bit--) {
if(bit_test(this_led, bit) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
}
}
delay_us(50); // latch
}
void single_color(unsigned int32 color1) {
unsigned long i;
for(i=0; i<LED_STRIP_LEN; i++) {
led_strip_colors[i] = color1;
}
}
void main(void) {
unsigned int16 i;
unsigned int j;
setup_spi( SPI_MASTER | SPI_L_TO_H | SPI_CLK_DIV_16 );
delay_ms(250);
single_color(0xFFFFFF);
send_frame();
delay_ms(5000);
single_color(0xFFC58F);
send_frame();
delay_ms(5000);
single_color(0xFF9329);
send_frame();
delay_ms(5000);
single_color(0xFF0000);
send_frame();
delay_ms(1000);
single_color(0x00FF00);
send_frame();
delay_ms(1000);
single_color(0x0000FF);
send_frame();
sleep;
}
|
|
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon Sep 17, 2012 12:54 pm |
|
|
You may be able to debug your problem by running your program in
MPLAB simulator. Modify it as shown below in the lines marked "TESTING"
and setup the UART1 feature of MPLAB to display hardware UART text in
the Output Window of MPLAB. This post explains how to do that:
http://www.ccsinfo.com/forum/viewtopic.php?t=23408&start=1
For example, if your send_frame() routine gets an input value that is all
1's, then it should transmit 24 bits of all 1's, like this:
| Quote: | 111111111111111111111111
|
With this method you can test the "pure code" portion of your program
to see if it's working OK. It doesn't test SPI. It just tests your internal
algorithm. But at least you can easily test it now.
Note all lines marked with *** TESTING ***:
| Code: |
#include <18f46k22.h>
#Fuses INTRC_IO,PLLEN,PRIMARY_ON,NOFCMEN,NOIESO,PUT,
#Fuses BROWNOUT,BORV29,NOWDT,
#Fuses CCP2C1,NOPBADEN,CCP3B5,
#Fuses NOHFOFST,TIMER3B5,NOMCLR,
#Fuses NOSTVREN,NOLVP,NOXINST,NODEBUG
#Fuses NOPROTECT,NOCPB,NOCPD,NOWRT,NOWRTC
#Fuses NOWRTB,NOWRTD,NOEBTR,NOEBTRB
#use delay( clock=64000000,INTERNAL )
#use rs232(baud=9600, UART1, ERRORS) // *** TESTING ***
#define LED_STRIP_LEN 1 // *** FOR TESTING *** set to 1 (was 50)
unsigned int32 led_strip_colors[LED_STRIP_LEN];
#define ws2811_zero 0b10000000
#define ws2811_one 0b11110000
void send_frame() {
unsigned int16 i;
unsigned int bit;
unsigned int32 this_led;
for(i=0;i<LED_STRIP_LEN;i++) {
this_led = led_strip_colors[i];
for(bit=23; bit!=255; bit--) {
if(bit_test(this_led, bit) == 1) {
// spi_write(ws2811_one);
putc('1'); // *** TESTING ***
}
else {
//spi_write(ws2811_zero);
putc('0'); // *** TESTING ***
}
}
}
delay_us(50); // latch
putc('\r'); // *** FOR TESTING ***
}
void single_color(unsigned int32 color1) {
unsigned long i;
for(i=0; i<LED_STRIP_LEN; i++) {
led_strip_colors[i] = color1;
}
}
void main(void) {
unsigned int16 i;
unsigned int j;
// Commented out the next two lines *** FOR TESTING ***
//setup_spi( SPI_MASTER | SPI_L_TO_H | SPI_CLK_DIV_16 );
//delay_ms(250);
single_color(0xFFFFFF);
send_frame();
while(1); // *** TESTING ***
delay_ms(5000);
single_color(0xFFC58F);
send_frame();
delay_ms(5000);
single_color(0xFF9329);
send_frame();
delay_ms(5000);
single_color(0xFF0000);
send_frame();
delay_ms(1000);
single_color(0x00FF00);
send_frame();
delay_ms(1000);
single_color(0x0000FF);
send_frame();
// sleep;
} |
|
|
|
thefloyd
Joined: 02 Sep 2009
Posts: 46
|
|
| Posted: Mon Sep 17, 2012 3:33 pm |
|
|
Sometimes it takes someone else to remind you what you already know :D I've used putc/printf/etc to debug programs that deal with text. Silly me not to think of it in this case. I put a USB to rs232 converter on my breadboard and found some interesting things.
1) my loop seems to work fine. Tested it with several combinations of data and saw exactly what I expected. the bit!=255 part seems to stop the loop right where it should. I got 1s and 0s exactly where I was expecting them and exactly as many of them as I was expecting.
2) There's something wrong with my hardware or setup (or compiler?), and the loop timing may be throwing timing off enough to break things, see below.
3) My PIC can't be oscillating at 64mhz. Despite the use of:
| Code: |
#Fuses INTRC_IO,PLLEN,PRIMARY_ON,NOFCMEN,NOIESO,PUT,
and
#use delay( clock=64000000,INTERNAL )
|
The RS232 data was useless. Off to debugging land I went, I tried changing the use delay line to "clock=8000000, INTERNAL" and viola! My RS232 data made sense.. Note I had forgotten to change the fuse back to NOPLLEN yet this worked. huh?
So now the next question is why the hell isn't my PIC running at 64mhz? I have #Fuses PLLEN and a proper #use delay setting. Is this the bug I'm really chasing?
Am I missing something in the setup? |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Mon Sep 17, 2012 3:55 pm |
|
|
This program blinks an LED at a 1 Hz rate with vs. 4.134. I tested it in
hardware with an 18F45K22, but it should also work with an 18F46K22.
| Code: |
#include <18F45K22.h>
#fuses INTRC_IO,NOWDT,PUT,BROWNOUT,NOLVP
#use delay(clock=64M)
//======================================
void main(void)
{
// Blink the LED once per second. The 10/90 duty cycle
// makes it easier to time the flashes with a stopwatch.
while(1)
{
output_high(PIN_B0);
delay_ms(100);
output_low(PIN_B0);
delay_ms(900);
}
} |
|
|
|
thefloyd
Joined: 02 Sep 2009
Posts: 46
|
|
| Posted: Sat Sep 29, 2012 2:50 pm |
|
|
So after taking a break, head scratching, debugging, and taking another break I got to take a better look on a scope.
For some reason, it seems trying to execute the for() loop is severely retarding the timing. Enough that there's at least a 50us delay, and that's causing the light string to latch and move on with life. I really didn't think that'd be possible (at 64 mhz!), but I can definitely confirm the data is coming much more slowly with long(ish) pauses when the loop is executed versus doing the bit test sequentially and banging the bits out.
Am I being unrealistic with my timing demands here? Or is there something else I'm missing? Or is there something else possibly going on? I know there are people doing this on a 16mhz arduino (with some careful optimization, I get that.. but 64mhz should allow me to be a bit more "lazy" no?) |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Sat Sep 29, 2012 3:13 pm |
|
|
| Quote: | there's at least a 50us delay, and that's causing the light string to latch
|
Yes there is, and you put it in there. See the line in bold in your code below:
| Quote: |
void send_frame() {
unsigned int16 i;
unsigned int bit;
unsigned int32 this_led;
for(i=0;i<LED_STRIP_LEN;i++) {
this_led = led_strip_colors[i];
for(bit=23;bit!=255;bit--) {
if(bit_test(this_led, bit) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
}
}
delay_us(50); // latch
} |
|
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Sat Sep 29, 2012 3:46 pm |
|
|
style: bit_test is True or false Boolean purity ;-))
| Code: |
if(bit_test(this_led, bit) == 1)
// VS
if(bit_test(this_led, bit))
|
can't help but notice that |
|
|
thefloyd
Joined: 02 Sep 2009
Posts: 46
|
|
| Posted: Sun Sep 30, 2012 9:47 am |
|
|
| PCM programmer wrote: | | Quote: | there's at least a 50us delay, and that's causing the light string to latch
|
Yes there is, and you put it in there. See the line in bold in your code below:
| Quote: |
void send_frame() {
unsigned int16 i;
unsigned int bit;
unsigned int32 this_led;
for(i=0;i<LED_STRIP_LEN;i++) {
this_led = led_strip_colors[i];
| Code: | for(bit=23;bit!=255;bit--) {
if(bit_test(this_led, bit) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
} |
}
}
delay_us(50); // latch
} |
|
Yes. The latch is there but ONLY after iterating through the loop. What I'm saying is that WITHIN the loop, I'm finding at least a 50us delay.
this works:
| Code: |
void send_frame() {
unsigned int16 i;
unsigned int32 this_led;
for(i=0;i<LED_STRIP_LEN;i++) {
this_led = led_strip_colors[i];
if(bit_test(this_led, 23) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
if(bit_test(this_led, 22) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
...
if(bit_test(this_led, 1) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
if(bit_test(this_led, 0) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
}
delay_us(50);
}
|
THIS doesn't:
| Code: |
void send_frame() {
unsigned int16 i;
unsigned int bit;
unsigned int32 this_led;
for(i=0;i<LED_STRIP_LEN;i++) {
this_led = led_strip_colors[i];
for(bit=23;bit!=255;bit--) {
if(bit_test(this_led, bit) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
}
}
delay_us(50);
}
|
I'm seeing delays of > 50us INSIDE the for(bit=23;bit!=255;bit--) loop. Debugging prints the right number of 1s and 0s to the screen. But when executed on the actual hardware, only the first LED does things (flickers, does weird stuff). Probing with a scope and the timing inside the loop is all broken, but doing each of the 24 bit tests by hand manages to avoid the 50us+ pauses. Why? |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19433
|
|
| Posted: Sun Sep 30, 2012 10:27 am |
|
|
You don't show your SPI configuration code.
I'd say it is using soft SPI at perhaps about 200KHz.
This will then take 48uSec to send the bytes.
Best Wishes |
|
|
thefloyd
Joined: 02 Sep 2009
Posts: 46
|
|
| Posted: Sun Sep 30, 2012 10:47 am |
|
|
| Ttelmah wrote: | You don't show your SPI configuration code.
I'd say it is using soft SPI at perhaps about 200KHz.
This will then take 48uSec to send the bytes.
Best Wishes |
Sorry, the setup_spi command is earlier up in the thread. This is a PIC @ 64mhz with the following:
| Code: |
setup_spi( SPI_MASTER | SPI_L_TO_H | SPI_CLK_DIV_16 );
|
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19433
|
|
| Posted: Sun Sep 30, 2012 11:20 am |
|
|
However it'd be a _lot_ faster, to generate your own bit mask, load this with 2^23, and just rotate this once each time round the loop, and 'and' this with the value.
Problem is that 'bit test' with a constant generates a single machine instruction. With a variable, it generates a '1', then rotates this by the specified number, and 'and's this to find if the bit is true/false.
So your test generates an average of 13 4byte rotations each time round the loop. Add the loop count and the and operation, and you have something like perhaps 200 instructions, so I can see it taking perhaps 15+uSec.
Using your own mask instead, which only has to rotate once each time round the loop will save a lot of time.
Best Wishes |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19433
|
|
| Posted: Sun Sep 30, 2012 12:09 pm |
|
|
As a comment, the fastest way to do this is probably:
| Code: |
for (bit=0;bit<24 bit++) {
if(bit_test(this_led, 23) == 1) {
spi_write(ws2811_one);
}
else {
spi_write(ws2811_zero);
}
this_ked*=2;
}
|
This way you test the single _fixed_ bit (fast) and just rotate your stored number once each loop. I'd guess at least a dozen times faster.
Best Wishes |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
