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

Make LibGpiodDriver V2 driver public #2386

Merged
merged 44 commits into from
Apr 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
bd04378
Clean up LibGpiodDriver interface
pgrawehr Feb 1, 2025
36d12ee
Provide new GpioChipInfo static method
pgrawehr Feb 8, 2025
73d3791
Undo previous change
pgrawehr Feb 14, 2025
f6c93ca
Revert "Clean up LibGpiodDriver interface"
pgrawehr Feb 14, 2025
deee7f0
Revert to old behavior: LibgpiodDriver is V1, V2 is separate
pgrawehr Feb 14, 2025
51a9931
This was now doubled
pgrawehr Feb 15, 2025
13bb848
Add test for SysFs
pgrawehr Feb 16, 2025
106323b
Implement method
pgrawehr Feb 16, 2025
831cea2
Why does this not fit?
pgrawehr Feb 16, 2025
2dab36c
Something is strange here
pgrawehr Feb 16, 2025
eed3ece
There's a bug here
pgrawehr Feb 16, 2025
5fd5272
Better now?
pgrawehr Feb 16, 2025
364fd45
Provide implementation for LibgpiodV1
pgrawehr Feb 16, 2025
9aa4959
Need to debug this
pgrawehr Feb 16, 2025
c568085
Avoid crash, maybe?
pgrawehr Feb 16, 2025
cffd647
Could be working
pgrawehr Feb 16, 2025
ae2658a
No duplicates here
pgrawehr Feb 16, 2025
747e778
Why is that crashing?
pgrawehr Feb 16, 2025
cca6685
Disable this test, because it seems it does now crash
pgrawehr Feb 16, 2025
9356a87
Something crashes very ugly here
pgrawehr Feb 16, 2025
accbef1
Make sure this test is the culprit
pgrawehr Feb 16, 2025
e934b50
Revert previous change
pgrawehr Feb 16, 2025
278edb4
Maybe not closing the stuff avoids the crash
pgrawehr Feb 16, 2025
896a074
No crashes any more?
pgrawehr Feb 16, 2025
dd9fc5f
Chip info for V2 completed
pgrawehr Feb 27, 2025
0ecf0ff
Add Test for V2
pgrawehr Mar 1, 2025
ef00529
Minor fix
pgrawehr Mar 1, 2025
f6d1082
Fix #2384: Avoid exception when toggling a pin whose current value is…
pgrawehr Mar 1, 2025
4b3525c
Stylecop error fixed
pgrawehr Mar 2, 2025
9d33c73
Improve parsing
pgrawehr Mar 2, 2025
3c76e01
This should be the right pattern
pgrawehr Mar 2, 2025
d7f7012
Try to make it work on RPI3, too
pgrawehr Mar 2, 2025
8d627b9
Something behaves differently on the RPI3
pgrawehr Mar 2, 2025
6d34f83
Stupid me
pgrawehr Mar 2, 2025
4d611d9
Is a directory
pgrawehr Mar 2, 2025
761af06
Cleanup
pgrawehr Mar 2, 2025
fef36a8
Use new method to fix a TODO
pgrawehr Mar 10, 2025
a9f1a7d
Make it more clear how this handle is used
pgrawehr Mar 15, 2025
18f51af
Review findings
pgrawehr Mar 15, 2025
119e847
Adjust namespaces
pgrawehr Mar 15, 2025
d13bde4
That didn't compile
pgrawehr Mar 16, 2025
a921b74
Mark new stuff experimental for now
pgrawehr Mar 22, 2025
e56f9fb
Include ChipInfo in QueryComponentInformation
pgrawehr Mar 27, 2025
4a32169
Some more review findings
pgrawehr Mar 29, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/System.Device.Gpio.Tests/GpioControllerTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Device.Gpio.Drivers;
using System.Diagnostics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Xunit;
Expand All @@ -23,6 +24,8 @@ protected GpioControllerTestBase(ITestOutputHelper testOutputHelper)
_testOutputHelper = testOutputHelper;
}

protected ITestOutputHelper Logger => _testOutputHelper;

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down Expand Up @@ -624,4 +627,18 @@ public void UsingControllerAfterDisposeCausesException()
}

protected abstract GpioDriver GetTestDriver();

protected bool IsRaspi4()
{
if (File.Exists("/proc/device-tree/model"))
{
string model = File.ReadAllText("/proc/device-tree/model", Text.Encoding.ASCII);
if (model.Contains("Raspberry Pi 4") || model.Contains("Raspberry Pi Compute Module 4"))
{
return true;
}
}

return false;
}
}
34 changes: 30 additions & 4 deletions src/System.Device.Gpio.Tests/LibGpiodV1DriverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Device.Gpio.Drivers.Libgpiod.V1;
using System.Diagnostics;
using System.Threading;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -21,9 +21,7 @@ public LibGpiodV1DriverTests(ITestOutputHelper testOutputHelper)
{
}

#pragma warning disable SDGPIO0001
protected override GpioDriver GetTestDriver() => new LibGpiodDriver(0, LibGpiodDriverVersion.V1);
#pragma warning restore SDGPIO0001
protected override GpioDriver GetTestDriver() => new LibGpiodDriver(0);

[Fact]
public void SetPinModeSetsDefaultValue()
Expand Down Expand Up @@ -106,4 +104,32 @@ public void LeakingDriverDoesNotCrash()

GC.WaitForPendingFinalizers();
}

[Fact]
public void CheckAllChipsCanBeConstructed()
{
var chips = LibGpiodDriver.GetAvailableChips();
foreach (var c in chips)
{
Logger.WriteLine(c.ToString());
}

Assert.NotEmpty(chips);
if (IsRaspi4())
{
// 2 entries. Here they're always named 0 and 1
Assert.Equal(2, chips.Count);
}

foreach (var chip in chips)
{
var driver = new LibGpiodDriver(chip.Id);
var ctrl = new GpioController(driver);
Assert.NotNull(ctrl);
var driverInfo = driver.GetChipInfo();
Assert.NotNull(driverInfo);
Assert.Equal(chip, driverInfo);
ctrl.Dispose();
}
}
}
32 changes: 29 additions & 3 deletions src/System.Device.Gpio.Tests/LibGpiodV2DriverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Diagnostics;
using System.Threading.Tasks;
using Xunit;
using Xunit.Abstractions;

Expand All @@ -22,6 +20,34 @@ public LibGpiodV2DriverTests(ITestOutputHelper testOutputHelper)
{
}

[Fact]
public void CheckAllChipsCanBeConstructed()
{
var chips = LibGpiodV2Driver.GetAvailableChips();
foreach (var c in chips)
{
Logger.WriteLine(c.ToString());
}

Assert.NotEmpty(chips);
if (IsRaspi4())
{
// 3 real ones and the default 0 entry
Assert.Equal(3, chips.Count);
}

foreach (var chip in chips)
{
var driver = new LibGpiodV2Driver(chip.Id);
var ctrl = new GpioController(driver);
Assert.NotNull(ctrl);
var driverInfo = driver.GetChipInfo();
Assert.NotNull(driverInfo);
Assert.Equal(chip, driverInfo);
ctrl.Dispose();
}
}

#pragma warning disable SDGPIO0001
protected override GpioDriver GetTestDriver() => new LibGpiodDriver(ChipNumber, LibGpiodDriverVersion.V2);
protected override GpioDriver GetTestDriver() => new LibGpiodV2Driver(ChipNumber);
}
14 changes: 0 additions & 14 deletions src/System.Device.Gpio.Tests/RaspberryPiDriverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,6 @@ public RaspberryPiDriverTests(ITestOutputHelper testOutputHelper)

protected override GpioDriver GetTestDriver() => new RaspberryPi3Driver();

private bool IsRaspi4()
{
if (File.Exists("/proc/device-tree/model"))
{
string model = File.ReadAllText("/proc/device-tree/model", Text.Encoding.ASCII);
if (model.Contains("Raspberry Pi 4") || model.Contains("Raspberry Pi Compute Module 4"))
{
return true;
}
}

return false;
}

/// <summary>
/// Tests for setting the pull up/pull down resistors on the Raspberry Pi (supported on Pi3 and Pi4, but with different techniques)
/// </summary>
Expand Down
38 changes: 38 additions & 0 deletions src/System.Device.Gpio.Tests/SysFsDriverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.IO;
using Xunit;
using Xunit.Abstractions;

#pragma warning disable SDGPIO0001
namespace System.Device.Gpio.Tests;

[Trait("requirement", "root")]
Expand All @@ -19,4 +21,40 @@ public SysFsDriverTests(ITestOutputHelper testOutputHelper)
}

protected override GpioDriver GetTestDriver() => new SysFsDriver();

[Fact]
public void CheckAllChipsCanBeConstructed()
{
string[] fileNames = Directory.GetFileSystemEntries("/sys/class/gpio", $"*", SearchOption.TopDirectoryOnly);
Logger.WriteLine("Content of /sys/class/gpio:");
foreach (var f in fileNames)
{
Logger.WriteLine(f);
}

var chips = SysFsDriver.GetAvailableChips();
Logger.WriteLine("Available chips:");
foreach (var c in chips)
{
Logger.WriteLine(c.ToString());
}

Assert.NotEmpty(chips);
if (IsRaspi4())
{
// 2 real ones and the default 0 entry
Assert.Equal(3, chips.Count);
}

foreach (var chip in chips)
{
var driver = new SysFsDriver(chip);
var ctrl = new GpioController(driver);
Assert.NotNull(ctrl);
var driverInfo = driver.GetChipInfo();
Assert.NotNull(driverInfo);
Assert.Equal(chip, driverInfo);
ctrl.Dispose();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,25 @@ static LibgpiodV1()
}

/// <summary>
/// Release all resources allocated for the gpiochip iterator and close the most recently opened gpiochip(if any).
/// Release all resources allocated for the gpiochip iterator but does not close the most recently opened gpiochip
/// </summary>
/// <param name="iter">The gpiochip iterator object</param>
[DllImport(LibgpiodLibrary)]
internal static extern void gpiod_chip_iter_free(IntPtr iter);
internal static extern void gpiod_chip_iter_free_noclose(IntPtr iter);

/// <summary>
/// Start an iteration over the list of gpio chips.
/// </summary>
[DllImport(LibgpiodLibrary)]
internal static extern IntPtr gpiod_chip_iter_new();

/// <summary>
/// Iterate over a chip iterator. Does not close any previously opened chips.
/// </summary>
/// <param name="handle">Handle of a chip iterator</param>
/// <returns>The next chip handle or null if at the end of the iteration</returns>
[DllImport(LibgpiodLibrary)]
internal static extern IntPtr gpiod_chip_iter_next_noclose(SafeChipIteratorHandle handle);

/// <summary>
/// Close a GPIO chip handle and release all allocated resources.
Expand Down Expand Up @@ -195,14 +209,20 @@ static LibgpiodV1()
/// </summary>
/// <returns>GPIO chip pointer handle or NULL if an error occurred.</returns>
[DllImport(LibgpiodLibrary, SetLastError = true)]
internal static extern SafeChipHandle gpiod_chip_open_by_number(int number);
internal static extern IntPtr gpiod_chip_open_by_number(int number);

/// <summary>
/// Get the API version of the library as a human-readable string.
/// </summary>
/// <returns>Human-readable string containing the library version.</returns>
[DllImport(LibgpiodLibrary, SetLastError = true)]
internal static extern IntPtr gpiod_version_string();

[DllImport(LibgpiodLibrary)]
internal static extern IntPtr gpiod_chip_name(SafeChipHandle chip);

[DllImport(LibgpiodLibrary)]
internal static extern IntPtr gpiod_chip_label(SafeChipHandle chip);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ namespace System.Device.Gpio.Libgpiod.V1;
/// </summary>
internal class SafeChipHandle : SafeHandle
{
public SafeChipHandle()
: base(IntPtr.Zero, true)
public SafeChipHandle(IntPtr h)
: base(h, true)
{
}

protected override bool ReleaseHandle()
{
LibgpiodV1.gpiod_chip_close(handle);
SetHandle(IntPtr.Zero);
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ namespace System.Device.Gpio.Libgpiod.V1;
/// </summary>
internal class SafeChipIteratorHandle : SafeHandle
{
public SafeChipIteratorHandle()
: base(IntPtr.Zero, true)
public SafeChipIteratorHandle(IntPtr handle)
: base(handle, true)
{
}

protected override bool ReleaseHandle()
{
LibgpiodV1.gpiod_chip_iter_free(handle);
// We can't close the chip here, as this would possibly result in it being freed twice, which causes a crash
LibgpiodV1.gpiod_chip_iter_free_noclose(handle);
handle = IntPtr.Zero;
Copy link
Member

Choose a reason for hiding this comment

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

return true;
}

Expand Down
44 changes: 6 additions & 38 deletions src/System.Device.Gpio/Interop/Unix/libgpiod/V2/Proxies/Chip.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,19 @@ namespace System.Device.Gpio.Libgpiod.V2;
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class Chip : LibGpiodProxyBase
{
private readonly int _chipNumber;
private readonly ChipSafeHandle _handle;

/// <summary>
/// Constructor for a chip-proxy object. This call will try to open the chip.
/// </summary>
/// <param name="chipNumber">The chip number</param>
/// <param name="handle">Safe handle to the libgpiod object.</param>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__chips.html#ga25097f48949d0ac81e9ab341193da1a4"/>
public Chip(ChipSafeHandle handle)
public Chip(int chipNumber, ChipSafeHandle handle)
: base(handle)
{
_chipNumber = chipNumber;
_handle = handle;
}

Expand All @@ -46,7 +49,7 @@ public string GetPath()
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__chips.html#gad65098ba48da0f22d1b0be149b4cf578"/>
/// <exception cref="GpiodException">Unexpected error invoking native function</exception>
public ChipInfo GetInfo()
public LibGpiodChipInfo GetInfo()
{
return CallLibgpiod(() =>
{
Expand All @@ -57,7 +60,7 @@ public ChipInfo GetInfo()
throw new GpiodException($"Could not get chip info: {LastErr.GetMsg()}");
}

return new ChipInfo(chipInfoHandle);
return new LibGpiodChipInfo(_chipNumber, chipInfoHandle);
});
}

Expand Down Expand Up @@ -207,39 +210,4 @@ public LineRequest RequestLines(RequestConfig requestConfig, LineConfig lineConf
return new LineRequest(lineRequestHandle);
});
}

/// <summary>
/// Helper function for capturing information and creating an immutable snapshot instance.
/// </summary>
/// <exception cref="GpiodException">Unexpected error when invoking native function</exception>
public Snapshot MakeSnapshot()
{
using var chipInfo = GetInfo();
var chipInfoSnapshot = chipInfo.MakeSnapshot();
int numLines = chipInfoSnapshot.NumLines;
List<LineInfo.Snapshot> lineInfos = new();
for (uint offset = 0; offset < numLines; offset++)
{
using var lineInfo = GetLineInfo(offset);
var lineInfoSnapshot = lineInfo.MakeSnapshot();
lineInfos.Add(lineInfoSnapshot);
}

return new Snapshot(chipInfoSnapshot, GetPath(), GetFileDescriptor(), lineInfos);
}

/// <summary>
/// Contains all readable information that was recorded at one and the same time
/// </summary>
public sealed record Snapshot(ChipInfo.Snapshot ChipInfo, string Path, int FileDescriptor, IEnumerable<LineInfo.Snapshot> LineInfos)
{
/// <summary>
/// Converts the whole snapshot to string
/// </summary>
public override string ToString()
{
return
$"{nameof(ChipInfo)}: {ChipInfo}, {nameof(Path)}: {Path}, {nameof(FileDescriptor)}: {FileDescriptor}, {nameof(LineInfos)}: {string.Join("\n", LineInfos)}";
}
}
}
Loading