PORTICO: add support for wt_rgb

[TerryMathews]

Description

Updates necessary to move Portico from RGB_MATRIX to WT_RGB

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

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

[drashna]

This is not a wilba tech board, and should not be calling code from another keyboard's folder.

In fact, the removal of the core feature is the wrong direction. At the BARE minimum that needs to stay in, even if it's disabled by default.

[TerryMathews]

Sorry, I was trying to emulate what others have done before in order to bring per-key RGB light control into VIA. There are a number of keyboards that have had support added in this way that are not Wilba Tech boards - HS60, NK65, NK87, Dawn60, and I'm sure there are others in the list as well.

I will be glad to structure this PR however the maintainers feel it should be structured, however I don't think I would describe what I did as removing a core feature; more like we swapped out one core feature implementation for another.

Would it be better for us to fork wt_rgb_backlight.c and place it inside our project folder? That seems duplicative, but I can certainly do it if it is what is needed.

[fauxpark]

Well, the idea is that we will obsolete the wt_rgb code, disallowing support for any new boards, and combine as much of its functionality as possible into core RGB Matrix, eventually. So switching away from RGB Matrix is kind of a step backwards, even though you are gaining VIA support.

[drashna]

To be honest, those boards shouldn't have been allowed to use the code either. If code is being shared, then should be pushed into core. And by core, I mean in quantum or tmk_core.

the wilba tech code would not be considered part of core, because of it's location, and it's configuration.

It should be moved into quantum, or ideally, merged into the core RGB Matrix feature. But that also requires support in the VIA app, as well. (or intercepting and interpreting the raw hid calls)

Would it be better for us to fork wt_rgb_backlight.c and place it inside our project folder? That seems duplicative, but I can certainly do it if it is what is needed.

No. This would cause fragmentation of the code, and make it a nightmare to clean up later. Much like the split code.

Eg, this is a tough spot, and we're sorry that you been caught in it.

[TerryMathews]

I'm willing to put in the legwork to have both implementations in and use ifdefs to navigate the correct codepaths, but if the official stance here is that QMK will not allow the Portico to have RGB LED controls working in VIA as other similar boards have done I ask that it be explicitly stated here please.

Barring that, I ask for a list of changes to this PR under which it will be allowed to be merged and I will work on it promptly. I think I've demonstrated over the years, we don't let our keyboard code stagnate even when it was in the correct style in the past. SSD1306 screen support on TKC1800 is the example I can think of off the top of my head. We had that PR merged at the time in the style it was supported, which was not very well organized. As the support for those screens matured in the core code, we did reimplement and utilize the common code even though it was a lot of work for us with very little upside in terms of new features added.

I would ask that the ramifications be carefully considered here. Disallowing a feature on one board that another very similar board supports is significantly punitive.

[drashna]

I would be okay with allowing both sets of features, at least for the time being.

[drashna]

it also looks like if you use the "custom lighting" option, you can still use the via lighting stuff

[TerryMathews]

OK, the core RGB is added back in and I've added conditionals to navigate the codepaths in the various source files depending on whether we're using wt_rgb or rgb_matrix. I've tried it, it compiles successfully both ways.

[TerryMathews]

@fauxpark Can you take a look at this again and hopefully approve?

[TerryMathews]

@drashna Just checking in on this.

[TerryMathews]

@drashna @fauxpark Is there anything I still need to do to get this merged? It's been pending further review for 28 days now.

[fauxpark]

I think I'd like to get a signoff from @wilba first.

[TerryMathews]

I think I'd like to get a signoff from @wilba first.

And that's perfectly fine. I assume they won't have a problem with it, they already merged the changes into VIA's JSON library.

[drashna]

Thanks!