MidEvil keyboard support

[dcpedit]

Description

Support for MidEvil keyboard PCB

Types of Changes

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

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).

[elpekenin]

Does the code work? Like, the exact same files on the PR

• Not assigning a MISO pin should fine since Chibios SPI driver: allow some SPI pins to be left unassigned #20315, but it is only available on develop as of now
• Didnt specify the SPI driver to be used
• These names for MOSI/SCK aren't the ones that spi_master.h(or .c) use -and i dont see any sort of aliasing on this PR files-, so im pretty sure it shouldn't even compile. For example, fallback value for SCK is B15, which wouldn't even be a valid pin under RP2040.

[dcpedit]

Yes, it compiles and works on my keyboard. Here's a video: https://imgur.com/a/JDhwkuG

I basically just followed the documentation for quantum painter, and used the handwired onekey lvgl code as an example:
https://github.com/qmk/qmk_firmware/blob/master/docs/quantum_painter.md?id=quantum-painter-cli#-ili9488-
https://github.com/qmk/qmk_firmware/tree/master/keyboards/handwired/onekey/keymaps/lvgl

Isn't the driver defined in rules.mk:

QUANTUM_PAINTER_DRIVERS += ili9488_spi

And the pins defined when calling qp_ili9488_make_spi_device?

[elpekenin]

I mean the SPI driver (hardware on the RP to emit such protocol), as the MCU has two of them and it needs to be setup.
Not the device driver (low level code to interact with the display)

As per pins, the example you linked uses SPI_SCK_PIN (and same for MISO and MOSI), which as you can see arent used on the make_device function as they are hardcoded in the file i named (spi_master). Thus, naming them LCD_ shouldnt work afaik

[dcpedit]

I think I understand. I made another commit to fix the defines. Tested the code again to make sure it compiles and works with the keyboard. Not sure why it was fine before the fix. Also, the display doesn't use a MISO pin, which should be fine according to the QP docs:

Most TFT display panels use a 5-pin interface -- SPI SCK, SPI MOSI, SPI CS, D/C, and RST pins.

If it has to be used, I don't have any extra pins I left on my Pi Pico. Can it be left unassigned, or can I assign a bogus value?

[elpekenin]

Will be fine with the changes on develop for the SPI code, you can change the target branch. And you'd then set it to NO_PIN

[sigprof]

and i dont see any sort of aliasing on this PR files-, so im pretty sure it shouldn't even compile

The defaults are somewhat hidden:

• platforms/chibios/mcu_selection.mk sets BOARD ?= GENERIC_PROMICRO_RP2040 as the default board for RP2040.
• platforms/chibios/boards/GENERIC_PROMICRO_RP2040/configs/config.h sets the default values for I2C, SPI, USART pins to match the Pro Micro RP2040 pinout:

#if !defined(SPI_DRIVER)
#    define SPI_DRIVER SPID0
#endif

#if !defined(SPI_SCK_PIN)
#    define SPI_SCK_PIN GP18
#endif

#if !defined(SPI_MISO_PIN)
#    define SPI_MISO_PIN GP20
#endif

#if !defined(SPI_MOSI_PIN)
#    define SPI_MOSI_PIN GP19
#endif

So the default SPI_MISO_PIN is GP20, which actually conflicts with one of the row pins. However, with a COL2ROW matrix the row pins get reconfigured on every matrix scan, and the SPI code sets up the pins only once during init, therefore the GP20 pin ends up being reconfigured as a plain GPIO; this does not prevent the SPI peripheral from working (on RP2040 it probably results in the peripheral always seeing 0 on the unmapped input), but still looks unclean in comparison with the proper solution (using NO_PIN with the code in the develop branch).

[elpekenin]

I would maybe add some way for user to disable the animation being done by the keyboard-level file. And a few keys on the JSON have too many decimal places (precision will be lost when drawing, and doesnt need to be 100% perfect anyway). Other than that... Looks good to me

[dcpedit]

I would maybe add some way for user to disable the animation being done by the keyboard-level file. And a few keys on the JSON have too many decimal places (precision will be lost when drawing, and doesnt need to be 100% perfect anyway). Other than that... Looks good to me

Can I instead set QUANTUM_PAINTER_DISPLAY_TIMEOUT = 30000 so that it turns off automatically?

[elpekenin]

That wouldn't help at all.

• Code still being run (and slowing down scan rate), just not seen due to screen being off
• Main concern i had: Users have no way to disable this default behaviour from their keymaps. So they would have to edit keyboard-level code if they wanted to draw anything else

[dcpedit]

I've added a way to toggle the animation with a keypress

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[dcpedit]

Awaiting review

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.