Create replacement for now lacking physical-to-logical pin mapping #2435
Conversation
dotnet-policy-service
bot
added
the
area-device-bindings
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new VirtualGpioControllerWithDefault class to support physical-to-logical pin mapping for GPIO controllers, specifically addressing the need for Raspberry Pi physical pin numbering. The implementation allows dynamic pin mapping through a default controller and a mapping function, replacing previous approaches that lacked this capability.
- Adds
VirtualGpioControllerWithDefaultclass extendingVirtualGpioControllerwith default controller support - Implements physical-to-logical pin mapping for Raspberry Pi boards via
ConvertPinNumberFromPhysicalToLogical - Updates
VirtualGpioController.OpenPinCoredocumentation to guide users toward the new alternative
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/devices/Board/VirtualGpioControllerWithDefault.cs | New class providing virtual GPIO controller with default controller and pin mapping function |
| src/devices/Board/VirtualGpioController.cs | Updated documentation for OpenPinCore to reference new alternative implementation |
| src/devices/Board/RaspberryPiBoard.cs | Added methods to create physical pin mapping and conversion logic from physical to logical numbering |
| throw new InvalidOperationException($"Virtual Pin Number {pinNumber} is in use already"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The method MapToRealPin is public but lacks XML documentation comments. Public APIs should have comprehensive documentation explaining parameters, return values, and behavior, especially for methods that perform critical mapping operations.
| /// <summary> | |
| /// Maps a virtual pin number to the corresponding real pin number using the provided mapping function. | |
| /// </summary> | |
| /// <param name="virtualPin">The virtual pin number to map.</param> | |
| /// <returns> | |
| /// The real pin number corresponding to the virtual pin, or -1 if the mapping function does not recognize the pin. | |
| /// </returns> |
Copilot uses AI. Check for mistakes.
|
|
||
| /// <summary> | ||
| /// Creates a derived GPIO controller that uses physical pin mapping for the Raspberry Pi. | ||
| /// </summary> |
There was a problem hiding this comment.
The method CreatePhysicalPinMapping() lacks a <returns> XML documentation tag describing what type of controller is returned and its behavior. This is important for API consumers to understand the returned controller's capabilities.
| /// </summary> | |
| /// </summary> | |
| /// <returns> | |
| /// A <see cref="VirtualGpioController"/> that uses physical board numbering for GPIO pins on the Raspberry Pi. | |
| /// The returned controller maps physical pin numbers to the logical numbering scheme used by the underlying driver. | |
| /// </returns> |
Copilot uses AI. Check for mistakes.
| /// <param name="defaultController">The controller to use. Must be for a Raspberry Pi.</param> | ||
| /// <returns>A controller that uses physical board numbering</returns> | ||
| /// <remarks>If the provided <paramref name="defaultController"/> is not for a Raspberry Pi, the behavior might be undefined.</remarks> | ||
| public static VirtualGpioController CreatePhysicalPinMapping(GpioController defaultController) |
There was a problem hiding this comment.
The static overload of CreatePhysicalPinMapping is missing a <returns> XML documentation tag. Public API methods should document their return values to clarify what the returned controller provides.
Copilot uses AI. Check for mistakes.
1 participant
Microsoft Reviewers: Open in CodeFlow