Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VIA Encoder Map Support #17734

Merged
merged 3 commits into from
Jul 20, 2022
Merged

VIA Encoder Map Support #17734

merged 3 commits into from
Jul 20, 2022

Conversation

wilba
Copy link
Contributor

@wilba wilba commented Jul 20, 2022

As per the-via/app#26 this change adds VIA support for the encoder map functionality that is already in QMK.

The current version of VIA (i.e. what is live now at https://usevia.app/#/) now supports this minor change to the protocol, to get and set the encoder map.

via_encoders_ui_example

This change has no impact on existing firmware and VIA functionality. VIA works as usual for firmware built before this change, and these new commands will only be called by VIA if the keyboard definition includes the encoders and the firmware reports the correct protocol version. This allows keyboard maintainers to update at their discretion.

How to implement

  • Enable encoder maps as per QMK
  • Add ENCODER_ENABLE = yes to rules.mk
  • Add ENCODER_MAP_ENABLE = yes to keymaps/via/rules.mk
  • Add an encoder map to keymaps/via/keymap.c
  • The encoder map should be defined for the same number of layers as configured for VIA (default 4)
  • Add the encoder to the VIA keyboard definition
  • Add a "key" to the KLE JSON with e0, e1, etc. as the center label. The number will match the encoder ID used in the encoder map.
    via_encoders_kle_example
  • If the rotary encoder has a push switch, define the switch matrix co-ordinates like other switches
    via_encoders_kle_example_2
  • If the rotary encoder is optional (combined switch/rotary encoder footprint), use VIA Layout Options like other switches. VIA can render either a knob or switch or empty space.
    via_encoders_kle_example_3

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added the core label Jul 20, 2022
@tzarc tzarc requested a review from a team July 20, 2022 23:48
@tzarc tzarc changed the base branch from master to develop July 20, 2022 23:48
@tzarc tzarc merged commit 4efe633 into qmk:develop Jul 20, 2022
@vinorodrigues
Copy link
Contributor

vinorodrigues commented Jul 23, 2022

hey @wilba - is the dynamic_keymap_reset() function in dynamic_keymap.c working for encoders. I'm asking because I'm getting KC_NO across each layer cw & ccw values of the encoder when I reset the EEPROM.

Also your "write" code has some << 8 happening that I can't see in dynamic_keymap_reset:

via.c > raw_hid_receive():

case id_dynamic_keymap_set_encoder: {
            dynamic_keymap_set_encoder(command_data[0], command_data[1], command_data[2] != 0, (command_data[3] << 8) | command_data[4]);
            break;
        }

vs.

dynamic_keymap.c > dynamic_keymap_reset:

#ifdef ENCODER_MAP_ENABLE
        for (int encoder = 0; encoder < NUM_ENCODERS; encoder++) {
            dynamic_keymap_set_encoder(layer, encoder, true, pgm_read_word(&encoder_map[layer][encoder][0]));
            dynamic_keymap_set_encoder(layer, encoder, false, pgm_read_word(&encoder_map[layer][encoder][1]));
        }
#endif // ENCODER_MAP_ENABLE

Could that be it?

@vinorodrigues
Copy link
Contributor

Lesson Learned: When porting existing code to this functionality on an existing KB one will need to clear the dynamic memory portion on the KB. How depends on what type of kb one has:

  1. AVR/ATMega - use the "Clear EEPROM" button in QMK_Toolbox
  2. ARM/STM32 (using EFL/WL driver) - (do nothing, the flashing will erase the buffer)
  3. ARM/STM32 (using External EEPROM) - Toolbox does not clear EEPROM on ARM's. One will need to manually clear the EEPROM by temporarily mapping a key to the QK_CLEAR_EEPROM function.

@sigprof
Copy link
Contributor

sigprof commented Jul 24, 2022

ARM/STM32 (using EFL/WL driver) - (do nothing, the flashing will erase the buffer)

This actually happens only on the STM32F4x1 chips, and only when not using the tinyuf2 bootloader. The reason why it happens on STM32F4x1 with stm32-dfu is that the flash structure of those chips is non-uniform, and the only place where it has 16K erase blocks is at the start of flash (there are 4×16K erase blocks, then 1×64K, the rest are 128K); one of those 16K blocks is used as the EEPROM backing store, so it ends up in the middle of the firmware and gets overwritten when flashing the firmware. (It could be possible to change that by generating the firmware in the ST-specific “DfuSe” format, which supports sparse regions, so that the EEPROM backing store could be skipped when flashing the firmware; however, doing that is complicated, not only because some tool to generate the DfuSe format must be implemented, but also because flashing the firmware in the DfuSe format would require using different command line options for dfu-util, which then don't work for the usual DFU format, therefore QMK Toolbox would need to parse the firmware file to detect its format.)

When using the tinyuf2 bootloader on STM32F4x1, the bootloader occupies the first 32K of flash, but the firmware starts at the 64K offset; the EEPROM backing store is located in the 48…64K range and is preserved when flashing the firmware.

When using most other STM32 chips, the EEPROM backing store is located at the end of internal flash and is preserved when flashing the firwmare.

However, even though the EEPROM backing store may be preserved in some cases, VIA also checks the magic number which is derived from the firmware build date; therefore, if you flash a firmware which was built on a different day than the previously flashed version, the dynamic keymap will still be reset to default. But rebuilding the firmware multiple times during the same day won't trigger that reset — in that case you would need to perform EEPROM reset manually.

And if you are using Bootmagic to enter the bootloader, that would reset the EEPROM too.

@wilba
Copy link
Contributor Author

wilba commented Jul 24, 2022

@vinorodrigues I got the message and looked at the code again to see if I could explain your symptoms but was too slow to assist, sorry. Glad you got it sorted.

@wilba
Copy link
Contributor Author

wilba commented Jul 24, 2022

@sigprof If you can think of a neat way of increasing the granularity of the firmware timestamp in the magic number, let me know 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants