keyboardio / Kaleidoscope

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

Focus commands for controlling layers #780

Closed algernon closed 4 years ago

algernon commented 4 years ago

I'm about to write a small plugin to allow changing layers via focus. I have two sets of commands in mind, both with their pros and cons, and would like to solicit opinions about which one to implement.

One set is layer.activate, layer.deactivate, layer.move, layer.isActive, and layer.state. These activate, deactivate, move to the given layer, or check if its active. And layer.state would be used to get/set the whole state, every layers.

The other set is layer.state N [state] and layer.states [states...]. The first would return or set the state of layer N (thus replace .activate and .deactivate, and .isActive). The second would get/set the whole state (replacing .move and .state in the previous set).

The first is more userfriendly, but is more code, and noticably more PROGMEM.

The second is much less code, and much less PROGMEM, but a bit more awkward to work from the host side.

Though, for the primary purpose this will likely serve (activating/deactivating one layer at a time), the two are almost equally easy to use: layer.activate N vs layer.state N 1.

obra commented 4 years ago

the layer.state implementation sounds like it pushes the bookkeeping onto the client -and- doesn't match the way that we do things inside Kaleidoscope keyboard firmware sketches. Which definitely makes it much less appealing.

I feel like exposing the same shiftToLayer, lockLayer and moveToLayer primitives we have inside sketches is conceptually cleaner and more friendly to implementors.

algernon commented 4 years ago

Will go with the first then. Thanks!