Add libgpiodv2 support

[huesla]

Fixes: https://github.com/dotnet/iot/issues/2179 Hi there, This PR aims to integrate version 2 of libgpiod

C library and tools for interacting with the linux GPIO character device (gpiod stands for GPIO device)

Structure Basically, my strategy was to extend as much functionality as possible while touching as little existing code as possible. A lot has changed in the API between v1 and v2 of libgpiod. For example, the control of GPIOs is purely via so-called request objects. If you want to learn more about the concepts, see:

1. overview https://www.youtube.com/watch?v=EcUULioruj0&t=1s
2. docs https://libgpiod.readthedocs.io/en/latest/modules.html

As the new functions are very different from the existing ones, I have decided to differentiate between V1 and V2 in the namespace or class name. (What changes together should go together). This concerns the Interop class, as well as the driver and event handler.

The new structure from the lower to the higher level is shown as follows:

Binding Interop.libgpiod Maps all methods of the current libgpiod API, structured in the same way as in the docs.

Proxies libgpiod offers so-called "opaque data classes". These are objects whose fields are not visible and can only be changed via methods. libgpiod divides the API logically according to objects (or modules) see docs. To represent these in C#, there are now so-called proxy classes. Such a class offers the same methods as the controlled gpiod object, but in a more C#-friendly way. LibGpiodProxyBase is the base class and provides methods for thread security. (more information on the threading contract of libgpiod) and wraps any exception from the native call in GpiodException (easier for clients to handle). I think when you know the concept of a libgpiod module, looking at the corresponding proxy class should give a clear picutre in what it does (Basically it only forwards stuff).

Handles Each proxy class works with a SafeHandle, the object that holds the pointer to the gpiod object. SafeHandle comes from the framework and makes sure to call ReleaseHandle() once, which frees the gpiod object.

Enums Constants see docs

LibGpiodV2Driver This class is the "bridge" between GpioController and the binding. Each driver instance manages one gpiochip. It implements the usual methods and manages proxies, mainly the LineRequest to control GPIOs. The methods of the driver were implemented to have as little traffic as possible, e.g. reuse existing LineRequest objects etc.

LibGpiodV2EventObserver Is closely coupled with LibGpiodV2Driver and manages EdgeEvent subscriptions / callbacks.

GpioController Existing class that instantiates the driver. LibGpiodDriverFactory helps to load the correct version suitable for the system. If libgpiodv2 is available, v2 is loaded, otherwise v1 or v0.

Testing Tested with manual tests and GpioControllerTestBase tests on Raspberry Pi 3/4/5 with latest RPI OS (lite). I had to adapt the gpiochip number with Pi5 to 4, instead of 0 on Pi3/4. I am not sure how you treat different boards in unit tests, so I left 0 as default, because Pi5 is not yet as popular.

Testing (continued) The current tests of GpioControllerTestBase use delays, which provokes flakyness and makes the tests slow. I have an improvement branch that uses reset events instead of delays. Again I tested it on the Pi 3/4/5... the time got improved alot and I did not encounter flakyness anymore:

Pi3: 50.2 sec -> 32.2 sec Pi4: 42.8s -> 19.3s Pi5: 40.3s -> 13.2s (measured only once)

I could make a seperate PR for this after this one, depending on how the integration of this will go. The real question is whether other drivers can keep up... would probably have to be tested with your CI pipeline.

Commits The commits are chronologically logically ordered.

Advanced use It should be noted that the current GpioController/Driver architecture imposes restrictions on the libgpiodv2 functionality. For example, information such as ChipInfo, LineInfo, EventInfo, Event sequence numbers etc. are not available. Also functions like setting Debounce Period, EventBuffer size etc. are not available. (One of my use cases is, for example, the time-accurate detection of edges, and the time info is in the edge event as Ns timestamp). In order not to change the existing architecture, I have stuck to it and made proxies etc. internal. However, this also means that this functionality cannot be used externally. My suggestion is therefore to make it public. This allows advanced users extended functionality if they need it. Normal users can continue to use the GpioController API. Since it would probably be a bit tedious (even for advanced users) to manage proxy objects directly, and to use every fine grained function, it would make sense to create an abstraction that would simplify this and cover the most relevant use cases. The code could look much nicer then, for example:

var request = new GpioChip(4).PrepareRequest()
    .SetConsumer("me")
    .AddLineSettings(lineSettings => 
    {
        lineSettings.Lines = new { 12, 18 };
        lineSettings.Direction = LineDirection.Input;
        lineSettings.EdgeDetection = EdgeDetection.Both;
    })
    .DoReqeust();

Task.Run(() =>
{
    foreach (var event in request.GetNextEvent())
    {
        ...
    }
};

request.SetLine(12, High);
request.SetLine(12, Low);

(By the way, this example is inspired by the Python wrapper for libgpiodv2)

I would of course also make documentation for a few examples.

I look forward to comments and opinions :)

Microsoft Reviewers: Open in CodeFlow

EDIT (@krwq): adding link to the issue

[huesla]

@dotnet-policy-service agree

[pgrawehr]

Thanks for the submission! Reviewing this will take a while, in particular because it touches the core parts of our library. So please be patient.

Can you please specify on what system you ran your tests and what systems you expect to work with the new library? Are there any systems where the old driver is no longer supported and only the new one will work?

[huesla]

Thanks for the submission! Reviewing this will take a while, in particular because it touches the core parts of our library. So please be patient.

Can you please specify on what system you ran your tests and what systems you expect to work with the new library? Are there any systems where the old driver is no longer supported and only the new one will work?

All right, I will be patient then :)

If you ask me then the answer is Raspberry Pi. It was also Pi's 3,4 and 5 on which I ran the tests. Basically, the libgpiod C# drivers are not directly tied to a specific system, because wherever libgpiod is supported, the drivers are too (and that is on systems where gpiolib is supported and GPIO devices are available).

On the question of whether the V2 driver can replace the V1 driver: It depends :-) What distinguishes V2 from V1, apart from a more sophisticated API, are of course the features, such as sequence number mapping to edge events, bias setting, kernel clock setting etc... These features could be offered publicly for advanced users of dotnet iot. Otherwise, however, it probably depends mostly on which libgpiod the known distros deliver. It will probably take some time until v2 is delivered by default.

It's also not the goal of this PR to replace the v1 driver, rather to "prepare for the future" and have a v2 implementation in place, whose features can be used.

[joperezr]

Thanks a bunch for the contribution @huesla, I went through it and added some feedback (might seem like a lot but most are just either NITs or questions) but this is overall looking really good. Thanks a lot again for taking the time to contribute this.

[krwq]

[Triage] Is it possible to install both libgpiod and libgpiod2 side by side? If not then we should try to make existing implementation work (it could use new code as implementation detail or derive).

[huesla]

[Triage] Is it possible to install both libgpiod and libgpiod2 side by side? If not then we should try to make existing implementation work (it could use new code as implementation detail or derive).

Usually different versions of a library can coexist. I am not sure what you mean with the second sentence

[raffaeler]

Usually different versions of a library can coexist. I am not sure what you mean with the second sentence

During the triage we were discussing whether the user of the library should be able or not to decide to bind against libgpiod v1 or v2.

• If both libraries can coexist, there may be multiple reason to pick one or the other (for example, the v2 is installed after the software is working, but you still want to rely an app on v1 for whatever reason).
• If instead the v2 installation process automatically uninstall v1, there is no reason for the user to decide to use v1 or v2. Our library should instead detect and use the currently installed library. This is what happens now when libgpiod is not installed at all. In this case the sysfs driver is automatically picked and this process is transparent to the user. This is what the second sentence was referring to.

Since you are actively working on this PR (thanks for that :-) ) we want to know for sure whether both libraries can coexists on the same installation as this implies a different strategy as I hope to have explained above.

[huesla]

Thanks @raffaeler for the clarification. I guess the question is how do package managers on common distros deal with present library files on updates. When I find time I will investigate that.

[raffaeler]

You are welcome @huesla. For our purpose it should be sufficient to know whether different versions of libgpiod can coexist or not.

[joperezr]

And just to add to what @raffaeler explained, this will be helpful on whether we should make LibgpiodV2 driver public or keep it as an implementation detail. If you find that the library can't be installed side by side with other version, then there is no point in making the v2 driver public, as consumers will mainly only want to select to use libgpiod and we should internally figure out which driver to use (v1 or v2). If, on the other hand, the versions can live side by side, then we should probably keep the driver public and let the user decide if they choose.

[joperezr]

I think a safe bet would be to start by keeping it internal (we can always make it public later), in favor of not over complicating what we expose as well as allowing existing apps to just automatically retarget to v2 when the version in their OS is updated.

[huesla]

@joperezr It really depends on how the user installs libgpiod. Probably most users will use a package manager that "normally" replaces older library files. I would have at least tested it with APT, but there is no v2 package yet. In the rare case that the user installs the library manually, 2 versions of it may exist in the filesystem. So to answer your question about whether 2 versions can exist: maybe, but in most cases probably not. I think the more important argument for making the driver version public or not is the question of whether the features of v2 should be public (as discussed). Since you do not share this desire (yet?), it will make no difference to the user whether they choose the v1 or v2 driver. So I agree that it's best to leave it internal and load the appropriate version dynamically under the hood. I would even go so far as to have v1 selected by default if multiple versions are installed. This is more in line with the existing behaviour.

[huesla]

As you can see LibGpiodDriver is now "hiding" the fact which driver version is loaded. Still the types LibGpiodV1Driver and LibGpiodV2Driver are public. They are used in tests and cannot simply be made internal, except if we use "InternalsVisibleTo". I gave this a shot locally, but the compiler complains: error CS1726: Friend assembly referen ce 'System.Device.Gpio.Tests' is invalid. Strong-name signed assemblies must specify a public key in their InternalsVis ibleTo declarations Which route would you go here?

[huesla]

I found another solution. Making tests use the LibGpiodDriverFactory which is driver version aware, and marking the versioned drivers internal. This is the same factory that LibGpiodDriver uses, you know, the thing that is not a driver but a factory...

[pgrawehr]

I found another solution. Making tests use the LibGpiodDriverFactory which is driver version aware, and marking the versioned drivers internal. This is the same factory that LibGpiodDriver uses, you know, the thing that is not a driver but a factory...

That is a good solution I think, particularly because you moved the actual factory to its own class. I'd only make that API a bit more versatile, so that it is future-proof.

[krwq]

[Triage] We've discussed the design a bit and we think it should look like following:

partial class LibGpiodDriver
{
   public LibGpiodVersion Version { get; protected set; }
   // existing
   public LibGpiodDriver Create(int chip); // env var to pick default

   public LibGpiodDriver Create(int chip, LibGpiodVersion version);

   public LibGpiodVersion[] GetAvailableVersions();
}

public enum LibGpiodVersion
{
    LibGpiodV1,
    LibGpiodV2,
}

So the factory can be internal, but the wrapper class gets a method With an overload taking a version. The naming of the enum and its members is still TBD.

When the existing create method is called, an environment variable is checked. If not set, the default is taken. (Reason for this is backwards compatibility, maybe can be dropped later)

Environment to control version DOTNET_IOT_LIBGPIOD_VERSION=<enumName LibGpiodVersion> i.e. DOTNET_IOT_LIBGPIOD_VERSION=LibGpiodV2 specified => always use explicit version but do not allow fallback not specifed => latest

[huesla]

Cool stuff. I can see which path you want to take. Make V1 and V2, as well as the implicit selection of these, internal. The user can pick the version with an ENV variable. Unit tests use the ENV variable to target a versioned driver independently of the access modifier :+1:.

However, the attached example still raises some questions, which I will try to describe as precisely as possible:

Are the methods Create() and GetAvailableVersions() in the provided example meant to be static... shall they replace the constructor? I would guess so. Note that in current main branch LibGpiodDriver has no Create() method, only a constructor taking the chip number.

Second, I am not sure whether I fully get the class structure, it could mean one of the following:

1. LibGpiodDriver is a partial class that contains implementations for both V1 and V2 (maybe in different files), where each method requires an "if" to call either implementation V1 or V2 (etc.)
2. LibGpiodDriver is a base class (partial needed?) that has V1/V2 as sub classes, both internal, that override all base methods. LibGpiodDriver cannot provide any implementation at that point (which normally calls for being declared abstract, but again this would be a breaking change)
3. LibGpiodDriver, V1 and V2 are independent classes (no inheritance, no partial). LibGpiodDriver instantiates one of V1/V2 and simply forwards calls to it.

I would go with 3. either breaking or non breaking:

Non breaking:

public class LibGpiodDriver : UnixDriver
{ 
    private readonly UnixDriver _versionedLibgpiodDriver;

    // exists
    public LibGpiodDriver(int chipNumber = 0)
    {
        // if env var set select that version, if found ok, if not found, fail
        // else pick version automatically based on installed library
        // instantiate _versionedLibgpiodDriver with proper driver implementation (other class)
    }

    // new
    public LibGpiodDriver(int chipNumber = 0, LibGpiodVersion version)
    {
        // choose selected version, or fail if not found
        // instantiate _versionedLibgpiodDriver with proper driver implementation (other class)
    }

    public LibGpiodVersion Version { get; protected set; }

    public LibGpiodVersion[] GetAvailableVersions() => { ... };

    protected internal override int PinCount => _versionedLibgpiodDriver.PinCount;
    ...
}

Breaking:

public class LibGpiodDriver : UnixDriver
{ 
    private UnixDriver _versionedLibgpiodDriver;

    private LibGpiodDriver() { ... }

    // static method instead of constructor
    public LibGpiodDriver Create(int chip)
    {
        // if env var set select that version, if found ok, if not found, fail.
        // else pick version automatically based on installed library
        // instantiate _versionedLibgpiodDriver with proper driver implementation (other class)
     }

    public LibGpiodDriver Create(int chip, LibGpiodVersion version)
    {
        // choose selected version, or fail if not found
        // instantiate _versionedLibgpiodDriver with proper driver implementation (other class)
    }

    public LibGpiodVersion Version { get; protected set; }

    public LibGpiodVersion[] GetAvailableVersions() => { ... };

    protected internal override int PinCount => _versionedLibgpiodDriver.PinCount;
    ...
}

[pgrawehr]

Cool stuff. I can see which path you want to take. Make V1 and V2, as well as the implicit selection of these, internal. The user can pick the version with an ENV variable. Unit tests use the ENV variable to target a versioned driver independently of the access modifier 👍.

Unit tests don't need to use the ENV variable, they can just use the new ctor.

3. `LibGpiodDriver`, `V1` and `V2` are independent classes (no inheritance, no partial). LibGpiodDriver instantiates one of V1/V2 and simply forwards calls to it.

I would go with 3. either breaking or non breaking:

Non breaking:

public class LibGpiodDriver : UnixDriver
{ 
    private readonly UnixDriver _versionedLibgpiodDriver;

    // exists
    public LibGpiodDriver(int chipNumber = 0)
    {
        // if env var set select that version, if found ok, if not found, fail
        // else pick version automatically based on installed library
        // instantiate _versionedLibgpiodDriver with proper driver implementation (other class)
    }

    // new
    public LibGpiodDriver(int chipNumber = 0, LibGpiodVersion version)
    {
        // choose selected version, or fail if not found
        // instantiate _versionedLibgpiodDriver with proper driver implementation (other class)
    }

    public LibGpiodVersion Version { get; protected set; }

    public static LibGpiodVersion[] GetAvailableVersions() => { ... };

    protected internal override int PinCount => _versionedLibgpiodDriver.PinCount;
    ...
}

Sorry, yea, we missed some points here (unfortunately, the team's weekly telephone call is often interrupted by various other meetings, so we need to be quick). I do think Case 3 (non-breaking) is exactly what we meant to have. We have Create() methods in other places, but since here, the ctor is already public, we should stick to that scheme.

The method LibGpiodVersion[] GetAvailableVersions() should be static, though. Otherwise it is of little use, since no instance of LibGpiodDriver can be created when not knowing whether it is available at all.

[huesla]

Unit tests don't need to use the ENV variable, they can just use the new ctor.

True, almost missed that! I like the proposed solution and will implement it soon.

[huesla]

I just pushed new changes. As discussed the driver now has additional constructor taking the driver version, the driver factory is now internal and considers an environment variable. @pgrawehr while writing the code I couldn't find sense in GetAvailableVersions... what should it return? All the values contained in the LibGpiodDriverVersion enum? Or did you deliberately call it "LibGpiodVersion" and not "LibGpiodDriverVersion" i.e. you are talking about the libgpiod version and not the driver version

[pgrawehr]

while writing the code I couldn't find sense in GetAvailableVersions... what should it return?

The intent was to have a function that returns the driver versions that can (most likely) be instantiated. So it would return V1 when the V1 .so exists and/or V2 when the V2 .so exists. Basically the translated return value of GetInstalledLibraries.

[huesla]

@pgrawehr makes sense. I added it now

[pgrawehr]

/azp run dotnet.iot

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[huesla]

/azp run dotnet.iot

The documentation should be fixed now. I don't see why tests are failing though, is libgpiodv2 installed on the build agent?

[pgrawehr]

/azp run dotnet.iot

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[pgrawehr]

/azp run dotnet.iot

The documentation should be fixed now. I don't see why tests are failing though, is libgpiodv2 installed on the build agent?

Unfortunately, there are more documentation issues (yes, the linter is touchy). See https://github.com/dotnet/iot/pull/2168#issuecomment-1826429276 for a tip on how to test that locally.

When that doesn't work, the actual build isn't even started, so we don't see that result. And no, it's very unlikely the runners have v2 installed. They have not been updated in a very long time.

[huesla]

@pgrawehr hopefully I could fix all the md issues this time

[huesla]

API looks fine to me now. I did take a look at some of the implementation details and found some possible improvements. Especially the disposal should be reviewed carefully. See also PR #1861 for a known issue with the existing driver (which we shouldn't copy to the new).

I have carefully read #1849 and PR #1861 to understand the root problem and make sure it does not sneak into V2. So I tried reproducing the issue with the provided code, but I wasn't able to cause a segfault. Unsurprisingly, because the example expects the controller objects to be garbage collected (it calls GC.Collect) even though these objects live in the same scope (?) So I tried other scenarios, but even the simplest one where first the chip is freed, then the line is released, no segfault occured.

I somehow have the feeling that either the analysis or the provided reproduction code of the issue isn't 100% right.

Nonetheless, I can't see resource release issues in the V2 implementation as it heavily relies on SafeHandles (and freeing only once). And as mentioned in another comment, releasing them seems to be independent of each other. (Even though that is not documented, and so far only proven by tests).

[pgrawehr]

/azp run dotnet.iot

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[pgrawehr]

There's some problem with the build too: You can ignore the Windows builds for now (what's failing there is some strange compatibility check), but the Linux Helix tests (which run the code on actual Raspberry Pi 3's) fails with an error about LibGpiod not found. You should take a look at that.

[huesla]

There's some problem with the build too: You can ignore the Windows builds for now (what's failing there is some strange compatibility check), but the Linux Helix tests (which run the code on actual Raspberry Pi 3's) fails with an error about LibGpiod not found. You should take a look at that.

The LibGpiodDriverV2Tests expect libgpiodv2 (.so3) to be installed, otherwise they fail. Who can/may take care of that?

[pgrawehr]

The LibGpiodDriverV2Tests expect libgpiodv2 (.so3) to be installed, otherwise they fail. Who can/may take care of that?

We can look into that (nobody has touched those in a while, so I dono), but the issue is not with V2, but with V1. Here's the relevant error:

 System.Device.Gpio.Tests.LibGpiodV1DriverTests.AddCallbackRemoveAllCallbackTest [FAIL]
      System.Device.Gpio.Drivers.GpiodException : No suitable libgpiod library found for LibGpiodDriverVersion.V1. Supported versions: libgpiod.so.0, libgpiod.so.1, libgpiod.so.2
      Searched paths: /lib, /usr/lib, /usr/local/lib
      Stack Trace:
        /_/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriverFactory.cs(128,0): at System.Device.Gpio.Drivers.LibGpiodDriverFactory.CreateInternal(LibGpiodDriverVersion version, Int32 chipNumber)
        /_/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriverFactory.cs(106,0): at System.Device.Gpio.Drivers.LibGpiodDriverFactory.Create(Int32 chipNumber, LibGpiodDriverVersion driverVersion)
        /_/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriver.cs(38,0): at System.Device.Gpio.Drivers.LibGpiodDriver..ctor(Int32 gpioChip, LibGpiodDriverVersion driverVersion)
        /_/src/System.Device.Gpio.Tests/LibGpiodV1DriverTests.cs(22,0): at System.Device.Gpio.Tests.LibGpiodV1DriverTests.GetTestDriver()
        /_/src/System.Device.Gpio.Tests/GpioControllerTestBase.cs(320,0): at System.Device.Gpio.Tests.GpioControllerTestBase.AddCallbackRemoveAllCallbackTest()

[huesla]

We can look into that (nobody has touched those in a while, so I dono), but the issue is not with V2, but with V1. Here's the relevant error: ...

The exception message is a bit misleading, since it does not include the installed library file names. I will add this in my next commit. The pipeline logs complain about V1 and also V2 so there is definitely an issue with finding the library and thus with the factory, because I assume this previously worked. We could try to find out where the library is actually installed on the build agent and why the factory doesn't find it, but the framework in the previous implementation could. Something like sudo find / -iname "libgpiod.so*" If it turns out that the library is installed not in an expected directory, we could think further what to do. Btw. I am testing this locally, where libgpiod is installed under /usr/local/lib/

[pgrawehr]

/azp run dotnet.iot

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[huesla]

Open points:

1. Who ever has the rights, should investigate why the tests fail on this pipeline, i.e. find out why the factory implementation does not find libgpiod in the standard paths (where is libgpiod installed?)
2. Depending on 1. find out what to do with the factory implementation. I am almost sure that the previous implementation could find the library, and it was utilizing DllImport for that. We may want to discuss using NativeLibrary only for this, but it would mean dropping netstandard (as pointed out in some comment).

[pgrawehr]

/azp run dotnet.iot

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[pgrawehr]

@huesla I've tried to fix the remaining issues, but the thing with the library search path is complicated. On my Rpi4 (with 64 bit Raspberry Pi OS) the correct path for libgpiod.so.2 is /lib/aarch64-linux-gnu/libgpiod.so.2. The only way to get to this path appears to be the output of ldconfig -p. This tool shows the search path of the dynamic linker. I'm not sure why this is different for you. Which operating system are you using?

[huesla]

@pgrawehr thanks for your investigation. I just did a fresh install of 32/64 bit RPI OS on Pi4/Pi5. On both Pi's the result was:

• 32 Bit: /usr/lib/arm-linux-gnueabihf/libgpiod.so.2.2.2
• 64 Bit: /usr/lib/aarch64-linux-gnu/libgpiod.so.2.2.2

I think on my previous system setup the libraries ended up in "/usr/lib" because I may have compiled and installed them manually only. But under Debian there is multi arch support. An easy fix would be to make the search recursive, going downwards starting from "/usr/lib", wdyt?

[pgrawehr]

Sounds like a plan. I was first thinking about parsing the output of ldconfig, but that's quite ugly to do. Of course, recursively iterating the subfolders of /usr/lib could theoretically find the wrong library (e.g. the 32bit library for a 64 bit process), but normally you would have either both or none, I suppose.

[huesla]

@pgrawehr the search is now recursive. Should we enable tests again?

[pgrawehr]

/azp run dotnet.iot

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[huesla]

@pgrawehr the V1 tests succeeded, can we run V2 aswell? (they are still disabled)

[pgrawehr]

@huesla No, because the devices have not been upgraded yet. That may take some time, but I will push that further. I don't have the necessary permissions myself. When the old driver works, it's fine for me to merge anyway.

[huesla]

Great work and great collaboration in this PR. I've been following the different threads and progress.

Thanks to all for the collaboration aswell. If there is anything else to improve, let me know