RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.94k stars 1.99k forks source link

Refactor microbit LED matrix into generic LED matrix #16256

Open chrysn opened 3 years ago

chrysn commented 3 years ago

Description

The current microbit module takes a somewhat odd role in being a driver that is specific to a group of boards, and in that it has a top block of defines that pick based on the board how to be configured. (This is where this all came up, in https://github.com/RIOT-OS/RIOT/pull/16250#discussion_r603981661).

There isn't anything microbit specific in the module implementation itself, though -- it's "just" a timer-driven setting of row and column GPIOs fast enough to show a good matrix display image. The parameters like row and column count that are alike for all microbits are already generalized, and the microbit versions are distinct enough to show that the module can both be used with logically aligned rows / columns (as is in the v2) and with arbitrary wiring (as in the v1, which drives 3x9 LEDs in a matrix but places them in a 5x5 grid spatially).

Useful links

In the current docs:

Open questions

@aabadie, if there was good reason to not do this, please let me know here so I won't waste time trying.

aabadie commented 3 years ago

if there was good reason to not do this, please let me know here so I won't waste time trying.

The main reason was to keep the change (grouping common code across boards) small initially. But I agree this module could go under drivers with sensible parameters that could overloaded at board level.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

chrysn commented 2 years ago

I still hope to resume this at one time, especially with the special case that one axis may have PWM pins assigned.