[Bug] PrimeKB/Meridian LED indicator lights won't turn off.

[newtonapple]

Meridian's LED indicator lights won't turn off. Further more, it seems like rgblight_setrgb_at can no longer control the LEDs, which was previously working in the initial commit.

Describe the Bug

It seems like rgblight_setrgb_at no longer works properly for the Meridian firmwares (both VIA & default) on master. I'm not sure what could have caused it or if other STM32 related PCBs have the same issue. After git-bitsecting, I have narrowed it down to this commit to be the source of the problem.

System Information

• Keyboard:
  • Revision (if applicable): PrimeKB/Meridian (revision master)
• Operating system: macOS Mojave (v10.14.6)
• AVR GCC version: avr-gcc (Homebrew AVR GCC 8.4.0) 8.4.0
• ARM GCC version: arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 8-2019-q3-update) 8.3.1 20190703 (release) [gcc-8-branch revision 273027]
• QMK Firmware version: 0.11.56
• Any keyboard related software installed?
  • AutoHotKey
  • [x ] Karabiner
  • Other:

Additional Context

• git-bitsect points to this commit to be the source of the problem: c66df16.

[fauxpark]

Can't see anything in the commit list for the last breaking changes PR (https://github.com/qmk/qmk_firmware/pull/11053/commits) that stands out to me. There doesn't seem to have been much activity near the RGBLight stuff this round. I'm thinking it may be something to do with the ChibiOS config files. @tzarc?

[alvinsee]

The offending PR seems to be this change.
It is not apparent to me that platforms/chibios/GENERIC_STM32_F072XB/board/board.c is equivalent to lib/chibios/os/hal/boards/ST_NUCLEO64_F072RB/board.c.

I ended up performing the following steps to revert it back to its intended functionality:

git checkout f079dd9 -- platforms/
make primekb/meridian:via:flash

[fauxpark]

They don't seem to be equivalent. board.c appears to be functionally the same, accounting for whitespace/formatting.

board.h however has significant differences. In particular the STM32_HSECLK defaults to 8000000U, but was 0U in the deleted board.h. I'm not sure if the SPI peripheral uses this.

[alvinsee]

I tried grepping for STM32_HSECLK but failed to find any usages. I've also tried adding the value 0U to config.h, but that seems moot.

[sigprof]

Looking at the message dates here and in #11812, I see that the attempt to fix this board by switching to the bitbang driver was apparently made before PR #13022 was merged, which should have brought the code mostly to the same state as it was before those ChibiOS changes (the problem was that SPI on STM32F072 requires assigning SCK to a physical pin to have a functional SPI receiver, and the SPI driver relies on the receiver part for transfer completion; however, the board defaults for F072 were changed to no longer assign SPI2_SCK to a physical pin by default).

But the Meridian code has one more potential problem that might show up when using the WS2812 SPI driver — it issues several rgblight_setrgb_at() calls without any delay between them. The WS2812 SPI driver may not handle such calls properly with the default options. However, you may try adding the following code to config.h:

#define WS2812_SPI_USE_CIRCULAR_BUFFER

This makes the WS2812 SPI driver behave similar to the PWM driver — it will repeat the bitstream continuously, and any rgblight_setrgb_at() calls will just change the bitstream data in the RAM buffer without even touching the SPI controller.

Alternatively, you can try another option:

#define WS2812_SPI_SYNC

This option makes the WS2812 SPI driver wait until the transfer is completed before returning (similar to the bitbang driver behavior, except interrupts are not disabled during all that time), therefore the error condition (starting a new SPI transfer before the previous one has completed) is avoided. However, this mode stops the normal keyboard processing while the bitstream is sent (which may be really noticeable with larger chains), and if the SPI completion is not detected properly (e.g., because the SCK output is not mapped to a physical pin), the driver will just hang indefinitely, so this mode should probably be used only for testing.

@holtenc @Woovie Could you test these options on the real hardware instead of changes suggested in #11812 or #14678?

[Woovie]

I appreciate you digging into it @sigprof, We will see if works.