A better way to override the short name

[algernon]

There's a function in PluggableUSB called getShortName(), which is used to send a response to an ISERIAL descriptor request. We use this to send a device-specific name back, like kbio01, atreus, etc. The way we accomplish this overrideability is by marking HID_::getShortName() as a weak symbol, and overriding it with our custom function that calls Kaleidoscope.device().getShortName(). Thus, the linker will use our override for HID_::getShortName().

This has a number of downsides, including that of not being particularly clean, and requiring us to inject the override somewhere into the firmware, which we currently do by doing it as part of the KEYMAPS() macro - something completely unrelated.

So, I was thinking about what would be a better way to accomplish the same thing? One that doesn't require us to do a link-time function override, nor using an unrelated macro to accomplish the override.

I see two paths that might be worth exploring:

A shortName member variable

Using a member variable, and an appropriate setter has the benefit of being very flexible, and allows us to change the shortname at any time, during runtime. The downside of it is that with the shortname no longer being constant, this is slightly more code and data used. It's trivial to implement, though, and can be entirely backwards compatible.

Inheritance

Another option, which is considerably more complicated, would be to not initialize a HID object, but only ship the class in the library, and defer the instantiation to users of the library, such as Kaleidoscope. We could then update Kaleidoscope's driver::hid::KeyboardioHID to subclass KeyboardioHID, and override getShortName() there, at the c++ level, rather than at link time.

This has the advantage of the short name still being constant. The downside is that the library would no longer be a drop-in replacement for Arduino's HID, but slightly more Kaleidoscope-specific, and it breaks backwards compatibility, as users of the library will have to instantiate a HID object themselves. With our major user being Kaleidoscope, I believe that such a breakage is acceptable.

[algernon]

Hm, inheritance may end up triggering the same issues we had in #59, but that was years ago, so we'd need to compare sizes all over again.

[obra]

arduino/ArduinoCore-avr#387 may be relevant here.

…

On Sat, Sep 25, 2021 at 4:06 AM Gergely Nagy ***@***.***> wrote: Hm, inheritance may end up triggering the same issues we had in #59 <#59>, but that was years ago, so we'd need to compare sizes all over again. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#70 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAALC2GMTXWE3SFU7YRLJIDUDWULDANCNFSM5EXPR56Q> .

[algernon]

That PR pretty much implements the first method I described, with a very important twist: the shortname is stored in PROGMEM, rather than RAM. That's a great idea, but is pretty platform specific. Not a problem for ArduinoCore-avr's HID, because that's an AVR-specific HID. KeyboardioHID isn't.

We can ifdef around, I suppose, and use RAM on non-AVR (I don't feel its worth using progmem on ARM, in this particular case).