tmk / tmk_keyboard

Keyboard firmwares for Atmel AVR and Cortex-M
3.99k stars 1.71k forks source link

New better API for feature extension #304

Open tmk opened 8 years ago

tmk commented 8 years ago

When users want to add special feature to their keyboard current API is obviously poor or doesn't allow them to do so. For example, LCD support: https://github.com/tmk/tmk_core/pull/2

Event hooks(Callback)

I'm thinkg about to insert callbacks where interested like:

Power cycle events

These events may not be supported in some protcol implementations.

Need more generic API instead of eeconfig.

Magic commands

BootMagic commands

flabbergast commented 8 years ago

This would be great to have! Hopefully this will also allow a clean integration of advanced features that need extra libraries, e.g. LCD, LEDs-over-I2C. Would be also good if the main scan loop hook would be good enough to cleanly implement split PCBs (e.g. Ergodox). I'll try to write some code and report.

flabbergast commented 8 years ago

Here's the code: https://github.com/flabbergast/tmk_core/commit/1c8e8aae8ea15a37f678d70ad6f86e731efcf7df The hooks are implemented as weak functions, all the declarations are in hooks.h, the default hardware-independent definitions in hooks.c, and the hardware-dependent ones are so far only done for chibios so far (I'd add them for the other protocols as well, if this approach would be deemed acceptable).

The list of hooks can be seen here: https://github.com/flabbergast/tmk_core/blob/hooks/common/hooks.h

The ones that I'd want to still do is for tapping and for holding.

I think with this most of the requested features can be now implemented on the keyboard side; mainly using one of the init hooks and then either matrix_change_hook or scan_loop_hook. The keyboard side has access to all the global variables (global timer, matrix state, led state, etc...), so it is possible to implement timeouts and things like that if needed. {Of course on chibios one can define extra threads like this: https://github.com/flabbergast/flabber_kbs/blob/master/kb45p/user_thread.c }

I'm not sure about the LED hooks - I've separated to two cases (change from host, restore from firmware), but this is maybe unnecessarily complicated, and we should just stick with led_set. I don't think that the core needs more LED-specific stuff - everything can (and IMO should) be implemented using _hooks, and for some extra fancy backlight stuff one can also use action_function: a-la https://github.com/flabbergast/flabber_kbs/blob/master/6shooter_tsy3.2/keymap_plain.c#L64-L75 .

For the suspend/wakeup hooks, I've moved a bit of the code into those - so that it's possible to completely customise the behaviour (e.g. write own sleep/power_save code, etc...)

tmk commented 8 years ago

Thank you, @flabbergast . Looks neat! I did cherry-pick your commit and tweak it a bit. And created newapi branch. https://github.com/tmk/tmk_core/commits/newapi

I'll have to rethink about LED stuff. Yes, I like to separate backlight and sleep LED from core.

flabbergast commented 8 years ago

Thanks! Of course feel free to rename the hooks any way you see fit.

I agree with separating the backlight support from the core. It's possible to implement it with hooks now (setup in one of the init hooks, and controlling it through action_function). The only thing left to do in core is to make it possible for a KB to save some information (e.g. the current backlight level) into EEPROM.

I propose the following: add one (or two or 4) bytes to the EEPROM mechanism as "user bytes", and expose hooks to read and write these. I think this is general enough to be useful to enable "remembering the state across reboots". If someone wants to write more stuff to EEPROM (e.g the whole layout or something like that), they should write keyboard-specific EEPROM code.

While we're at it, we might as well make bootloader_jump and the low-level eeprom functions also ((weak)), so that people can implement those themselves if they run on new chips that are not supported by TMK directly, or use non-so-often used bootloaders. https://github.com/tmk/tmk_keyboard/issues/256

If you think this proposal makes sense, I can write some code ;) This would be a big change, as it would break the current backlight support, and people would have to migrate their configs. However I would write a guide about how to go about that.

fredizzimo commented 8 years ago

I have added this pull request https://github.com/tmk/tmk_core/pull/9, since I still needed a couple of things for the Infinity Ergodox

njbair commented 8 years ago

So just to be clear, would this API allow users to define the callback functions in their keymap file? For instance, right now if I want to enable layer 5 on boot, I have to put the layer_on(5) code inside the matrix_init() function in matrix.c. It would be much cleaner and easier to follow if it were a callback in the keymap file. Or, even better, #include callback.c at the bottom of the keymap file.

njbair commented 8 years ago

Also, for the bootmagic callback, would that allow you to define your own bootmagic commands? This would be extremely useful, especially if you could define a secondary salt key to add a whole new "layer" of bootmagic commands.

flabbergast commented 8 years ago

Yes, this would allow the users to define their own callbacks. It does not matter in which .c file they would be (as long as they get compiled, i.e. they are defined in a file that is included to be compiled in the Makefile). {BTW, you can define the matrix_init function in whatever .c file you like as well. Or you can make your own 'set_starting_layer' function, call it in matrix.c and define it in keymap if you want to.}

Ad bootmagic: yes, you can define your own bootmagic commands with this.

njbair commented 8 years ago

Thanks, @flabbergast. Your suggestion was really helpful. I created a new file in my project root called init.h which includes a function to enable certain layers by default. Then I included that file in my keymap file header, and I call the function inside matrix_init() in matrix.c.

As someone well-versed in JavaScript/PHP/Python, I think if I was more familiar with compiled programming processes I would have been able to figure that all out. But I still get pretty confused by preprocessing, compile time vs runtime, etc.

But I am learning!

tmk commented 8 years ago

Updated first comment and chaged name of event hook functions *_hook to hook_ and file names to hook.c / hook.h.

I added 'develop' branch in tmk_core repo and merged newapi branch. Also @flabbergast 'chibios' branch were merged. I'll use and test the develop branch for a while.

https://github.com/tmk/tmk_core/tree/develop