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

Adding GpioPin #1895

Merged
merged 18 commits into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions src/System.Device.Gpio.Tests/GpioControllerSoftwareTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ public void PinCountReportedCorrectly()
}

[Fact]
public void OpenTwiceThrows()
public void OpenTwiceGpioPinAndClosedTwiceThrows()
{
var ctrl = new GpioController(PinNumberingScheme.Logical, _mockedGpioDriver.Object);
_mockedGpioDriver.Setup(x => x.OpenPinEx(1));
_mockedGpioDriver.Setup(x => x.ClosePinEx(1));
ctrl.OpenPin(1);
Assert.Throws<InvalidOperationException>(() => ctrl.OpenPin(1));
GpioPin gpioPin1 = ctrl.OpenPin(1);
GpioPin gpiopin2 = ctrl.OpenPin(1);
Assert.Equal(gpioPin1, gpiopin2);
ctrl.ClosePin(1);
Assert.Throws<InvalidOperationException>(() => ctrl.ClosePin(1));
}
Expand Down
83 changes: 83 additions & 0 deletions src/System.Device.Gpio.Tests/GpioPinTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Text;
using Moq;
using Xunit;

namespace System.Device.Gpio.Tests
{
public class GpioPinTests : IDisposable
{
private const int PinNumber = 2;
private Mock<MockableGpioDriver> _mockedGpioDriver;

public GpioPinTests()
{
_mockedGpioDriver = new Mock<MockableGpioDriver>(MockBehavior.Default);
_mockedGpioDriver.CallBase = true;
}

public void Dispose()
{
_mockedGpioDriver.VerifyAll();
}

[Fact]
public void TestOpenPin()
{
// Arrange
_mockedGpioDriver.Setup(x => x.OpenPinEx(PinNumber));
_mockedGpioDriver.Setup(x => x.IsPinModeSupportedEx(PinNumber, It.IsAny<PinMode>())).Returns(true);
_mockedGpioDriver.Setup(x => x.SetPinModeEx(PinNumber, It.IsAny<PinMode>()));
_mockedGpioDriver.Setup(x => x.GetPinModeEx(PinNumber)).Returns(PinMode.Input);
var ctrl = new GpioController(PinNumberingScheme.Logical, _mockedGpioDriver.Object);
// Act
GpioPin pin = ctrl.OpenPin(PinNumber, PinMode.Input);
// Assert
Assert.Equal(pin.PinNumber, PinNumber);
Assert.Equal(PinMode.Input, pin.GetPinMode());
}

[Fact]
public void TestClosePin()
{
// Arrange
_mockedGpioDriver.Setup(x => x.OpenPinEx(PinNumber));
_mockedGpioDriver.Setup(x => x.IsPinModeSupportedEx(PinNumber, It.IsAny<PinMode>())).Returns(true);
_mockedGpioDriver.Setup(x => x.SetPinModeEx(PinNumber, It.IsAny<PinMode>()));
var ctrl = new GpioController(PinNumberingScheme.Logical, _mockedGpioDriver.Object);
// Act
GpioPin pin = ctrl.OpenPin(PinNumber, PinMode.Input);
ctrl.ClosePin(PinNumber);
// Assert
// This should work even if the pin is closed in the controller as the driver has no idea
// Of the controller behavior.
var ret = pin.Read();
}

[Fact]
public void TestToggleReadWrite()
{
// Arrange
PinValue pinValue = PinValue.High;
_mockedGpioDriver.Setup(x => x.OpenPinEx(PinNumber));
_mockedGpioDriver.Setup(x => x.IsPinModeSupportedEx(PinNumber, It.IsAny<PinMode>())).Returns(true);
_mockedGpioDriver.Setup(x => x.SetPinModeEx(PinNumber, It.IsAny<PinMode>()));
_mockedGpioDriver.Setup(x => x.ReadEx(PinNumber)).Returns(pinValue);
var ctrl = new GpioController(PinNumberingScheme.Logical, _mockedGpioDriver.Object);
// Act
GpioPin pin = ctrl.OpenPin(PinNumber, PinMode.Input);
pin.Write(pinValue);
// Assert
Assert.Equal(pinValue, pin.Read());
pin.Toggle();
// Make sure we setup the drive properly
pinValue = !pinValue;
_mockedGpioDriver.Setup(x => x.ReadEx(PinNumber)).Returns(pinValue);
Assert.Equal(pinValue, pin.Read());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Runtime.InteropServices;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Net.NetworkInformation;

namespace System.Device.Gpio.Drivers;

Expand All @@ -21,6 +22,7 @@ public class LibGpiodDriver : UnixDriver
private readonly ConcurrentDictionary<int, SafeLineHandle> _pinNumberToSafeLineHandle;
private readonly ConcurrentDictionary<int, LibGpiodDriverEventHandler> _pinNumberToEventHandler;
private readonly int _pinCount;
private readonly ConcurrentDictionary<int, PinValue> _pinValue;
Comment on lines 22 to +25
Copy link
Member

Choose a reason for hiding this comment

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

I'm looking at this and wondering - perhaps GpioDriver could have virtual CreateGpioPin with default implementation - then in here you'd return LibGpiodPin and move CurrentPinValue, EventHandler, SafeLineHandle to LibGpiodPin - this way you could have just one ConcurrentDictionary here and using pin directly would have advantage of not touching ConcurrentDictionary

Copy link
Member

@krwq krwq Nov 18, 2022

Choose a reason for hiding this comment

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

we talked offline and we can do this as a separate change most likely without any API changes (only addition suggested here)

private SafeChipHandle _chip;

/// <inheritdoc />
Expand Down Expand Up @@ -76,6 +78,7 @@ public LibGpiodDriver(int gpioChip = 0)
_pinCount = Interop.libgpiod.gpiod_chip_num_lines(_chip);
_pinNumberToEventHandler = new ConcurrentDictionary<int, LibGpiodDriverEventHandler>();
_pinNumberToSafeLineHandle = new ConcurrentDictionary<int, SafeLineHandle>();
_pinValue = new ConcurrentDictionary<int, PinValue>();
}
catch (DllNotFoundException)
{
Expand Down Expand Up @@ -134,6 +137,7 @@ protected internal override void ClosePin(int pinNumber)
pinHandle?.Dispose();
// We know this works
_pinNumberToSafeLineHandle.TryRemove(pinNumber, out _);
_pinValue.TryRemove(pinNumber, out _);
}
}
}
Expand Down Expand Up @@ -211,6 +215,9 @@ protected internal override void OpenPin(int pinNumber)
}

_pinNumberToSafeLineHandle.TryAdd(pinNumber, pinHandle);
// This is setting up a default value without reading the driver as it's the default behavior.
// If the Setmode with an initial value is used, this is going to be corrected automatically
_pinValue.TryAdd(pinNumber, PinValue.Low);
}
}

Expand All @@ -225,12 +232,16 @@ protected internal override PinValue Read(int pinNumber)
throw ExceptionHelper.GetIOException(ExceptionResource.ReadPinError, Marshal.GetLastWin32Error(), pinNumber);
}

_pinValue[pinNumber] = result;
return result;
}

throw ExceptionHelper.GetInvalidOperationException(ExceptionResource.PinNotOpenedError, pin: pinNumber);
}

/// <inheritdoc/>
protected internal override void Toggle(int pinNumber) => Write(pinNumber, !_pinValue[pinNumber]);

/// <inheritdoc/>
protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNumber, PinChangeEventHandler callback)
{
Expand Down Expand Up @@ -307,6 +318,7 @@ protected internal override void SetPinMode(int pinNumber, PinMode mode, PinValu
pinNumber);
}

_pinValue[pinNumber] = initialValue;
pinHandle.PinMode = mode;
return;
}
Expand Down Expand Up @@ -372,6 +384,7 @@ protected internal override void Write(int pinNumber, PinValue value)
}

Interop.libgpiod.gpiod_line_set_value(pinHandle, (value == PinValue.High) ? 1 : 0);
_pinValue[pinNumber] = value;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -399,6 +412,7 @@ protected override void Dispose(bool disposing)
}

_pinNumberToSafeLineHandle.Clear();
_pinValue.Clear();
}

_chip?.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ protected internal override int ConvertPinNumberToLogicalNumberingScheme(int pin
/// <inheritdoc/>
protected internal override PinValue Read(int pinNumber) => InternalDriver.Read(pinNumber);

/// <inheritdoc/>
protected internal override void Toggle(int pinNumber) => InternalDriver.Toggle(pinNumber);

/// <inheritdoc/>
protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNumber, PinChangeEventHandler callback) => InternalDriver.RemoveCallbackForPinValueChangedEvent(pinNumber, callback);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,13 @@ protected internal unsafe override PinValue Read(int pinNumber)
return Convert.ToBoolean((register >> (pinNumber % 32)) & 1) ? PinValue.High : PinValue.Low;
}

/// <inheritdoc/>
protected internal override void Toggle(int pinNumber)
{
ValidatePinNumber(pinNumber);
_interruptDriver!.Toggle(pinNumber);
}

/// <summary>
/// Removes a handler for a pin value changed event.
/// </summary>
Expand Down
10 changes: 10 additions & 0 deletions src/System.Device.Gpio/System/Device/Gpio/Drivers/SysFsDriver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class SysFsDriver : UnixDriver

private readonly List<int> _exportedPins = new List<int>();
private readonly Dictionary<int, UnixDriverDevicePin> _devicePins = new Dictionary<int, UnixDriverDevicePin>();
private readonly Dictionary<int, PinValue> _pinValues = new Dictionary<int, PinValue>();
private TimeSpan _statusUpdateSleepTime = TimeSpan.FromMilliseconds(1);
private int _pollFileDescriptor = -1;
private Thread? _eventDetectionThread;
Expand Down Expand Up @@ -118,6 +119,8 @@ protected internal override void OpenPin(int pinNumber)
SysFsHelpers.EnsureReadWriteAccessToPath(pinPath);

_exportedPins.Add(pinNumber);
// Default value is low, otherwise it's the set pin mode with default value that will override this
_pinValues.Add(pinNumber, PinValue.Low);
}
catch (UnauthorizedAccessException e)
{
Expand Down Expand Up @@ -145,6 +148,7 @@ protected internal override void ClosePin(int pinNumber)
{
_devicePins[pinNumber].Dispose();
_devicePins.Remove(pinNumber);
_pinValues.Remove(pinNumber);
}

// If this controller wasn't the one that opened the pin, then Remove will return false, so we don't need to close it.
Expand Down Expand Up @@ -248,9 +252,13 @@ protected internal override PinValue Read(int pinNumber)
throw new InvalidOperationException("There was an attempt to read from a pin that is not open.");
}

_pinValues[pinNumber] = result;
return result;
}

/// <inheritdoc/>
protected internal override void Toggle(int pinNumber) => Write(pinNumber, !_pinValues[pinNumber]);

private PinValue ConvertSysFsValueToPinValue(string value)
{
return value.Trim() switch
Expand All @@ -275,6 +283,7 @@ protected internal override void Write(int pinNumber, PinValue value)
{
string sysFsValue = ConvertPinValueToSysFs(value);
File.WriteAllText(valuePath, sysFsValue);
_pinValues[pinNumber] = value;
}
catch (UnauthorizedAccessException e)
{
Expand Down Expand Up @@ -588,6 +597,7 @@ protected override void Dispose(bool disposing)
}

_devicePins.Clear();
_pinValues.Clear();
if (_pollFileDescriptor != -1)
{
Interop.close(_pollFileDescriptor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public class Windows10Driver : GpioDriver
{
private static readonly WinGpio.GpioController s_winGpioController = WinGpio.GpioController.GetDefault();
private readonly Dictionary<int, Windows10DriverPin> _openPins = new Dictionary<int, Windows10DriverPin>();
private readonly Dictionary<int, PinValue> _pinValues = new Dictionary<int, PinValue>();
Copy link
Member

Choose a reason for hiding this comment

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

why does LibGpiodDriver need ConcurrentDictionary while this one is ok with regular Dictionary?

Copy link
Member

Choose a reason for hiding this comment

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

(it's an existing issue anyway so don't worry about this in this PR)


/// <summary>
/// Initializes a new instance of the <see cref="Windows10Driver"/> class.
Expand Down Expand Up @@ -68,6 +69,7 @@ protected internal override void ClosePin(int pinNumber)
{
pin.ClosePin();
_openPins.Remove(pinNumber);
_pinValues.Remove(pinNumber);
}
}

Expand Down Expand Up @@ -113,14 +115,24 @@ protected internal override void OpenPin(int pinNumber)

WinGpio.GpioPin windowsPin = s_winGpioController.OpenPin(pinNumber, WinGpio.GpioSharingMode.Exclusive);
_openPins[pinNumber] = new Windows10DriverPin(this, windowsPin);
// Default value, override by the set pin mode with another default value
_pinValues[pinNumber] = PinValue.Low;
}

/// <summary>
/// Reads the current value of a pin.
/// </summary>
/// <param name="pinNumber">The pin number in the driver's logical numbering scheme.</param>
/// <returns>The value of the pin.</returns>
protected internal override PinValue Read(int pinNumber) => LookupOpenPin(pinNumber).Read();
protected internal override PinValue Read(int pinNumber)
{
var value = LookupOpenPin(pinNumber).Read();
_pinValues[pinNumber] = value;
return value;
}

/// <inheritdoc/>
protected internal override void Toggle(int pinNumber) => Write(pinNumber, !_pinValues[pinNumber]);

/// <summary>
/// Removes a handler for a pin value changed event.
Expand Down Expand Up @@ -152,7 +164,11 @@ protected internal override WaitForEventResult WaitForEvent(int pinNumber, PinEv
/// </summary>
/// <param name="pinNumber">The pin number in the driver's logical numbering scheme.</param>
/// <param name="value">The value to be written to the pin.</param>
protected internal override void Write(int pinNumber, PinValue value) => LookupOpenPin(pinNumber).Write(value);
protected internal override void Write(int pinNumber, PinValue value)
{
LookupOpenPin(pinNumber).Write(value);
_pinValues[pinNumber] = value;
}

/// <summary>
/// Lookups an open pin in the driver.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ protected internal override PinValue Read(int pinNumber)
throw new PlatformNotSupportedException();
}

/// <inheritdoc />
protected internal override void Toggle(int pinNumber)
{
throw new PlatformNotSupportedException();
}

/// <inheritdoc />
protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNumber, PinChangeEventHandler callback)
{
Expand Down
Loading