-
Notifications
You must be signed in to change notification settings - Fork 580
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
Changed rock pi 4 board pin numbering to match documentation. #2082
Conversation
@dotnet-policy-service agree company="Edwards Vacuum" |
int num = _pinNumberConverter[pinNumber]; | ||
|
||
return num != -1 ? num : throw new ArgumentException($"Board (header) pin {pinNumber} is not a GPIO pin on the {GetType().Name} device.", nameof(pinNumber)); | ||
return pinNumber switch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this improvement bu you still miss -1 which is a valid option. It means "no pin". So you should definitely add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? That seems inconsistent with the rest of the code in this library. I can't find anywhere else where we treat -1 as a valid pin in a board numbering. Look at the same function in RaspberryPi3LinuxDriver for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, I got catch by the previous implementation.
38 => MapPinNumber(4, 'A', 6), | ||
40 => MapPinNumber(4, 'A', 7), | ||
_ => throw new ArgumentException($"Board (header) pin {pinNumber} is not a GPIO pin on the {GetType().Name} device.", nameof(pinNumber)) | ||
}; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I double checked the 40 pin numbering assignment with the specs and they match.
/azp run dotnet.iot |
Azure Pipelines successfully started running 1 pipeline(s). |
Thanks @Sleepwalk42! |
This corrects the board pin numbers used by the RockPi4bPlusDriver to match the numbering in the documentation (see this product brief for the official documentation.
The original implementation used pin numbers that were 1 less than what the board document specified. If you wanted GPIO code that would work on both the raspberry pi and the rock 4 (their form factor and GPIO header are almost identical), you'd need to apply a -1 to the pin numbers if you were running on a rock 4 board.
This will break existing code using this driver with board pin numbering.
Microsoft Reviewers: Open in CodeFlow