keyboardio / Kaleidoscope

Firmware for Keyboardio keyboards and other keyboards with AVR or ARM MCUs.
http://keyboard.io
GNU General Public License v3.0
759 stars 258 forks source link

Proposal for a central point in the sketch for auto-code-generation #761

Open noseglasses opened 4 years ago

noseglasses commented 4 years ago

The sketch is the place where all user configuration comes together.

Based on the configuration in the sketch we do a lot of magic. Macros invoked in the sketch (mainly under the hood) implement functions that are called from other places in the firmware core. Examples are the hooks/handlers of the plugin system, sketch exploration, the generation of HID names and possibly more (to come).

Currently we have two options how to trigger code generation either by sneaking macro invocations into the KEYMAPS(...) or the KALEIDOSCOPE_INIT_PLUGINS(...) array.

The problem is that it can make sense (in special cases) to have a sketch without invoking either of the two macros or just one or the other. Currently there is nothing that warns in such a case or throws an error. Thinks might just silently malfunction.

A possible solution could be to define a macro KALEIDOSCOPE_INIT that must be present at the bottom of any Kaleidoscope sketch. This macro could contain all the code generation stuff that is currently invoked by KEYMAPS(...).

To enforce this macro being present, it could e.g. define a symbol

char ____________Please_add_macro_KALEIDOSCOPE_INIT_to_the_very_bottom_of_your_sketch_____;

that would be dummy-accessed by the core. If the KALEIDOSCOPE_INIT macro would be omitted, this would lead to a quite verbose linker error.

I'd find this a much cleaner solution than the macro-sneaking approach in the long term.

Does that make sense?

obra commented 4 years ago

Not having a guard on the missing code is definitely a bug and I'd be happy to see that addressed directly.

I understand where you're coming from in terms of conceptual cleanliness, but I'm incredibly hesitant to require a change that adds new sketch boilerplate.

While I can certainly envision a world where the KEYMAPS() macro might get skipped in a sketch, "somebody might not have any plugins" feels like a bit of a strawman. It's exceedingly unlikely that a real user would create a sketch with zero plugins and expect it to work.

It's worth looking also at how Arduino processes .ino files - They're already doing a post-processing step. It may be the case that we have a hook there we could use.

noseglasses commented 4 years ago

@obra, you are a genious. The idea of messing with the preprocessed sketch is perfect.

I submitted two PRs (https://github.com/keyboardio/Kaleidoscope/pull/763 and https://github.com/keyboardio/Kaleidoscope-Bundle-Keyboardio/pull/23) that add everything we need to have sketch header and footer include files.

The footer is the place where we could put all our magic initialization stuff that needs to see the whole sketch's definitions.

This also solves another problem that I have spend quite some time on but couldn't solve satisfactorily. It answers the question of how to let code know whether it is compiled in the sketch's compilation unit or in another one. That was something I tried to exploit when looking for a way to reduce compile time. Now after the Runtime-refactoring, I don't see a current need for it but one never knows.

By adding a tag macro, say #define KALEIDOSCOPE_COMPILING_SKETCH to kaleidoscope_internal/sketch_preprocessing/sketch_header.h is all we need to enable conditional compilation.

algernon commented 4 years ago

Out of curiosity, where did you find the sketch hooks? I've been looking at arduino wiki and boards.txt, and a few other places, and never came across these hooks. (Or if I did, I managed to miss them by a mile...)

noseglasses commented 4 years ago

They are here: https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5-3rd-party-Hardware-specification