Skip to content

Marking Libgpiod v2 api as experimental #2323

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

Merged
merged 1 commit into from
Jun 12, 2024
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
9 changes: 9 additions & 0 deletions Documentation/DiagnosticIDs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Diagnostic IDs

Some of the APIs in this repository are still in development and may be changed in the future, so they were marked with the Experimental attribute. If you want to consume these APIs and understand the risks, you can disable the diagnostic IDs directly in your project either by adding a diagnostic id to the `NoWarn` property in your project file or by using the `#pragma warning disable` directive in your code.

## Diagnostic list

| Diagnostic ID | Description |
| :---------------- | :---------- |
| `SDGPIO0001` | Experimental APIs related to LibGpiod v2 driver used by newer versions of the library |
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -13,6 +14,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// allows callers to retrieve information about each line, watch lines for state changes and make line requests.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__chips.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to add this attribute to internal classes? They're not part of the API.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, short answer is that this way we know if any stable API depends on any unstable API.

Long answer: when marking some of the new public APIs as experimental, the internal code we added in libgpiod started to throw warnings as we were calling experimental APIs. There are two ways of getting rid of those warnings, you either suppress them (either in the callsite or just add it to NoWarn so they are suppressed for the whole project) or you mark the caller also as Experimental, which would just bubble the warning one level up the callstack. I chose the latter as otherwise we wouldn't know if any stable public facing API depended on any nom stable public facing API, so I marked each of the classes that showed warnings Experimental all the way up till O reached public API, and only suppressed the warnings there. This way, it requires a conscious decision to depend on experimental stuff from non experimental stuff.

Copy link
Member Author

@joperezr joperezr Jun 12, 2024

Choose a reason for hiding this comment

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

For instance, I know that today, the only two existing public APIs that are stable which may depend on any of the new experimental codepaths are UnixDriver.Create and LibGpiod constructor, because those are the only two places where I am suppressing the warnings.

Without making the internal types experimental in order to bubble up errors on the static analysis, figuring this out would be a challenge, and may lead us to depend on experimental stuff in places where we shouldn't.

internal class Chip : LibGpiodProxyBase
{
private readonly ChipSafeHandle _handle;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -11,6 +12,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// The chip info contains all the publicly available information about a chip.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__chip__info.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class ChipInfo : LibGpiodProxyBase
{
private readonly ChipInfoSafeHandle _handle;
Expand Down Expand Up @@ -62,6 +64,7 @@ public int GetNumLines()
/// Helper function for capturing information and creating an immutable snapshot instance.
/// </summary>
/// <exception cref="GpiodException">Unexpected error when invoking native function</exception>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
public Snapshot MakeSnapshot()
{
return new Snapshot(GetName(), GetLabel(), GetNumLines());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -13,6 +14,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// of events are being read.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__edge__event.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class EdgeEvent : LibGpiodProxyBase
{
private readonly EdgeEventSafeHandle _handle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -13,6 +14,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// of events are being read.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__edge__event.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class EdgeEventBuffer : LibGpiodProxyBase
{
internal EdgeEventBufferSafeHandle Handle { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;

namespace System.Device.Gpio.Libgpiod.V2;

[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal abstract class LibGpiodProxyBase : IDisposable
{
/// <remarks>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;

[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal static class LibGpiodProxyFactory
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Linq;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -20,6 +21,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// If any of the offsets was duplicated, the last one will take precedence.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__line__config.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class LineConfig : LibGpiodProxyBase
{
internal LineConfigSafeHandle Handle { get; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -13,6 +14,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// The line must be requested to access the line value.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__line__info.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class LineInfo : LibGpiodProxyBase
{
private readonly LineInfoSafeHandle _handle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -12,6 +13,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// as well as a snapshot of line's status in the form of a line-info object.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__line__watch.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class LineInfoEvent : LibGpiodProxyBase
{
private readonly LineInfoEventSafeHandle _handle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -16,6 +17,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// This object provides exclusive usage, i.e. reading or setting lines state.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__line__request.html#details"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class LineRequest : LibGpiodProxyBase
{
private readonly LineRequestSafeHandle _handle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -12,6 +13,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// as well as a snapshot of line's status in the form of a line-info object.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__line__watch.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class LineSettings : LibGpiodProxyBase
{
internal LineSettingsSafeHandle Handle { get; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -11,6 +12,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// Contains functions that are not part of any specific concept.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__misc.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class Miscellaneous
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Device.Gpio.Drivers;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using LibgpiodV2 = Interop.LibgpiodV2;

namespace System.Device.Gpio.Libgpiod.V2;
Expand All @@ -12,6 +13,7 @@ namespace System.Device.Gpio.Libgpiod.V2;
/// The mutators don't return error values. If the values are invalid, in general they are silently adjusted to acceptable ranges.
/// </summary>
/// <seealso href="https://libgpiod.readthedocs.io/en/latest/group__request__config.html"/>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal class RequestConfig : LibGpiodProxyBase
{
internal RequestConfigSafeHandle Handle { get; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.IO;

namespace System.Device.Gpio.Drivers;

/// <summary>
/// Exception in the context of calling libgpiod
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
public class GpiodException : IOException
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.Threading;

namespace System.Device.Gpio.Drivers;
Expand All @@ -23,9 +24,11 @@ public class LibGpiodDriver : UnixDriver
/// </remarks>
public LibGpiodDriver(int gpioChip = 0)
{
#pragma warning disable SDGPIO0001 // Suppressing diagnostic for using experimental APIs from this same repository.
LibGpiodDriverFactory.VersionedLibgpiodDriver versionedLibgpiodDriver = LibGpiodDriverFactory.Instance.Create(gpioChip);
_driver = versionedLibgpiodDriver.LibGpiodDriver;
Version = versionedLibgpiodDriver.DriverVersion;
#pragma warning restore SDGPIO0001 // Suppressing diagnostic for using experimental APIs from this same repository.
}

/// <summary>
Expand All @@ -36,6 +39,7 @@ public LibGpiodDriver(int gpioChip = 0)
/// <param name="gpioChip">The number of the GPIO chip to drive</param>
/// <param name="driverVersion">Version of the libgpiod driver to create</param>
/// <remarks>Alternatively, specify the environment variable DOTNET_IOT_LIBGPIOD_DRIVER_VERSION, see documentation</remarks>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
public LibGpiodDriver(int gpioChip, LibGpiodDriverVersion driverVersion)
{
LibGpiodDriverFactory.VersionedLibgpiodDriver versionedLibgpiodDriver = LibGpiodDriverFactory.Instance.Create(gpioChip, driverVersion);
Expand All @@ -46,12 +50,14 @@ public LibGpiodDriver(int gpioChip, LibGpiodDriverVersion driverVersion)
/// <summary>
/// Version of the libgpiod driver
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
public LibGpiodDriverVersion Version { get; protected set; }

/// <summary>
/// A collection of driver versions that correspond to the installed versions of libgpiod on this system. Each driver is dependent
/// on specific libgpiod version/s. If the collection is empty, it indicates that libgpiod might not be installed or could not be detected.
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
public static LibGpiodDriverVersion[] GetAvailableVersions() => LibGpiodDriverFactory.Instance.DriverCandidates;

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Device.Gpio.Drivers.Libgpiod.V1;
using System.Device.Gpio.Drivers.Libgpiod.V2;
using System.Device.Gpio.Libgpiod.V2;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;

Expand All @@ -13,6 +14,7 @@ namespace System.Device.Gpio.Drivers;
/// <summary>
/// Driver factory for different versions of libgpiod.
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal sealed class LibGpiodDriverFactory
{
private const string DriverVersionEnvVar = "DOTNET_IOT_LIBGPIOD_DRIVER_VERSION";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace System.Device.Gpio.Drivers;

/// <summary>
/// Each driver version supports specific libgpiod version/s.
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
public enum LibGpiodDriverVersion
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Device.Gpio.Libgpiod;
using System.Device.Gpio.Libgpiod.V2;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;

Expand All @@ -13,6 +14,7 @@ namespace System.Device.Gpio.Drivers.Libgpiod.V2;
/// <summary>
/// Driver that uses libgpiod V2 for GPIO control.
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal sealed class LibGpiodV2Driver : UnixDriver
{
private static readonly string ConsumerId = $"C#-{nameof(LibGpiodV2Driver)}-{Process.GetCurrentProcess().Id}";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Device.Gpio.Libgpiod.V2;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -12,6 +13,7 @@ namespace System.Device.Gpio.Drivers.Libgpiod.V2;
/// <summary>
/// Class that observes libgpiod line requests for events.
/// </summary>
[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal sealed class LibGpiodV2EventObserver : IDisposable
{
private readonly Dictionary<EventSubscription, List<PinChangeEventHandler>> _handlersBySubscription = new();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Device.Gpio.Libgpiod;
using System.Device.Gpio.Libgpiod.V2;
using System.Diagnostics.CodeAnalysis;

namespace System.Device.Gpio.Drivers.Libgpiod.V2;

[Experimental(DiagnosticIds.SDGPIO0001, UrlFormat = DiagnosticIds.UrlFormat)]
internal static class Translator
{
public static (GpiodLineDirection? _direction, GpiodLineBias? _bias) Translate(PinMode pinMode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ public static UnixDriver Create()
UnixDriver? driver = null;
try
{
driver = (UnixDriver)LibGpiodDriverFactory.Instance.Create(0).LibGpiodDriver;
#pragma warning disable SDGPIO0001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
driver =
(UnixDriver)LibGpiodDriverFactory.Instance.Create(0).LibGpiodDriver;
#pragma warning restore SDGPIO0001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
}
catch (PlatformNotSupportedException)
{
Expand Down
Loading