[Buildroot] [PATCH] package/rpi-rgb-led-matrix: new package

[Arnout Vandecappelle]

On 11/10/2021 12:06, Grzegorz Blach wrote:
> A library to control commonly available 64x64, 32x32 or 16x32
> RGB LED panels with the Raspberry Pi. Can support PWM up to
> 11Bit per channel, providing true 24bpp color with CIE1931
> profile.
> 
> Signed-off-by: Grzegorz Blach <grzegorz at blach.pl>

  Applied to master, thanks.

[snip]
> diff --git a/package/rpi-rgb-led-matrix/0001-Rename-LDFLAGS-to-RGB_LDFLAGS.patch b/package/rpi-rgb-led-matrix/0001-Rename-LDFLAGS-to-RGB_LDFLAGS.patch
> new file mode 100644
> index 0000000000..f7a92f5fb2
> --- /dev/null
> +++ b/package/rpi-rgb-led-matrix/0001-Rename-LDFLAGS-to-RGB_LDFLAGS.patch
> @@ -0,0 +1,46 @@
> +From 056a189836a82b832cd8e6f886cb2090344bfa08 Mon Sep 17 00:00:00 2001
> +From: Grzegorz Blach <grzegorz at blach.pl>
> +Date: Sat, 29 Aug 2020 20:48:57 +0200
> +Subject: [PATCH] Rename LDFLAGS to RGB_LDFLAGS.
> +
> +This change allows additional flags to be added to the linker
> +from the command line.
> +
> +Signed-off-by: Grzegorz Blach <grzegorz at blach.pl>

  Please send this patch upstream.

  Preferably, that's something you do before submitting the patch to Buildroot, 
and you add something like "Upstream: <PR URL>" to the patch.

> +---
> + utils/Makefile | 8 ++++----
> + 1 file changed, 4 insertions(+), 4 deletions(-)
> +
> +diff --git a/utils/Makefile b/utils/Makefile
> +index e751adc..e704442 100644
> +--- a/utils/Makefile
> ++++ b/utils/Makefile
> +@@ -12,7 +12,7 @@ RGB_INCDIR=$(RGB_LIB_DISTRIBUTION)/include
> + RGB_LIBDIR=$(RGB_LIB_DISTRIBUTION)/lib
> + RGB_LIBRARY_NAME=rgbmatrix
> + RGB_LIBRARY=$(RGB_LIBDIR)/lib$(RGB_LIBRARY_NAME).a
> +-LDFLAGS+=-L$(RGB_LIBDIR) -l$(RGB_LIBRARY_NAME) -lrt -lm -lpthread

  I've kept the patch, but is it really needed? "LDFLAGS +=" should make sure 
that variables passed in through the environment are just appended to.


> ++RGB_LDFLAGS+=-L$(RGB_LIBDIR) -l$(RGB_LIBRARY_NAME) -lrt -lm -lpthread
[snip]

> +comment "rpi-rgb-led-matrix needs a toolchain w/ C++, threads, dynamic library"
> +	depends on BR2_aarch64 || BR2_arm
> +	depends on !(BR2_INSTALL_LIBSTDCPP && BR2_TOOLCHAIN_HAS_THREADS) || BR2_STATIC_LIBS

  I've rewritten this as !LIBSTDCPP || !THREADS || STATIC, which is what we 
mostly do. I've also split it into three lines.

> diff --git a/package/rpi-rgb-led-matrix/rpi-rgb-led-matrix.hash b/package/rpi-rgb-led-matrix/rpi-rgb-led-matrix.hash
> new file mode 100644
> index 0000000000..3b1cb5fd76
> --- /dev/null
> +++ b/package/rpi-rgb-led-matrix/rpi-rgb-led-matrix.hash
> @@ -0,0 +1,3 @@
> +# Locally computed:
> +sha256  5efc0293bb180b958079910e968721ff2dac291c435d065ecec9ceba8589567d  rpi-rgb-led-matrix-63e3e7ffdbe88223cc80e1faa508bc4f3cf2bea4.tar.gz
> +sha256  8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643  COPYING
> diff --git a/package/rpi-rgb-led-matrix/rpi-rgb-led-matrix.mk b/package/rpi-rgb-led-matrix/rpi-rgb-led-matrix.mk
> new file mode 100644
> index 0000000000..55b6b363c1
> --- /dev/null
> +++ b/package/rpi-rgb-led-matrix/rpi-rgb-led-matrix.mk
> @@ -0,0 +1,79 @@
> +################################################################################
> +#
> +# rpi-rgb-led-matrix
> +#
> +################################################################################
> +
> +RPI_RGB_LED_MATRIX_VERSION = 63e3e7ffdbe88223cc80e1faa508bc4f3cf2bea4
> +RPI_RGB_LED_MATRIX_SITE = $(call github,hzeller,rpi-rgb-led-matrix,$(RPI_RGB_LED_MATRIX_VERSION))
> +RPI_RGB_LED_MATRIX_LICENSE = GPL-2.0
> +RPI_RGB_LED_MATRIX_LICENSE_FILES = COPYING
> +RPI_RGB_LED_MATRIX_INSTALL_STAGING = YES
> +
> +ifeq ($(BR2_PACKAGE_RPI_RGB_LED_MATRIX_IMAGE_VIEWER),y)
> +RPI_RGB_LED_MATRIX_DEPENDENCIES += graphicsmagick
> +RPI_RGB_LED_MATRIX_POST_BUILD_HOOKS += RPI_RGB_LED_MATRIX_BUILD_IMAGE_VIEWER_CMDS

  We normally put the definition of the hook within the condition as well and 
put the HOOKS += part immediately after it. So I changed that for all hooks, and 
also moved all the conditions below the basic build/install commands.

> +RPI_RGB_LED_MATRIX_POST_INSTALL_TARGET_HOOKS += RPI_RGB_LED_MATRIX_INSTALL_IMAGE_VIEWER_CMDS
> +endif
> +
> +ifeq ($(BR2_PACKAGE_RPI_RGB_LED_MATRIX_TEXT_SCROLLER),y)
> +RPI_RGB_LED_MATRIX_POST_BUILD_HOOKS += RPI_RGB_LED_MATRIX_BUILD_TEXT_SCROLLER_CMDS
> +RPI_RGB_LED_MATRIX_POST_INSTALL_TARGET_HOOKS += RPI_RGB_LED_MATRIX_INSTALL_TEXT_SCROLLER_CMDS
> +endif
> +
> +ifeq ($(BR2_PACKAGE_RPI_RGB_LED_MATRIX_VIDEO_VIEWER),y)
> +RPI_RGB_LED_MATRIX_DEPENDENCIES += ffmpeg
> +RPI_RGB_LED_MATRIX_POST_BUILD_HOOKS += RPI_RGB_LED_MATRIX_BUILD_VIDEO_VIEWER_CMDS
> +RPI_RGB_LED_MATRIX_POST_INSTALL_TARGET_HOOKS += RPI_RGB_LED_MATRIX_INSTALL_VIDEO_VIEWER_CMDS
> +endif
> +
> +define RPI_RGB_LED_MATRIX_BUILD_CMDS
> +	$(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D)/lib all

  Ah, this explains why the patch is needed. If you call it like this:

	$(TARGET_CONFIGURE_OPTS) $(MAKE) ...

then it is possible for the Makefile to use += to extend variables instead of 
overriding them. That only works if the Makefile doesn't do something stupid 
like unconditionally setting CFLAGS or CC.



  Regards,
  Arnout

[snip]