[API Proposal]: Adding GpioPin to GpioController in .NET IoT

[Ellerbach]

Background and motivation

In .NET IoT (https://github.com/dotnet/iot/) we do have a GpioController allowing operations on GPIO (referred as pin later). To open, write, read or close a pin, it is necessary to use the GpioController.
We want to propose adding a GpioPin class that will make it easier to access directly a pin.

This GpioPin is already present and used in .NET nanoFramework (https://github.com/nanoframework/). We've been working to align as best as possible all hardware related API between .NET IoT and .NET nanoFramework. So GPIO, SPI, I2C, PWM, Serial API are aligned. Due to the platform differences, there are minor differences.

The GpioPin in .NET nanoFramework can be found here: https://github.com/nanoframework/System.Device.Gpio/blob/main/System.Device.Gpio/GpioPin.cs

Adding a GpioPin makes it easy for simple operations and a simpler concept as well for beginners. It is important to keep in mind that only the GpioController is allow to open pins and is responsible ultimately for the life of the GpioPin. You cannot open a GpioPin without the GpioController. You can dispose your GpioPin which will close it in the GpioController. And if the GpioController is disposed, the GpioPin won't be able to operate the pin. This deisgn exist in .NET nanoFramework and is very sucessfull. It was existing before the GpioController API aligned and based on developers' feedback has been kept.

Adding this GpioPin to the GpioController will result in a breaking change in the OpenPin function. This is not a code breaking change, it is a binary breaking change. See the risk section on this.

When using multiple GpioControllers or with specific controllers like the FT family, this can be an advantage to simplify the management of the pins. It could allow some better performance as well in those scenarios.

An active issue is open in .NET IoT which also reference other closed issues with more detailed discussions: #1671

API Proposal

namespace System.Device.Gpio;

public class GpioController : IDisposable
{
// Changing current OpenPin overloads to return a GpioPin instead of void. Binary breaking, non source breaking.
-   public void OpenPin(int pinNumber) { }
+   public GpioPin OpenPin(int pinNumber) { }

-   public void OpenPin(int pinNumber, PinMode mode) { }
+   public GpioPin OpenPin(int pinNumber, PinMode mode) { }

-   public void OpenPin(int pinNumber, PinMode mode, PinValue initialValue) { }
+   public GpioPin OpenPin(int pinNumber, PinMode mode, PinValue initialValue) { }
}

+public sealed class GpioPin
+{
+    public int PinNumber { get; }
+    public PinMode GetPinMode() { }
+    public bool IsPinModeSupported(PinMode pinMode) { }
+    public void SetPinMode(PinMode value) { }
+    public PinValue Read() { }
+    public void Write(PinValue value) { }
+    public event PinChangeEventHandler ValueChanged;
+    public void Toggle() { }
+}

API Usage

// As always get a GpioController
const int PinNumber = 42;
GpioController ctrl = new GpioController();
// Then open a pin and get the GpioPin
GpioPin pin = ctrl.OpenPin(PinNumber, PinMode.Input);
// Then do directly the operations on the pin:
// Write operation, equivalentin to ctrl.Write(PinNumber, PinValue.High)
pin.Write(PinValue.High)
// Read operation, equivalent to ctrl.Read(PinNumber);
var val = pin.Read();
// Togle the pin value, no equivalent using the GpioController except reading and writing the opposite value
pin.Toggle();

Something we are trying to solve is when we do have extender and need a lot of pins. In the current situation, the only way to do this is to create your own controller taking the other controllers you need. So you basically have to do something like this:

    // Very simplified version, just to show the inconvenient of the solution
    public class MyGpioController : GpioController
    {
        private GpioController _controller1;
        private GpioController _controller2;
        private readonly ConcurrentDictionary<int, PinValue?> _openPins = new();

        public MyGpioController(GpioController driver1, GpioController driver2)
        {
            _controller1 = driver1;
            _controller2 = driver2;
        }

        // We say each driver has 4 pins
        public new int PinCount => 8;

        public new void OpenPin(int pinNumber)
        {
            if ((pinNumber >= 0) && (pinNumber < 4))
            {
                _controller1.OpenPin(pinNumber);
            }
            else if ((pinNumber >= 4) && (pinNumber < 8))
            {
                _controller2.OpenPin(pinNumber - 4);
            }
        }

        public new void Write(int pinNumber, PinValue pinValue)
        {
            if ((pinNumber >= 0) && (pinNumber < 4))
            {
                _controller1.Write(pinNumber, pinValue);
            }
            else if ((pinNumber >= 4) && (pinNumber < 8))
            {
                _controller2.Write(pinNumber - 4, pinValue);
            }
        }

        // All the rest of the controller will have to be implemented that way!
    }

// Then you will have to instantiate your own GpioController with the 2 drivers:
GpioController ctrl = new(
  new GpioController(new GpioDriver1()),
  new GpioController(new GpioDriver2()));

// You usually need a list of pins
List<int> _pins = new() { 0, 1, 2, 3, 4, 5, 6, 7 };

// And then pass it to your device
var myDevice = new MyDevice(_pins, _ctrl);

With GpioPin, everything you need is just a list of GpioPins, regardless of the GpioController they are coming from:

// One controller will come from one driver
GpioController ctrl1 = new(new GpioDrive1());
GpioController ctrl2 = new(new GpioDrive2());
List<GpioPin> _lotsOfPins = new();
for (int i = 0; i< 4; i++)
{
  _lotsOfPins.Add(ctrl1.Open(1));
  _lotsOfPins.Add(ctrl1.Open(2));
}

// The device will take GpioPin list:
var myDevice = new MyDevice(_lotsOfPins);

Alternative Designs

The alternative design is to not introduce the GpioPin and stay as it is.

Risks

Main risk is the binary breaking change introduce by the change of the OpenPin function, now returning a GpioPin rather than being void. This change is not a code breaking change, it's only in the case of updating the .NET IoT nuget in an application without recompiling it. We think, this scenario is very minimal, and most people are rebuilding the solution when updating nugets.

When recompiling the solution, no change is necessary, just to recompile the code as everything is source compatible.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[danmoseley]

@Ellerbach @krwq do we track API reviews on IoT API here? I wonder whether https://apireview.Net should look at that repo also. @terrajobst

[krwq]

I'm not sure if API review does it (guessing no), we only do occasional API reviews in IoT so I don't think it makes sense to set it up permanently.

This one was explicitly requested because we had some more discussion on this particular set of APIs and need advice from API review folks.

I'd prefer we make this as one time thing since for most of the APIs we'd do API review during our weekly triage meeting rather than official one.

[krwq]

Hmm I missed the fact this was filed on dotnet/runtime and not dotnet/iot. I think that's fine in that case, this is a one time request for API review, we normally don't want that. I'm not strongly opinionated where issue should live - if there is easier way we can request one time API review then we can go ahead with that

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

In .NET IoT (https://github.com/dotnet/iot/) we do have a GpioController allowing operations on GPIO (referred as pin later). To open, write, read or close a pin, it is necessary to use the GpioController.
We want to propose adding a GpioPin class that will make it easier to access directly a pin.

This GpioPin is already present and used in .NET nanoFramework (https://github.com/nanoframework/). We've been working to align as best as possible all hardware related API between .NET IoT and .NET nanoFramework. So GPIO, SPI, I2C, PWM, Serial API are aligned. Due to the platform differences, there are minor differences.

The GpioPin in .NET nanoFramework can be found here: https://github.com/nanoframework/System.Device.Gpio/blob/main/System.Device.Gpio/GpioPin.cs

Adding a GpioPin makes it easy for simple operations and a simpler concept as well for beginners. It is important to keep in mind that only the GpioController is allow to open pins and is responsible ultimately for the life of the GpioPin. You cannot open a GpioPin without the GpioController. You can dispose your GpioPin which will close it in the GpioController. And if the GpioController is disposed, the GpioPin won't be able to operate the pin. This deisgn exist in .NET nanoFramework and is very sucessfull. It was existing before the GpioController API aligned and based on developers' feedback has been kept.

Adding this GpioPin to the GpioController will result in a breaking change in the OpenPin function. This is not a code breaking change, it is a binary breaking change. See the risk section on this.

When using multiple GpioControllers or with specific controllers like the FT family, this can be an advantage to simplify the management of the pins. It could allow some better performance as well in those scenarios.

An active issue is open in .NET IoT which also reference other closed issues with more detailed discussions: #1671

API Proposal

See the PR in .NET IoT with the proposed implementation: #1895

API Usage

// As always get a GpioController
const int PinNumber = 42;
GpioController ctrl = new GpioController();
// Then open a pin and get the GpioPin
GpioPin pin = ctrl.OpenPin(PinNumber, PinMode.Input);
// Then do directly the operations on the pin:
// Write operation, equivalentin to ctrl.Write(PinNumber, PinValue.High)
pin.Write(PinValue.High)
// Read operation, equivalent to ctrl.Read(PinNumber);
var val = pin.Read();
// Togle the pin value, no equivalent using the GpioController except reading and writing the opposite value
pin.Toggle();

Alternative Designs

The alternative design is to not introduce the GpioPin and stay as it is.

Risks

Main risk is the binary breaking change introduce by the change of the OpenPin function, now returning a GpioPin rather than being void. This change is not a code breaking change, it's only in the case of updating the .NET IoT nuget in an application without recompiling it. We think, this scenario is very minimal, and most people are rebuilding the solution when updating nugets.

When recompiling the solution, no change is necessary, just to recompile the code as everything is source compatible.

Author:	Ellerbach
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged
Milestone:	-

[tannergooding]

@krwq, I think its probably better to live in dotnet/iot with requests like we do for winforms and other repos. It helps with tracking/labeling/etc.

It's worth noting to @Ellerbach that API review would appreciate if the following could be fixed up to actually contain the API surface:

API Proposal
See the PR in .NET IoT with the proposed implementation

Doing so will greatly help with API review and ensure that its short/clear/immediately available. We don't need (and prefer not to have) the implementation as part of the proposed API surface. A link after the proposal to the implementation is, however, welcome 😄

[joperezr]

Oh, I also missed the fact that this is in the wrong repo. Will transfer to dotnet/iot. I already sent an internal email to API Review board to get it on the books.

@danmoseley I don't think we need to have API Review monitor dotnet/iot as most of the API changes we make don't go through API Review. This particular one is a rather impacting one that could use expert's advice, which is why we are asking for consultation, but it is more of a one-off kind of thing.

[pgrawehr]

One observation that just jumped on me: If GpioPin is not sealed, it's methods should be abstract or virtual, otherwise it is not really extensible.

[pgrawehr]

After the above comments, I think the proposal should be cleaned up as follows:

namespace System.Device.Gpio;

public class GpioController : IDisposable
{
// Changing current OpenPin overloads to return a GpioPin instead of void. Binary breaking, non source breaking.
-   public void OpenPin(int pinNumber) { }
+   public GpioPin OpenPin(int pinNumber) { }

-   public void OpenPin(int pinNumber, PinMode mode) { }
+   public GpioPin OpenPin(int pinNumber, PinMode mode) { }

-   public void OpenPin(int pinNumber, PinMode mode, PinValue initialValue) { }
+   public GpioPin OpenPin(int pinNumber, PinMode mode, PinValue initialValue) { }
}

+public class GpioPin : IDisposable
+{
+    public GpioPin(int pinNumber, GpioController controller) {};
+    public virtual int PinNumber { get; }
+    public virtual PinMode GetPinMode() { }
+    public virtual bool IsPinModeSupported(PinMode pinMode) { }
+    public virtual void SetPinMode(PinMode value) { }
+    public virtual PinValue Read() { }
+    public virtual void Write(PinValue value) { }
+    public event PinChangeEventHandler ValueChanged;
+    protected virtual void Dispose(bool disposing) {};
+    public void Dispose() {};
+}

Open Questions:

Should GpioPin be Disposable?

Pro:

• Can just use an using statement around an OpenPin call
• Owners of the GpioPin class (e.g. bindings) can clean up after themselves

Cons:

• The pin is still owned by the GpioController, making GpioPin disposable appears to move ownership

Should the ctor take GpioController or GpioDriver?

The later may allow for some performance optimization, while it allows bypassing the checks of the controller. Bypassing the GpioController could allow for dangling GpioPin instances (instances pointing to closed pins or pins whose designation has changed).

Should the class be sealed instead?

This would slightly increase performance, but obviously not allow hardware-dependent implementations.

Does the ctor need to be public?

I believe so, since GpioController (who will normally create instances of this type) is public and inheritable.

Do we need the Toggle() method?

Depending on the hardware, implementing it in a thread-safe fashion might be difficult or even impossible. So rather not present a method that may or may not work.

What's the risk in the binary breaking change from void OpenPin() to GpioPin OpenPin()?

What can possibly go wrong?

[raffaeler]

I agree with @pgrawehr. Let me add a couple of notes:

1. I prefer the binary breaking change on OpenPin. Returning GpioPin gives more sense to the proposal, increase the usability and IMO resolve the doubts around the ownership of the GpioPin.
2. Toggle. Reading the state and toggling manually could make the transition longer than expected. This method could use the GC.TryStartNoGCRegion to minimize the delays.
3. I would love to see the Pulse method as well: void Pulse(int microseconds) which also could use GC.TryStartNoGCRegion.

Not sure if I will be able to join the call, sorry.

[krwq]

For 1, 100% agree - for 2/3 I'd only consider that if there is a hardware support - for Toggle I believe we can support it at least on some drivers I'm not sure about pulses

[raffaeler]

@krwq I understand your point but given the library is an abstraction over many vendors, it is not likely to see the same support for all of them.
I believe we can offer an "optimistic" implementation where the hardware is used where available or a software implementation where it is not.
It is roughly simiilar to the current support for pull-ups / pull-downs. Not all the Gpio support them, but the library provides the option in any case (even if there is no software equivalent).

[joperezr]

Here are my few notes on the open questions from above:

Should GpioPin be Disposable?

I could be convinced otherwise, but I wouldn't mark it as disposable. Disposable types either hold native resources, or have members that do which need to be disposed of. GpioPin's case doesn't apply to any of those, as the native resources are actually hold by the driver, and GpioPin doesn't have any other members that need to be disposed of. As for the comment regarding cleaning up after use, device bindings should just call Close() instead.

Should the ctor take GpioController or GpioDriver?
Does the ctor need to be public?

My answer here would be: no, the constructor for Gpio Pin should only be internal, and the only way to get a GpioPin back should be via calling OpenPin in a controller. If you want a GpioPin dervied type back, then the way to do it is via creating a controller which has a specific custom driver passed in, which returns this custom gpiopin back. For that, this proposal is also missing changing the driver's open pin to also return GpioPin so that derived drivers can return derived GpioPins.

Should the class be sealed instead?

Unless we have a very specific use in mind, I'd start with making it sealed, and then in the future we can unseal it if we need to. We can't go the other way around.

Do we need the Toggle() method?

I would prefer not to. We have a Write method, and Toggle is a method that can be very easily added on the consuming code as a helper. In any case, I think this decision doesn't need to be made now, so I'd rather not to include it as part of this proposal, and then add it later as a subsequent proposal if we see it provides enough value.

What's the risk in the binary breaking change from void OpenPin() to GpioPin OpenPin()?

Risk is pretty high. The main issue will be that this is a binary breaking change and it will very likely break anyone in the library case. By this I mean: Imagine a own a robotic company which has a robot, and I wanted to ship a nuget package with a binding for my robot that internally uses System.Device.Gpio. I will depend on one version of that, for example, in version 2.2. If a consuming app then wants to use my robot nuget package, they won't be able to use our 3.0 System.Device.Gpio package as well, since if they do, it would break the robot package. In other words, any code that depends on System.Device.Gpio today would be binary roken after this change, and would be forced to recompile.

[Ellerbach]

Unless we have a very specific use in mind, I'd start with making it sealed, and then in the future we can unseal it if we need to. We can't go the other way around.

I'm always more in favor of not sealing. But we can for sure.

Toggle. Reading the state and toggling manually could make the transition longer than expected. This method could use the GC.TryStartNoGCRegion to minimize the delays

Toggle is actually available natively on many hardware. Most extenders like the FT ones have this capability. Also in MCU it's always the case making the change of a pin really fast. Now, that's as well a reason we should not seal the implementation to allow faster usage.

I would love to see the Pulse method as well: void Pulse(int microseconds) which also could use GC.TryStartNoGCRegion

Like @krwq, I'm not super in favor of the pulse one. It will always be very dependent of the driver to get a maximum performance. Something that can be added later. Also the pulse one is most of the time a PWM, so I would recommend then to go for a PWM pin instead at least it makes things clear.

For the rest aligned with the comments from @joperezr.

[josesimoes]

My take on this:

1. Toggle: I keep thinking this is very useful and it should be added. For platforms that don't support it natively the best course of action would be to perform a read and toggle in the low level layer.
   Those few lines of code wont' have any noticeable impact on overall performance. And for sure that will much faster than performing the usual approach of read in C#, toggle it and write to the GPIO.
   I don't understand why this should be a problem regarding thread safety execution.
   One last motivation: with this, we'll be closer to have API compatibility between IoT and .NET nanoFramework. 😉

2. OpenPin(): sometimes breaking changes are required and have to be assumed. In this case I think having OpenPin() returning a GpioPin would be preferable. It not only makes the API more "object oriented" like (as it should be, being C#) but it also makes it clear who owns the GpioPin and it makes it soooo much easier in code to be able to take that GpioPin object after it's being created and do stuff with it. As opposed to have to, at a later stage, have to go and having to grab it from some other place. Plus (you'll forgive me on keeping insisting on this 😅), it would be in line with .NET nanoFramework: OpenPin() and following.

3. Pulse(): I don't think this belongs here... This is more in the realm of PWM or timers. If the GPIO API is bringing time into play, that would be much useful implementing a property to handle debounce at driver level. Like there is in .NET nanoFramework: DebounceTimeout.

[raffaeler]

Risk is pretty high.

@joperezr got it. If the breaking change is unavoidable, I would rather prefer to introduce a new, let's say, GpioController2 satisfying the new contract. BTW I don't like the 2 appended to the end, it is just an example.

@Ellerbach : About Pulse, I didn't even think to pwm. There are many times where ICs require a pulse to enable features, reset, etc.. The timings for the pulse are critical and this is why we can provide an appropriate implementation.

With regards to Toggle and Pulse, I am trying to see it from the opposite perspective. We have the chance to write the best possible implementation (eventually with hw support or suspended GC timings). On the other end the user risks to write down a lot of useless and error-prone code. Let's think about it.

[bartonjs]

• The binary breaking change seems to be mitigated by a fairly tight coupling to higher level types that ship out of the same repository, and low usage of these primitive types directly.
• Based on discussion it seems like the controller needs a Toggle method as well (or TogglePin, whatever you think is best)
• During discussion it was stated that GpioPin was going to reach through straight to the driver, potentially bypassing any overridden methods in the controller class. This may be too much of a surprise.
  • It was mentioned that the straight-to-the-driver behavior could be conditional on whether this.GetType() == typeof(GpioController).

namespace System.Device.Gpio;

public class GpioController : IDisposable
{
// Changing current OpenPin overloads to return a GpioPin instead of void. Binary breaking, non source breaking.
-   public void OpenPin(int pinNumber) { }
+   public GpioPin OpenPin(int pinNumber) { }

-   public void OpenPin(int pinNumber, PinMode mode) { }
+   public GpioPin OpenPin(int pinNumber, PinMode mode) { }

-   public void OpenPin(int pinNumber, PinMode mode, PinValue initialValue) { }
+   public GpioPin OpenPin(int pinNumber, PinMode mode, PinValue initialValue) { }

+   public void Toggle(int pinNumber) { }
}

+public sealed class GpioPin
+{
+    public int PinNumber { get; }
+    public PinMode GetPinMode() { }
+    public bool IsPinModeSupported(PinMode pinMode) { }
+    public void SetPinMode(PinMode value) { }
+    public PinValue Read() { }
+    public void Write(PinValue value) { }
+    public event PinChangeEventHandler ValueChanged;
+    public void Toggle() { }
+}

[Ellerbach]

Closing this issue as the implementation has been done and merged.