WSD2811 led time code in C, optimize further

Go To Last Post
16 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

1. I am using WSD2811 to run led in series. I am using C-code for programming. My below code is working by carefully adjusting some of NOP instructions. 

 

2. 100 leds are working fine on WSD2811, my code is in C, so no issues. Dont want to write in assembly.

 

3. Below is my code, in which I check each bit of red/green/blue by if/else, then make pin high/low

 

4. Want to know if there any other way I can optimize code in c language or better way to do that in C-language.

 

5. Please dont suggest to write in assembly or if code is already working why to change. 
 
I want to write in C only, & check if there any way I can further optimize code in C language.

 

Again by current code, my leds are working fine 

 

 

static void ws2811_rgb_write(uint8_t red, uint8_t green , uint8_t blue)  
{
//R1
    if(red & 0x80U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }  
///    
    
//R2
    if(red & 0x40U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///  

//R3
    if(red & 0x20U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///      
    
//R4
    if(red & 0x10U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///  

//R5
    if(red & 0x08U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///    

//R6
    if(red & 0x04U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }   
///     

//R7
    if(red & 0x02U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///    

//R8
    if(red & 0x01U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///    


    
    
//R1
    if(green & 0x80U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }  
///    
    
//R2
    if(green & 0x40U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///  

//R3
    if(green & 0x20U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }    
///      
    
//R4
    if(green & 0x10U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }       
///  

//R5
    if(green & 0x08U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///    

//R6
    if(green & 0x04U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///     

//R7
    if(green & 0x02U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///    

//R8
    if(green & 0x01U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///        

    
    
    
//R1
    if(blue & 0x80U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }  
///    
    
//R2
    if(blue & 0x40U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///  

//R3
    if(blue & 0x20U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///      
    
//R4
    if(blue & 0x10U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///  

//R5
    if(blue & 0x08U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///    

//R6
    if(blue & 0x04U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///     

//R7
    if(blue & 0x02U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }      
///    

//R8
    if(blue & 0x01U)
    {
        LED_PIN_HIGH();
    }  
    else
    {
        LED_PIN_LOW();
    }     
///        
    
      
  
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So different pin for each bit in red, green, blue?  In all, 24 different pins?  And they are uncorrelated (i.e., you can't just write "PORTB = red")?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would at least remove all of the redundant code like this:

void set_led_pin (bool on)
{
    if (on)
    {
        LED_PIN_HIGH();
    }
    else
    {
        LED_PIN_LOW();
    }
}

static void ws2811_rgb_write(uint8_t red, uint8_t green , uint8_t blue)  
{
    set_led_pin (red & 0x80U);
    set_led_pin (red & 0x40U);
    set_led_pin (red & 0x20U);
    .
    .
    .
}

 

Though this could change your NOP timing.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If you want to use Mike's code but a little more efficiently add "static inline __attribute__((always_inline))" to the set_led_pin function.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

static inline __attribute__((always_inline))

 

Can this be added to a C++ member function?

 

The inline keyword originally was a hint to a C++

compiler that a function could/should be done

without a call/return.  The compiler was allowed

to use call/return anyway if it felt like it.  Now

it's OK for the compiler to inline a function which

isn't marked inline. Reminiscent of the register

keyword which is now mostly historic.  So now

all inline means in C++ is you can put a function

definition in a header file (the whole function, not

just the prototype) and include the header in

multiple code files.

 

I'd like to be able to compel avr-gcc to inline a

member function when necessary, even if it thinks

it knows better and wants to make a callable

subroutine for it.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

WSD2811 need to send serial thorugh a single pin, 24 bits.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-mike wrote:

I would at least remove all of the redundant code like this:

void set_led_pin (bool on)
{
    if (on)
    {
        LED_PIN_HIGH();
    }
    else
    {
        LED_PIN_LOW();
    }
}

static void ws2811_rgb_write(uint8_t red, uint8_t green , uint8_t blue)
{
    set_led_pin (red & 0x80U);
    set_led_pin (red & 0x40U);
    set_led_pin (red & 0x20U);
    .
    .
    .
}

 

Though this could change your NOP timing.

 

--Mike

 

 

And this is the "optimize for size" version (not speed! might mess up timings beyond repair):

 

void set_led_pin (bool on)
{
    if (on)
    {
        LED_PIN_HIGH();
    }
    else
    {
        LED_PIN_LOW();
    }
}

static void ws2811_rgb_write(uint8_t red, uint8_t green , uint8_t blue)
{
    for (uint8_t mask = 0x80; mask; mask >>= 1) {
        set_led_pin (red & mask);
    }
    for (uint8_t mask = 0x80; mask; mask >>= 1) {
        set_led_pin (green & mask);
    }
    for (uint8_t mask = 0x80; mask; mask >>= 1) {
        set_led_pin (blue & mask);
   }
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

WSD2811, need very tight timing control, I have optimize for code for speed.

Thats why i have unrolled all the loops and make everything as much as constant possible.

 

these are defines for LED_PIN_HIGH() and LED_PIN_LOW()  , which indivually contain how to send 1 & 0 to wsd2811

 

 

#define  LED_PIN_HIGH()         do{\
	                              LED_PIN_ON();\
	                             _nop_();_nop_();_nop_();_nop_();_nop_();_nop_();_nop_();_nop_();\
	                             _nop_();_nop_();_nop_();_nop_();_nop_();\
	                             LED_PIN_OFF();\
                               }while(0)

#define  LED_PIN_LOW()         do{\
	                             LED_PIN_ON();\
                            	 LED_PIN_OFF();\
							     _nop_();_nop_();_nop_();_nop_();_nop_();_nop_();_nop_();_nop_();\
								_nop_();_nop_();_nop_();_nop_();\
                               }while(0)

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

For such accurate timing this is one occasion when an Asm implementation is probably justified. If you look at other WSxxx code you'll see this is the usual choice.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

No doubt that I also would do it i ASM, you problem is that a compiler update can break your code.

 

If you want to do it in C perhaps use a timer with come compare matches. You can loop around with some while loops, (it's to slow with real ISR's).

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

d

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

Last Edited: Mon. Jun 24, 2019 - 08:58 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

d

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

Last Edited: Mon. Jun 24, 2019 - 08:58 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,  

   I would caution you not to do anything to the code because it is working.   The WD2811 multi-color programmable LEDs are very tight on timing when used by an AVR doing C language.  And you have a long string of LEDs.   You are probably using about 10-15 AVR instructions per each of the 24 bits used for each LED's color.   I assume that for about 100 LEDs and about 27 microseconds needed to send the logic stream for each one, that it takes about 3 milliseconds to light each "frame" of 100 LEDs.  Say there is some LED automation at 25 frames a second, about 40 milliseconds per frame.  Since loading takes 3 mS, you have 37 milliseconds per frame to compose the next frame's colors.

 

In any event, when you have working tight code, remember the first rule of engineering:  If it works, don't f*** with it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

IMO (and others here agree), you should be using ASM for this.  Why are you so determined to avoid it?  I'm guessing that if you looked at your signal timing on a scope, it wouldn't be pretty, and in any case you're at the mercy of the compiler from now until forever.  Use the right tool for the job.

 

https://www.embeddedrelated.com/...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

With C i am able to get it working at the moment.

Kept its hex file separate to in furtire even if compiler update breaks the code, i have hex file separate.

In furture, if required then will do in assembly, but for the time being i am able to solve the issue.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Vindhyachal Takniki wrote:

With C i am able to get it working at the moment.

Kept its hex file separate to in furtire even if compiler update breaks the code, i have hex file separate.

In furture, if required then will do in assembly, but for the time being i am able to solve the issue.

It would still be an interesting exercise to put a scope on your output and see how well it conforms with the LED timing spec.  Maybe you're well within spec, or maybe you're on the ragged edge, and at a different temperature, or with different LEDs, your LEDs will start behaving erratically.  If you have access to a scope I'd recommend using it.