Rewrite APA102 support

[aldehir]

Description

The APA102 source was broken by commit 16a15c1 as it did not include the quantum header. This PR addresses that, as well as other issues with transferring bytes over the SPI interface, i.e. it was not setting the clock pin back to low after sending a bit.

The deviation when sending the end frame is kept, but updated to the latest from the referenced project.

Additionally, these changes expose the global LED brightness parameter of the APA102. Brightness values are configurable through APA102_DEFAULT_BRIGHTNESS and APA102_MAX_BRIGHTNESS.

Question: Since it is using the QMK macros, does this still belong in under drivers/avr?

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

• [Feature Request] Use the Global Brightness Parameter of the APA102 LEDs #10026

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[Duckle29]

I'll give this a look and test once my LEDs arrive. It seems I originally wrote this code, but it has never worked. Thanks for the PR 👍

[aldehir]

Commit a906bed114c4919ec79e8ebecfe9cbc1412a9f75 adds a delay for ARM devices. From testing, it seems the APA102 LEDs cannot handle a pulse width less than 40 ns. I added a delay of 200 ns between clock high to low, just to be safe.

I did move the driver up a directory, since the avr directory no longer seemed appropriate. That being said, being in the drivers/ directory does not feel right either. I would appreciate some input on where this driver can live.

[Duckle29]

@aldehir I'm interested in how you're testing this on QMK? Also while the nop based code is pretty neat, it's not that readable, instead I think wait_us(x) is preferred. Testing on AVR wait_us(0.1) gives me a bit over 200 ns wide clock pulses. Interestingly, testing this on an STM32F303 (proton-c), that doesn't seem to make much of a difference, the clock pulse is around 20 µs regardless.

The datasheet also seems to suggest that anything over 15ns is fine, what kind of behavior did you observer under 40ns?

Which ARM board did you test on, and which pins? 20 us is slow enough that I can see it "animate" on a ring of 40 LEDs so I'd like to get it lower than that :(
https://gfycat.com/linearselfreliantgermanpinscher (Proton-c with no delay, 8x slower)

I also opened a PR against your fork to get this driver situated in a generic location, and tied in to the make build system :) aldehir#1

I'm curious if you can work your changes into that and report back how it performs? :)

Thanks :)

[Duckle29]

I also did some maths, and for a board with 100 LEDs, you have about 40 µs per bit if you want to hit 60fps refresh rate, however that doesn't account for dwell time, so I'd say a goal of 10 µs for the send-bit function would be neat :)

[aldehir]

@Duckle29 I hooked up an oscilloscope to the clock output (pin B5), and observed the following:

At < 40 ns the LEDs do not function as expected. Video

At > 40 ns the LEDs light up just fine. Video

I also noticed that a wait_us() on my STM32F303 discovery board does not go lower than 20 us, mimicking your findings. I found this article that provides some insight. Using with a custom chconf.h, I was able to lower that to 10 us by increasing CH_CFG_ST_FREQUENCY from 100 KHz to 1 MHz. Unfortunately, even that seems too slow for animation effects. I could not find a way to reduce that without resorting to busy looping with NOPs. I'm open to suggestions!

[Duckle29]

Yeah after playing more with it today I agree with your findings. I've made a few changes and described the reasons in the PR against your fork (linked above) :)

[Duckle29]

Apart from the requested changes. I also saw that the headers on the apa102.[c|h] files were wrong, so those should be updated.

Something simple like

/* Copyright 2020 you
 * Copyright 2017 Mikkel (Duckle29)
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

on both files should do

I'd also suggest adding the changes to keyboards/handwired/onekey I made in my PR. They make testing easier and gives a target that you can test compilation against with a simple make handwired/onekey:apa102

I'd add the clock pin definition on pro micro, elite-c and proton-c, and add an apa102 keymap.

add to proton_c config.h (B13 was chosen as it's the next available pin close to DI)

#define RGB_CI_PIN B13

add to pro micro and elite_c config.h (B1 was chosen as it's the same position as the proton_c pin chosen)

#define RGB_CI_PIN B1

And for the keymap, copy the rgb folder and rename it apa102, change the rules.mk file to RGBLIGHT_ENABLE = APA102
in keymap.c add

#include QMK_KEYBOARD_H
#include "apa102.h" // Only needed if you want to use the global brigthness function
// ...
void keyboard_post_init_user(void) {
    apa102_set_brightness(5);
// ...

As for the loop unrolling. I'm okay with leaving that as is, it does cost 74 bytes, which for some on AVR might be a problem, but as no-one is currently relying on this driver for AVR boards, it's fine.

[Duckle29]

I like it, and it compiles and runs, and doesn't seem to clash with ws2812! :)

[bhartshorn]

I've been fighting with the old driver all afternoon before noticing this PR. I'll give your work a test drive on Pro Micro. 👍

EDIT: Works great for solid colors, haven't tested any other modes (don't really need/want, not sure I have space). Good work, should merge if there's no outstanding complaints.

@aldehir @tzarc

[zvecr]

wrong radio button woops

[zvecr]

Not a fan of the apa102 parent folder, but that can be fixed once its in develop.