Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

system-tools: add gpiod (official Python bindings for libgpiod) #9592

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

HungerHa
Copy link
Contributor

gpiod are the official Python bindings for libgpiod. libgpiod is already part of the system tools add-on, only these Python bindings are currently missing to be able to use it in Python scripts.

lgpio and gpiozero are easier to use, but currently have some insurmountable problems, as I have tried to explain here:

Feature request: RPi-Tools with official Python bindings for libgpiod

@HungerHa HungerHa changed the title rpi-tools: add gpiod (official Python bindings for libgpiod) rpi-tools: add gpiod support Dec 19, 2024
@chewitt
Copy link
Member

chewitt commented Dec 23, 2024

@HiassofT can you look into this one? As long as it aligns to the direction direction RPi devs are pushing for with GPIO?

@HiassofT
Copy link
Member

@chewitt I'll do some tests next week when I'm finished with family visits.

In general I think using upstream libgpiod is a huge improvement over all the oddball gpiozero, lgpio, RPi-tools etc shenanigans which are poorly supported, have multiple long-time unfixed issues and don't seem to go anywhere with fixing them.

@HiassofT
Copy link
Member

HiassofT commented Jan 1, 2025

As we already have a libgpiod package, for the providing the libgpiod command line tools in the system-tools addon, and this is not really RPi-specific I'd suggest to add building the python bindings in the libgpiod package and including the python module in system-tools instead of rpi-tools.

For LE master this libgpiod change should work:

--- a/packages/addons/addon-depends/libgpiod/package.mk
+++ b/packages/addons/addon-depends/libgpiod/package.mk
@@ -7,9 +7,16 @@ PKG_SHA256="ae35329db7027c740e90c883baf27c26311f0614e6a7b115771b28188b992aec"
 PKG_LICENSE="GPLv2+"
 PKG_SITE="https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/about/"
 PKG_URL="https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/snapshot/libgpiod-${PKG_VERSION}.tar.gz"
-PKG_DEPENDS_TARGET="toolchain"
+PKG_DEPENDS_TARGET="toolchain Python3 setuptools:host"
 PKG_LONGDESC="Tools for interacting with the linux GPIO character device."
 PKG_TOOLCHAIN="autotools"
 PKG_BUILD_FLAGS="+pic"
 
 PKG_CONFIGURE_OPTS_TARGET="--enable-tools --disable-shared"
+
+post_make_target() {
+  (
+    cd ../bindings/python
+    python_target_env python3 -m build -n -w -x
+  )
+}

@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 4, 2025

As we already have a libgpiod package, for the providing the libgpiod command line tools in the system-tools addon, and this is not really RPi-specific I'd suggest to add building the python bindings in the libgpiod package and including the python module in system-tools instead of rpi-tools.

For LE master this libgpiod change should work:

--- a/packages/addons/addon-depends/libgpiod/package.mk
+++ b/packages/addons/addon-depends/libgpiod/package.mk
@@ -7,9 +7,16 @@ PKG_SHA256="ae35329db7027c740e90c883baf27c26311f0614e6a7b115771b28188b992aec"
 PKG_LICENSE="GPLv2+"
 PKG_SITE="https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/about/"
 PKG_URL="https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/snapshot/libgpiod-${PKG_VERSION}.tar.gz"
-PKG_DEPENDS_TARGET="toolchain"
+PKG_DEPENDS_TARGET="toolchain Python3 setuptools:host"
 PKG_LONGDESC="Tools for interacting with the linux GPIO character device."
 PKG_TOOLCHAIN="autotools"
 PKG_BUILD_FLAGS="+pic"
 
 PKG_CONFIGURE_OPTS_TARGET="--enable-tools --disable-shared"
+
+post_make_target() {
+  (
+    cd ../bindings/python
+    python_target_env python3 -m build -n -w -x
+  )
+}

I'm support the idea to make it general available and implemented this in system-tools addon instead of rpi-tools addon. I used your design and added the one-liner to include the bindings in the addon. On that way it's not every time the most recent version of gpiod, but guarantees to be in sync with libgpiod. Currently I'm testing with this approach for LE 12 and LE 13.

BTW: During compilation of system-tools I stumbled over the stanza issue (.threads/logs/xx.log) with hid_mapper and add a patch file to fix that:

--- a/debian/control	2014-08-10 22:43:26.000000000 +0200
+++ b/debian/control	2025-01-04 00:04:12.200522465 +0100
@@ -1,10 +1,14 @@
-Package: hid-mapper
+Source: hid-mapper
 Version: 2.0.1
 Section: misc
 Priority: optional
+Maintainer: Sylvain Leroux <[email protected]>
+
+Package: hid-mapper
+Section: misc
+Priority: optional
 Architecture: amd64
 Depends: libc6 (>= 2.11.3)
-Maintainer: Sylvain Leroux <[email protected]>
 Description: HID event mapper
  Developed to map USB HID IR remote to keyboard events.
  Should work with any USB HID device.

What would be the next step? After successful local testing, should I revise this pull request to move the Python bindings to the system-tools addon? And is it ok to include the hid_mapper patch file in this pull request or should I open a new one?

@heitbaum
Copy link
Contributor

heitbaum commented Jan 4, 2025

Hi @HungerHa - can you share the issue that is being addressed by the hid_mapper patch? These should go upstream (though I’m aware the code in currently not maintained.)

@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 4, 2025

Hi @heitbaum - without the patch the log will flooded and it seems no compilation progress:

�[1;36mUNPACK�[0m      hid_mapper
    �[1;32mAPPLY PATCH �[1;37m(common)�[0m�[0m      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-01_crosscompile.patch
patching file Makefile
    �[1;32mAPPLY PATCH �[1;37m(common)�[0m�[0m      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-02_Fix-key-code-reading.patch
patching file hid.c
patching file include/log.h
patching file log.cpp
patching file hid.c
patching file MapReader.cpp
    �[1;32mAPPLY PATCH �[1;37m(common)�[0m�[0m      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-03_include-sys-time.patch
patching file hid.c
�[1;33mBUILD�[0m      hid_mapper �[1;37m(target)�[0m
    �[1;35mTOOLCHAIN�[0m      make (auto-detect)
Executing (target): make 
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field

EDIT: I created a pull request at the source: s-leroux/hid_mapper#5

@heitbaum
Copy link
Contributor

heitbaum commented Jan 4, 2025

Hi @heitbaum - without the patch the log will flooded and it seems no compilation progress:

�[1;36mUNPACK�[0m      hid_mapper
    �[1;32mAPPLY PATCH �[1;37m(common)�[0m�[0m      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-01_crosscompile.patch
patching file Makefile
    �[1;32mAPPLY PATCH �[1;37m(common)�[0m�[0m      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-02_Fix-key-code-reading.patch
patching file hid.c
patching file include/log.h
patching file log.cpp
patching file hid.c
patching file MapReader.cpp
    �[1;32mAPPLY PATCH �[1;37m(common)�[0m�[0m      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-03_include-sys-time.patch
patching file hid.c
�[1;33mBUILD�[0m      hid_mapper �[1;37m(target)�[0m
    �[1;35mTOOLCHAIN�[0m      make (auto-detect)
Executing (target): make 
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field
dpkg-buildflags: error: syntax error in debian/control at line 10: first stanza lacks a 'Source' field

EDIT: I created a pull request at the source: s-leroux/hid_mapper#5

Your build environment is Debian from the guess of things as here is the output from within the docker build environment. The Makefile as below is the root cause of the issue - as it is dragging in settings from the local Operating system (not allowing the cross compile to do its thing.)

https://github.com/s-leroux/hid_mapper/blob/2a80972fab0810ca10237e49bb46beca3b6443b0/Makefile#L24-L26

s/build hid_mapper
UNPACK      hid_mapper
    APPLY PATCH (common)      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-01_crosscompile.patch
patching file Makefile
    APPLY PATCH (common)      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-02_Fix-key-code-reading.patch
patching file hid.c
patching file include/log.h
patching file log.cpp
patching file hid.c
patching file MapReader.cpp
    APPLY PATCH (common)      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-03_include-sys-time.patch
patching file hid.c
BUILD      hid_mapper (target)
    TOOLCHAIN      make (auto-detect)
Executing (target): make 
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC main.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -c -march=x86-64-v3 -Wall -pipe  -O2 -fomit-frame-pointer -DNDEBUG -Iinclude -fPIC uinput_device.c
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -c -march=x86-64-v3 -Wall -pipe  -O2 -fomit-frame-pointer -DNDEBUG -Iinclude -fPIC hid.c
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -c -march=x86-64-v3 -Wall -pipe  -O2 -fomit-frame-pointer -DNDEBUG -Iinclude -fPIC signals.c
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC keys_definition.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC EventMapping.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC Keys.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC Exception.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC MapReader.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC MapReaderMouse.cpp
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC log.cpp
hid.c: In function 'lookup_hid_product':
hid.c:80:15: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
   80 |         while(entry = readdir(dh))
      |               ^~~~~
/var/media/DATA/home-rudi/LibreELEC.tv/build.LibreELEC-Generic.x86_64-13.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++  -Wl,--as-needed -fuse-ld=gold main.o uinput_device.o hid.o signals.o keys_definition.o EventMapping.o Keys.o Exception.o MapReader.o MapReaderMouse.o log.o -o hid_mapper

Build timing details:
================================
              unpack:      0.121
     pre-build setup:      0.002
           configure:      0.002
                make:      0.052
        make install:      0.003
        copy sysroot:      0.006
     cleanup install:      0.017
--------------------------------
          total time:      0.203

@HiassofT
Copy link
Member

HiassofT commented Jan 4, 2025

@HungerHa if the suggested changes work, please update the PR and create a LE12 backport PR.

Unfortunately your current LE12 PR has been merged prematurely, if it's not too much hassle please also add a drop of the gpiod package and the rpi-tools changes, plus a PKG_REV bump of rpi-tools so LE12 is in sync with master.

Don't worry about hid_mapper, if anything that's a separate issue and should be handled separately (if necessary at all)

@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 4, 2025

@HiassofT,

now that the cause of the hid_mapper problem has been clarified, I am currently looking for a way to compile the bindings from libgpiod with the LE12 toolchain. If I copy the function “python_target_env” into the LE 12 environment, I am already one step further, but no build module is found. So I can't use the same mechanism at the moment. Is there an easy way to add this Python module to the toolchain of LE12?

EDIT: Okay I see, there also package.mk files below ...python/devel...

make[2]: Entering directory '/home/andreas/src/LibreELEC.tv/build.LibreELEC-ARMv8.aarch64-12.0-devel/build/libgpiod-2.2/.aarch64-libreelec-linux-gnu'
make[2]: Leaving directory '/home/andreas/src/LibreELEC.tv/build.LibreELEC-ARMv8.aarch64-12.0-devel/build/libgpiod-2.2/.aarch64-libreelec-linux-gnu'
make[1]: Leaving directory '/home/andreas/src/LibreELEC.tv/build.LibreELEC-ARMv8.aarch64-12.0-devel/build/libgpiod-2.2/.aarch64-libreelec-linux-gnu'
/home/andreas/src/LibreELEC.tv/build.LibreELEC-ARMv8.aarch64-12.0-devel/toolchain/bin/python3: No module named build
FAILURE: scripts/build libgpiod:target during post_make_target (package.mk)
*********** FAILED COMMAND ***********
${PKG_CURRENT_CALL} "${@}"
**************************************
FAILURE: scripts/build libgpiod:target has failed!

The following log for this failure is available:
  /home/andreas/src/LibreELEC.tv/build.LibreELEC-ARMv8.aarch64-12.0-devel/.threads/logs/116.log

>>> libgpiod:target seq 116 >>>
[118/120] [FAIL] build   libgpiod:target

BTW: @heitbaum, your updated patch files for hid_mapper from master are not in the LE12 branch, but required to compile successfully the system-tools there too.

@heitbaum
Copy link
Contributor

heitbaum commented Jan 4, 2025

Build in LE12 CI/CD is fine. Just checked locally. Fine too.


s/build hid_mapper
UNPACK      hid_mapper
    APPLY PATCH (common)      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-01_crosscompile.patch
patching file Makefile
Hunk #1 succeeded at 38 with fuzz 2 (offset 29 lines).
    APPLY PATCH (common)      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-02_include-sys-time.patch
patching file hid.c
Hunk #1 succeeded at 23 (offset 1 line).
    APPLY PATCH (common)      packages/addons/addon-depends/system-tools-depends/hid_mapper/patches/hid_mapper-03_fix-parser.patch
patching file MapReader.cpp
BUILD      hid_mapper (target)
    TOOLCHAIN      make (auto-detect)
Executing (target): make 
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC main.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -c -march=x86-64 -Wall -pipe  -O2 -fomit-frame-pointer -DNDEBUG -Iinclude -fPIC uinput_device.c
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -c -march=x86-64 -Wall -pipe  -O2 -fomit-frame-pointer -DNDEBUG -Iinclude -fPIC hid.c
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-gcc -c -march=x86-64 -Wall -pipe  -O2 -fomit-frame-pointer -DNDEBUG -Iinclude -fPIC signals.c
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC keys_definition.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC EventMapping.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC Keys.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC Exception.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC MapReader.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC MapReaderMouse.cpp
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++ -c -Iinclude -fPIC log.cpp
In file included from log.cpp:21:
include/log.h:36:6: warning: declaration of 'void log(LogLevel, const char*, __va_list_tag*)' conflicts with built-in declaration 'double log(double)' [-Wbuiltin-declaration-mismatch]
   36 | void log(LogLevel severity, const char* fmt, va_list args);
      |      ^~~
In file included from MapReader.cpp:25:
include/log.h:36:6: warning: declaration of 'void log(LogLevel, const char*, __va_list_tag*)' conflicts with built-in declaration 'double log(double)' [-Wbuiltin-declaration-mismatch]
   36 | void log(LogLevel severity, const char* fmt, va_list args);
      |      ^~~
In file included from signals.c:22:
include/log.h:36:6: warning: conflicting types for built-in function 'log'; expected 'double(double)' [-Wbuiltin-declaration-mismatch]
   36 | void log(LogLevel severity, const char* fmt, va_list args);
      |      ^~~
include/log.h:23:1: note: 'log' is declared in header '<math.h>'
   22 | #include <stdarg.h>
  +++ |+#include <math.h>
   23 | 
In file included from main.cpp:28:
include/log.h:36:6: warning: declaration of 'void log(LogLevel, const char*, __va_list_tag*)' conflicts with built-in declaration 'double log(double)' [-Wbuiltin-declaration-mismatch]
   36 | void log(LogLevel severity, const char* fmt, va_list args);
      |      ^~~
In file included from hid.c:21:
include/log.h:36:6: warning: conflicting types for built-in function 'log'; expected 'double(double)' [-Wbuiltin-declaration-mismatch]
   36 | void log(LogLevel severity, const char* fmt, va_list args);
      |      ^~~
include/log.h:23:1: note: 'log' is declared in header '<math.h>'
   22 | #include <stdarg.h>
  +++ |+#include <math.h>
   23 | 
hid.c: In function 'lookup_hid_product':
hid.c:78:15: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
   78 |         while(entry = readdir(dh))
      |               ^~~~~
hid.c:125:41: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
  125 |                                 else if(strcmp(buf,manufacturer)!=0)
      |                                         ^~~~~~
hid.c:27:1: note: include '<string.h>' or provide a declaration of 'strcmp'
   26 | #include <sys/time.h>
  +++ |+#include <string.h>
   27 | 
/var/media/DATA/home-rudi/LibreELEC.libreelec-12.0/build.LibreELEC-Generic.x86_64-12.0-devel/toolchain/bin/x86_64-libreelec-linux-gnu-g++  -Wl,--as-needed -fuse-ld=gold main.o uinput_device.o hid.o signals.o keys_definition.o EventMapping.o Keys.o Exception.o MapReader.o MapReaderMouse.o log.o -o hid_mapper

Build timing details:
================================
              unpack:      0.087
     pre-build setup:      0.001
           configure:      0.003
                make:      0.181
        make install:      0.006
        copy sysroot:      0.013
     cleanup install:      0.029
--------------------------------
          total time:      0.320

@HiassofT
Copy link
Member

HiassofT commented Jan 4, 2025

LE12 doesn't have python_target_env, you need to build the python module with python setup.py build like you did for gpiod.

Just move the exports inside the post_make_target function, change directory like in LE13 and then (hopefully) python setup.py build should just work. I guess you'll also need the setuptools patch adapted to libgpiod directory structure

@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 5, 2025

@HiassofT,

thank you for the hint. I was believing that I tried that already and the build artifact _ext.so was missing afterwards. I must messed up something during my last attempt, now it was built and I can start to test locally.

@HungerHa HungerHa force-pushed the master-rpi-tools-gpiod branch from 03146d2 to 93804bf Compare January 7, 2025 22:26
@HungerHa HungerHa changed the title rpi-tools: add gpiod support system-tools: add gpiod (official Python bindings for libgpiod) Jan 7, 2025
@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 7, 2025

@HiassofT,

when I started almost from scratch (make clean), the problem with the failed build step reappeared. To fix this, I switched to post_makeinstall_target where the required build artifacts are available.

Now it's available within system-tools package (instead of rpi-tools).

@heitbaum
Copy link
Contributor

heitbaum commented Jan 8, 2025

This should fix the build of libgpiod (it should have been -sysroot)
If you could fix the system-tools

diff --git a/packages/addons/addon-depends/libgpiod/package.mk b/packages/addons/addon-depends/libgpiod/package.mk
index 7bd735191f..d278a8269e 100644
--- a/packages/addons/addon-depends/libgpiod/package.mk
+++ b/packages/addons/addon-depends/libgpiod/package.mk
@@ -10,14 +10,14 @@ PKG_URL="https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/snapshot/libg
 PKG_DEPENDS_TARGET="toolchain Python3 setuptools:host"
 PKG_LONGDESC="Tools for interacting with the linux GPIO character device."
 PKG_TOOLCHAIN="autotools"
-PKG_BUILD_FLAGS="+pic"
+PKG_BUILD_FLAGS="+pic -sysroot"

 PKG_CONFIGURE_OPTS_TARGET="--enable-tools --disable-shared"

-post_makeinstall_target() {
+post_make_target() {
   (
-    export LDFLAGS="${LDFLAGS} -L$(get_install_dir libgpiod)/usr/lib"
-    export CFLAGS="${CFLAGS} -I$(get_install_dir libgpiod)/usr/include"
+    LDFLAGS+=" -L${PKG_BUILD}/.${TARGET_NAME}/lib/.libs"
+    CFLAGS+=" -I${PKG_BUILD}/include"
     cd ../bindings/python
     python_target_env python3 -m build -n -w -x
   )

@heitbaum heitbaum removed the RPi label Jan 8, 2025
@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 8, 2025

@heitbaum

Thank you, very nice. Every day I learn more about it. I'll check it out and update the pull requests....

@heitbaum
Copy link
Contributor

heitbaum commented Jan 8, 2025

@heitbaum

Thank you, very nice. Every day I learn more about it. I'll check it out and update the pull requests....

your welcome.

When you finalise your commits - use the house-style “system-tools: blah blah blah” (with the colon.)

@HungerHa HungerHa force-pushed the master-rpi-tools-gpiod branch from 93804bf to 4f9d1eb Compare January 8, 2025 09:02
@heitbaum heitbaum requested a review from HiassofT January 8, 2025 09:13
Copy link
Contributor

@heitbaum heitbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good now

@HiassofT
Copy link
Member

HiassofT commented Jan 8, 2025

Thanks a lot for your work, the changes look fine and local build testing worked, too. Only 1 minor nit then this is good to be merged:

Please change the commit messages to our house-style:

the libgpiod change should be libgpiod: build python bindings and the system-tools change should be system-tools: add libgpiod python bindings

Please also adjust those commit messages in your LE12 PR

@HungerHa HungerHa force-pushed the master-rpi-tools-gpiod branch from 4f9d1eb to 3a90909 Compare January 9, 2025 07:59
@HungerHa
Copy link
Contributor Author

HungerHa commented Jan 9, 2025

You're welcome.
I had already feared that some little detail might still be wrong ... :-) Commit messages changed now. Hopefully followed the pattern right with the LE12 commits.

Copy link
Member

@HiassofT HiassofT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your contribution and your patience!

@HiassofT HiassofT merged commit 94059cd into LibreELEC:master Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants