Closed breml closed 1 year ago
@conejoninja any comments on this before we squash/merge?
Not yet, I had it on my TODO list, but my tinygo installation is broken and doesn't compile (there's a thread on #tinygo slack channel). Didn't have time to fix it yet, and therefor I couldn't test this PR on the real hardware. But changes looks good to me.
Let me point out some of the "more" important changes in order to make the review easier for you:
Display
method (https://github.com/tinygo-org/drivers/pull/472/files#diff-f2acbc722949a69b6f3941dde9d3653785c3ae27af65c5341e6ef9a4e84028f7R144-R179), where the LEDs are switched off in the order of the brightness levels (the ones with the lowest brightness first, the ones with the highest last). The code is heavily inspired by the C implementation used for MicroPython. Also the timing values (https://github.com/tinygo-org/drivers/pull/472/files#diff-f2acbc722949a69b6f3941dde9d3653785c3ae27af65c5341e6ef9a4e84028f7R54-R70) are taken from the C implementation.color.RGBA
for the brightness where a value of 0
is no transparency (and those full brightness) and 255
is full transparency (and those the LED is off). It supports the same 9 brightness levels that are supported by the MicroPython implementation (https://github.com/tinygo-org/drivers/pull/472/files#diff-f2acbc722949a69b6f3941dde9d3653785c3ae27af65c5341e6ef9a4e84028f7R106-R129). Because the new version of the code now respects the alpha channel, this might break existing code, so in that sense, this change is not backwards compatible.matrixRotations
, such that both rotate in clock wise orientation and so that both versions of microbit can use the same version of the Display
method (https://github.com/tinygo-org/drivers/pull/472/files#diff-d01a794ff34319d5ce51ce1f370a868db8d2e52350634183282cbeb5df012a78R17-R46)microbitmatrix.go
such that only hardware specific configuration (e.g. setup of PINs, number of hardware rows and cols) is left in the version dependent microbitmatrix-v1.go
and microbitmatrix-v2.go
cols
and rows
variables instead of x
and y
, because especially in the cases where the order of x
and y
was inverted, this was harder to read and to understand. Especially in the rotation matrix, the order of x
and y
is inverted for good reasons, because the nested slices are easier to read this way.GetPixel
method does not support the different brightness level, because I did not want to change the signature of this method. That being said, if you are comfortable with changing the method signature, it would be a simple change to return the brightness (e.g. as int8
) instead of a bool
.One thing to mention: I did some quick tests in the TinyGo Playground with code that uses different brightness level and this did not work well there. I assume, that the physical effect of the LEDs to keep glow for a little while when there is no longer current on the respective pin is maybe not supported. Therefore it is worth mentioning, that the updated version of this driver might not work well with the Playground. That being said, the main target for this driver would be the use with the real physical device anyway.
I hope, this is helpful.
Thank you very much for working on this @breml now merging.
This PR adds brightness levels for the LED pixels similar to the implementation present in the micro:bit MicroPython implementation.
Additionally, the code for V1 and V2 is harmonized and only the code that is really hardware (machine) dependent is in the respective v1/v2 file. All the generic code for the controll of the microbitmatrix is in the shared source file.
I tested this code on micro:bit V1 and V2 hardware.
CC: @conejoninja