Add the Pimoroni Blinkt device binding #2370
Conversation
dotnet-policy-service
bot
added
the
area-device-bindings
jimbobbennett
changed the title
src/devices/Blinkt/Blinkt.cs
Outdated
| _pixels[i] = Color.Empty; | ||
| } | ||
|
|
||
| _gpio = new GpioController(); |
There was a problem hiding this comment.
Check some other bindings: It should be possible to provide the GpioController as argument to the constructor.
src/devices/Blinkt/Blinkt.cs
Outdated
| public const int NUMBER_OF_PIXELS = 8; | ||
|
|
||
| private readonly Color[] _pixels = new Color[NUMBER_OF_PIXELS]; | ||
| private readonly int _sleepTime = 0; |
There was a problem hiding this comment.
Are you sure this can always stay 0? Depending on the CPU speed, additional sleep cycles might be needed.
There was a problem hiding this comment.
This is lifted from the Blinkt Python code, and is just there to allow any other threads some time to run if needed. I've tested on a RPi 4 and 5, and they both work without the sleeps that use this, but I was hesitant to remove it incase it is needed on older boards.
There was a problem hiding this comment.
I would rather expect it to require a delay for faster CPUs, however that is not my point: You've declared it as private field without any possibility to change it, so like this, it is useless. You should add a property so that the value can be changed.
dotnet-policy-service
bot
added
the
Needs: Author Feedback
|
/azp run dotnet.iot |
|
Azure Pipelines successfully started running 1 pipeline(s). |
dotnet-policy-service
bot
removed
the
Needs: Author Feedback
src/devices/Blinkt/Blinkt.cs
Outdated
| /// <summary> | ||
| /// The number of pixels in the Blinkt strip | ||
| /// </summary> | ||
| public const int NUMBER_OF_PIXELS = 8; |
There was a problem hiding this comment.
For constants, we're using like for variables Pascal Case. So, this one should be NumberOfPixels. We do exceptions for internal registers for examples to be closer to the binding documentation.
src/devices/Device-Index.md
Outdated
| @@ -20,6 +20,7 @@ | |||
| * [AXP192 - Enhanced single Cell Li-Battery and Power System Management IC](Axp192/README.md) | |||
| * [Bh1745 - RGB Sensor](Bh1745/README.md) | |||
| * [BH1750FVI - Ambient Light Sensor](Bh1750fvi/README.md) | |||
| * [Blinkt - 8-LED indicator strip](Blinkt/README.md) | |||
There was a problem hiding this comment.
that's not necessary and it's done automatically at merge time.
src/devices/README.md
Outdated
| @@ -203,6 +203,7 @@ Our vision: the majority of .NET bindings are written completely in .NET languag | |||
|
|
|||
| * [Adafruit Seesaw - extension board (ADC, PWM, GPIO expander)](Seesaw/README.md) | |||
| * [APA102 - Double line transmission integrated control LED](Apa102/README.md) | |||
| * [Blinkt - 8-LED indicator strip](Blinkt/README.md) | |||
There was a problem hiding this comment.
that's not necessary and it's done automatically at merge time.
Co-authored-by: Laurent Ellerbach <[email protected]>
|
/azp run dotnet.iot |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run dotnet.iot |
|
Commenter does not have sufficient privileges for PR 2370 in repo dotnet/iot |
Thanks. Removed the only issue I could see from the linter, but don't have rights to run this myself to test. |
|
I see the Arduino build has failed some unit tests. I can't see where these have happened as the output doesn't show any failures. Is this a flaky test that can be re-run, or is there more information I can use to resolve this? |
|
/azp run dotnet.iot |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@pgrawehr Could you please take a look at the failures? They are unrelated to this PR |
Fixes #2369
This adds a device binding for the Pimoroni Blinkt.
Microsoft Reviewers: Open in CodeFlow