tmedicci commented on code in PR #11428:
URL: https://github.com/apache/nuttx/pull/11428#discussion_r1436169123


##########
drivers/leds/ws2812.c:
##########
@@ -55,15 +55,7 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
-#ifdef WS2812_HAS_WHITE
-#  define WS2812_RW_PIXEL_SIZE  4
-#else 
-#  define WS2812_RW_PIXEL_SIZE  3
-#endif
-
-#ifdef CONFIG_WS2812_NON_SPI_DRIVER
-
-#else /* CONFIG_WS2812_NON_SPI_DRIVER */

Review Comment:
   > Hi guys, merry Christmas. I didn't want to allocate 32 bits unless it was 
strictly required. That being said, I understand that using packed structures 
could degrade the performance. One could say that there is plenty of memory and 
people shouldn't care too much about it. In my case, I was using 700+ leds, so 
there were some memory concerns.
   
   Hi, Merry Christmas!
   
   Yes, I understand your concerns about using a 32-bit when only 24-bit is 
necessary. However, no application that used the WS2812 encodes the LED pixel 
data in 24-bit space (this would depend on the application itself). 
   Even considering that the application could encode LED data in 24-bit/pixel, 
we still had:
   
   ```
   #ifdef WS2812_HAS_WHITE
   #  define WS2812_RW_PIXEL_SIZE  4
   #else 
   #  define WS2812_RW_PIXEL_SIZE  3
   #endif
   ```
    with `WS2812_HAS_WHITE` not being defined by Kconfig (it should be 
'CONFIG_WS2812_HAS_WHITE`). Also, the 
[`ws2812_write`](https://github.com/apache/nuttx/blob/d1326e81bca47617aa253705ca778c7d51f82038/drivers/leds/ws2812.c#L316)
 (for `CONFIG_WS2812_NON_SPI_DRIVER`) is not compliant with raw 24-bit data 
length (LED pixel data should be encoded in 32-bit space).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to