Naming of plugin headers and class instances diverges

[noseglasses]

i had a hard time establishing a testing sketch that includes and uses all plugins.

It would be desirable to have a common naming scheme for plugin headers and instance names. This would make it easier for the user to include a plugin in the sketch. For some plugins there is a difference between the plugin header and the name of the object that is passed to KALEIDOSCOPE_INIT_PLUGINS(...). For example Kaleidoscope-Heatmap.h and HeatmapEffect. To remain backward compatible, this could be solved with either providing macros alias such as #define Heatmap HeatmapEffect or by using wrapper headers, e.g. Kaleidoscope-HeatmapEffect.h.

[noseglasses]

Here's a table with those core plugins where naming differs.

Plugin Object	Plugin Header
EscapeOneShot	Kaleidoscope-Escape-OneShot.h
Focus	Kaleidoscope-FocusSerial.h
HeatmapEffect	Kaleidoscope-Heatmap.h
HostPowerManagement	Kaleidoscope-HostPowerManagement.h
LEDActiveLayerColorEffect	Kaleidoscope-LED-ActiveLayerColor.h
ActiveModColorEffect	Kaleidoscope-LED-ActiveModColor.h
AlphaSquareEffect	Kaleidoscope-LED-AlphaSquare.h
BootAnimationEffect	Kaleidoscope-LEDEffect-BootAnimation.h
BootGreetingEffect	Kaleidoscope-LEDEffect-BootGreeting.h
LEDBreatheEffect	Kaleidoscope-LEDEffect-Breathe.h
LEDChaseEffect	Kaleidoscope-LEDEffect-Chase.h
LEDRainbowEffect	Kaleidoscope-LEDEffect-Rainbow.h
LEDPaletteTheme	Kaleidoscope-LED-Palette-Theme.h
StalkerEffect	Kaleidoscope-LED-Stalker.h
WavepoolEffect	Kaleidoscope-LED-Wavepool.h
TestMode	Kaleidoscope-Model01-TestMode.h
GeminiPR	Kaleidoscope-Steno.h
USBQuirks	Kaleidoscope-USB-Quirks.h

[algernon]

Focus / FocusSerial is intentional, because we want to keep the option open to have different Focus implementations that use something else than USB Serial, but has the exact same interface.

Similarly, Kaleidoscope-Steno.h provides GeminiPR right now, but there are plans to make it provide TxBolt too (another Steno protocol), within the same plugin.

For most others, especially LED effects, I agree, more consistent naming would be beneficial.

[noseglasses]

Focus / FocusSerial is intentional, because we want to keep the option open to have different Focus implementations that use something else than USB Serial, but has the exact same interface.

Similarly, Kaleidoscope-Steno.h provides GeminiPR right now, but there are plans to make it provide TxBolt too (another Steno protocol), within the same plugin.b

Would it make sense to provide a default object through a macro that uses a full qualified symbol name?

#define Steno kaleidoscope::plugin::steno::GeminiPR

[noseglasses]

A possible solution for the name conflicts would be to turn the current plugin-headers in Kaleidoscope/src into wrappers that include newly introduced plugin headers whose names would match their plugin instances. The wrappers would then ideally contain deprecation warnings and would be removed in future firmware versions.

Another thing that occurred to me is that it is not easy to find out which of the files in Kaleidoscope/src belong to actual plugins, i.e. to something that comes with hooks and that must be passed to KALEIDOSCOPE_INIT_PLUGINS(...).

As the naming conflicts apply to a significant number of plugins, we could take advantage of the situation and introduce header filenames that are making it easier for the user to identify actual plugins, like e.g.

Plugin-LEDActiveLayerColorEffect.h

or

Kaleidoscope-Plugin-LEDActiveLayerColorEffect.h

[algernon]

Would it make sense to provide a default object through a macro that uses a full qualified symbol name?

That's a good idea, thanks!

Another thing that occurred to me is that it is not easy to find out which of the files in Kaleidoscope/src belong to actual plugins

All of them, except Kaleidoscope.h. Any non-plugin headers are in src/kaleidoscope/ or src/kaleidoscope_internal/. As such, I wouldn't add a -Plugin- tag to the header names, because all of them - save the one Kaleidoscope.h - are plugin headers.

A possible solution for the name conflicts would be to turn the current plugin-headers in Kaleidoscope/src into wrappers that include newly introduced plugin headers whose names would match their plugin instances. The wrappers would then ideally contain deprecation warnings and would be removed in future firmware versions.

This is doable. This would be a good opportunity to fix some of the naming inconsistencies too (LED- vs LEDEffect-). We have two kinds of LED effects: LED modes and stuff that overrides, and works independently of LED modes. I'd call the first LEDModeSomething, the latter LEDEffectSomething: LEDModeHeatmap and LEDActiveLayerColorEffect, for example. We can then provide compat objects and headers.

@obra, @noseglasses what do you both think?

[noseglasses]

Another thing that occurred to me is that it is not easy to find out which of the files in Kaleidoscope/src belong to actual plugins

All of them, except Kaleidoscope.h

There are some Kaleidoscope-Hardware-...h hardware wrappers in Kaleidoscope/src as well. For us devs its crystal clear that those are not plugins. But as all other headers, including the plugins start with Kaleidoscope- it might be less clear to the user.

Is it for technical reasons that those hardware wrapper-headers must reside in Kaleidoscope/src? AFAIK, the actual hardware header that matches the target keyboard is included by default in the sketch and those hardware wrappers are not supposed to be included by users.

We have two kinds of LED effects: LED modes and stuff that overrides, and works independently of LED modes. I'd call the first LEDModeSomething, the latter LEDEffectSomething: LEDModeHeatmap and LEDActiveLayerColorEffect, for example. We can then provide compat objects and headers.

To me as a developer, this distinction makes some sense. But I am not sure if including such a technical detail in the header-filenames and the plugin-instance names actually helps users. From a user's perspective all plugins are treated the same within the sketch. Thus, a user does not/should not care for the implementation (here type/inheritance) of a plugin. For a user it just needs good telling names to make it easier to distinguish all those plugins.

Would it possibly make sense to have different sub-directories of src/kaleidoscope/plugin that separate plugins by their purpose/type/base classes/etc., if necessary. Like src/kaleidoscope/plugin/led_effects and src/kaleidoscope/plugin/led_modes. This would help the developers to keep a better overview over the growing number of plugins and their types.

As we have wrapper headers in Kaleidoscope/src for all stock plugins already, it should be possible to move the actual plugin header and implementation files to such sub-directories without causing incompatibilities.

If there would be concerns about users accidentally including headers from src/kaleidoscope/plugin in their sketches, compatibility wrapper headers with deprecation warnings could be used for a while, that inform users that they are not supposed to include a specific header in src/kaleidoscope/plugin anymore or, even better, to point them to the equivalent header in Kaleidoscope/src.

[noseglasses]

Excuse me, what i wrote above somehow mixes two distinct subjects, usability and maintainability. Both are slightly intertwined through filenames but could be dealt with separately and also have different levels of importance.

[algernon]

Is it for technical reasons that those hardware wrapper-headers must reside in Kaleidoscope/src? AFAIK, the actual hardware header that matches the target keyboard is included by default in the sketch and those hardware wrappers are not supposed to be included by users.

Ooops, forgot about those. They're in src/ for historical reasons mostly: when they were in a separate repository, we needed them at top-level for Arduino to find them. Hardware plugins that are in the main repo can do without those, so we can remove them, once the default includes are adjusted. That will need a deprecation period, because boards.txt is in another repo, and we can't remove the top-level hardware headers until boards.txt is updated.

For a user it just needs good telling names to make it easier to distinguish all those plugins.

Good point, agreed.

Would it possibly make sense to have different sub-directories of src/kaleidoscope/plugin that separate plugins by their purpose/type/base classes/etc., if necessary. Like src/kaleidoscope/plugin/led_effects and src/kaleidoscope/plugin/led_modes. This would help the developers to keep a better overview over the growing number of plugins and their types.

Better grouping would certainly be useful from a developer/maintainer point of view.

As we have wrapper headers in Kaleidoscope/src for all stock plugins already, it should be possible to move the actual plugin header and implementation files to such sub-directories without causing incompatibilities.

Yup. The top-level headers just include the appropriate header in a subdir already, and since top-level headers are the only ones a user is expected to use in their sketch, we're free to move the other headers around. I wouldn't even keep compat headers around, since including those headers directly was never a supported thing.

So, the next step: come up with a grouping that makes sense. I'd love to hear @obra's opinion too. He's good at naming things.

[noseglasses]

Just for the sake of completeness: The header kaleidscope/src/Kaleidoscope-Hardware-Model01.h could be renamed to kaleidscope/src/Kaleidoscope-Hardware-Keyboardio-Model01.h to be in sync with the other hardware headers.