GPIOD Support through alternative implementation of the HWIF interface

[wrichter]

I'm running vzlogger on Fedora/RPi4 which no longer supports the legacy GPIO interface and has a changed MMAP layout. Neither of the existing approaches worked for me. Therefore I've implemented support for libGPIOD (which works both on Debian as well as Fedora).

In order to retain backwards compatibility with the config file syntax / not to introduce additional config fields, a GPIO line > 1000 will trigger the GPIOD implementation, and GPIO lines <1000 will trigger the old GPIO (/sys fs-based) implementation.

Currently there is no way to specify which gpiochip to use, it always defaults to gpiochip0.

[wrichter]

Am currently reworking this to prevent spurious readings - pls don't merge right now

[J-A-U]

I won't merge anything as long as the checks fail.

[wrichter]

sounds prudent :-) given that libgpiod is a new dependency, the test env needs to include it as well, otherwise it will continue to fail. How can this be done?

[r00t-]

@wrichter: add it here: https://github.com/volkszaehler/vzlogger/blob/master/.github/workflows/build%2Btest.yml#L8

[wrichter]

Thanks, am currently out and will do it when I'm back at home after next week.

[wrichter]

Hi @r00t- , hi @J-A-U can you please rerun the workflows to see if the code is now in better shape?

[wrichter]

the tests have additional #defines which interfere with the compilation… will work around

[wrichter]

@r00t- @J-A-U could you please run the workflows again?

[wrichter]

Thanks... so this is a bit annoying :-( the clang-format on my system creates a formatting incompatible with what the test requires:

wrichter@wrichter-mac vzlogger % uname -a
Darwin wrichter-mac 21.5.0 Darwin Kernel Version 21.5.0: Tue Apr 26 21:08:22 PDT 2022; root:xnu-8020.121.3~4/RELEASE_X86_64 x86_64
wrichter@wrichter-mac vzlogger % clang-format --version
clang-format version 14.0.4 
wrichter@wrichter-mac vzlogger % find include src tests  -type f -name '*.h' -o -name '*.cpp' -o -name '*.hpp' | xargs -I{} -P1 clang-format -i -style=file {}
wrichter@wrichter-mac vzlogger % git status
On branch gpiod
Your branch is up to date with 'origin/gpiod'.

nothing to commit, working tree clean
wrichter@wrichter-mac vzlogger %

Same when I try it from within a debian:stable container on another machine:

root@4d889486e0c9:/workdir# uname -a
Linux 4d889486e0c9 5.17.12-300.fc36.aarch64 #1 SMP Mon May 30 16:39:03 UTC 2022 aarch64 GNU/Linux
root@4d889486e0c9:/workdir# clang-format --version
Debian clang-format version 11.0.1-2
root@4d889486e0c9:/workdir# find include src tests  -type f -name '*.h' -o -name '*.cpp' -o -name '*.hpp' | xargs -I{} -P1 clang-format -i -style=file {}
root@4d889486e0c9:/workdir# git status
On branch gpiod
Your branch is up to date with 'origin/gpiod'.

nothing to commit, working tree clean
root@4d889486e0c9:/workdir#

Question 1: Do you have any tipps how to massage things so that the formatting is acceptable?

Question 2: The test fails because it doesn't link the GPIOD libs. This is my first exposure to CMake and I'm lost where to add them for the tests. Could you please help me? (for the vzlogger itself I made the following changes):

wrichter@wrichter-mac vzlogger % git diff master gpiod -- CMakeLists.txt src/CMakeLists.txt
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c053b62..d278612 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -171,6 +171,8 @@ endif(WIN32)

 find_library(LIBUUID uuid)
 find_library(LIBGCRYPT gcrypt)
+find_library(GPIOD_LIBRARY NAMES libgpiod.so)
+

 include_directories(${CMAKE_BINARY_DIR})
 include_directories(${CMAKE_SOURCE_DIR}/include)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index cd76ec2..473b5d0 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -81,7 +81,7 @@ if( TARGET )
     target_link_libraries(vzlogger dl)
   endif( ${TARGET} STREQUAL "ar71xx")
 endif( TARGET )
-target_link_libraries(vzlogger ${CURL_STATIC_LIBRARIES} ${CURL_LIBRARIES} unistring ${GNUTLS_LIBRARIES} ${OPENSSL_LIBRARIES} )
+target_link_libraries(vzlogger ${CURL_STATIC_LIBRARIES} ${CURL_LIBRARIES} unistring ${GNUTLS_LIBRARIES} ${OPENSSL_LIBRARIES} ${GPIOD_LIBRARY})

 # add programs to the install target
 INSTALL(PROGRAMS
wrichter@wrichter-mac vzlogger %

[leoguiders]

Hi Wolfram,

found your gpiod branch while trying to get S0 working on a Raspberry Pi 3B. Thanks for your work, I'm currently testing the gpiod support with my S0-meter.

In order to retain backwards compatibility with the config file syntax / not to introduce additional config fields, a GPIO line > 1000 will trigger the GPIOD implementation, and GPIO lines <1000 will trigger the old GPIO (/sys fs-based) implementation.

This does feel hacky.

Currently there is no way to specify which gpiochip to use, it always defaults to gpiochip0.

The "device" configuration-string is currently unused when setting the gpio value. That would be a good place to set the gpiochip. Maybe not less hacky than the current solution, but what about switching to gpiod when gpio is set and device string starts with "/dev/gpiochip" (or gpiod_is_gpiochip_device(device) returns true for that matter) ?

[wrichter]

Hi Wolfram,

found your gpiod branch while trying to get S0 working on a Raspberry Pi 3B. Thanks for your work, I'm currently testing the gpiod support with my S0-meter.

In order to retain backwards compatibility with the config file syntax / not to introduce additional config fields, a GPIO line > 1000 will trigger the GPIOD implementation, and GPIO lines <1000 will trigger the old GPIO (/sys fs-based) implementation.

This does feel hacky.

It sure is. Of course we could also introduce a new Boolean config parameter use_gpiod which defaults to false. Cleaner would be to just create a new config parameter hardware_interface_impl that has values like „uart“, „gpio_sysfs“, „gpio_mmap“, „gpiod“ and „legacy“ and defaults to „legacy“. Then existing users can continue to use the magic decisiontree to decide whether to use the uart/gpio/mmap implementations and new users have a single place to specify which implementation they want. This feels like a separate pull request though.

Currently there is no way to specify which gpiochip to use, it always defaults to gpiochip0.

The "device" configuration-string is currently unused when setting the gpio value. That would be a good place to set the gpiochip. Maybe not less hacky than the current solution, but what about switching to gpiod when gpio is set and device string starts with "/dev/gpiochip" (or gpiod_is_gpiochip_device(device) returns true for that matter) ?

again laziness on my side. I like the idea - would you want to send a pull request to https://github.com/wrichter/vzlogger/tree/gpiod ?

[leoguiders]

I'm still trying to get my head around what is required and what is the most user-friendly way to configure this.

The question here really is - do we need do support /sysfs and gpiod at the same time or can we just assume that gpiod is the better way to deal with gpios and use it if available and fallback to /sysfs if it isn't?

[wrichter]

so I personally do think that GPIOD is the better solution, and I have been running the implementation successfully for the last ~4 months. But it hasn't seen much testing beyond my setup, is not yet merged with the main development branch, etc... I expect both versions will need to be supported in parallel until the GPIOD version has seen more testing. At the moment it can't even be merged since some tests fail - which I believe is currently caused by something else than my code, but also the tests don't yet know anything about GPIOD (which was the reason it failed earlier - see https://github.com/volkszaehler/vzlogger/pull/525#issuecomment-1170490110 ).