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 V3 - The Custom UI Update #18222

Merged
merged 31 commits into from
Nov 9, 2022
Merged

VIA V3 - The Custom UI Update #18222

merged 31 commits into from
Nov 9, 2022

Conversation

wilba
Copy link
Contributor

@wilba wilba commented Aug 30, 2022

VIA V3 - The Custom UI Update

This PR contains the changes to VIA code in QMK to support VIA V3 protocol definitions.

  • Custom UI, i.e. defining UI controls in VIA to control firmware
  • Full support for RGB Matrix
  • Delays in macros
  • Lots of refactoring/deleting redundant code in keyboards

In VIA Version 1,the UI and the keyboard definitions were entirely self-contained (i.e. hard-coded). Adding new keyboards requires changing code and rebuilding/releasing the app.

VIA Version 2 moved the keyboard definitions into externally defined files, which are stored in a GitHub repository. The keyboard definitions are served to the VIA client at run-time. This allowed anyone to add new keyboards to VIA without requiring changing the source and rebuilding/releasing the app.

VIA Version 2 keyboard definitions include layout options and control of lighting, but definitions were restricted to the existing UI elements.

VIA Version 3 is a refactoring of how the UI works in VIA, to allow fully customized UI within VIA to control firmware parameters like lighting, but also any custom feature implemented in the firmware, either in QMK Core or at the keyboard level. It works by defining what UI elements VIA should show, and binding those UI elements to IDs, which VIA will use in communication with the firmware.

The full documentation of the changes in VIA V3 can be read here:

https://www.caniusevia.com/docs/v3_changes

https://www.caniusevia.com/docs/custom_ui

The VIA client available at https://usevia.app currently works with V2 and V3 definitions. The V2 definitions still exist in the-via/keyboards/src. These were refactored into V3 definitions in the-via/keyboards/v3. Thus building firmware from this branch, which would require a V3 definition, should work in the VIA app.

Note: while the amount of code changes here seems large, it pales in comparison to the awesome amount of coding that @olivia has done to make VIA what it is today, especially the custom UI feature, the web app, and all the backend optimization that allows VIA to handle 4,000+ sessions a day. Special thanks also to @mattsacks and @ValdisThomann for development, and @yiancar and @Xelus22 for maintenance work. <2

Types of Changes

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

Issues Fixed or Closed by This PR

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 core dependencies keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Aug 30, 2022
@drashna drashna changed the base branch from master to develop August 30, 2022 19:13
@drashna
Copy link
Member

drashna commented Aug 30, 2022

Retargeted to develop, per contribution guidelines.

Also, lib/ugfx needs to be removed, and lint wants a format pass of the code.

Also, ideally, with this change, the keycode definitions need to be updated.

@drashna drashna requested a review from a team August 30, 2022 19:40
@Xelus22 Xelus22 mentioned this pull request Sep 18, 2022
6 tasks
@adophoxia
Copy link
Contributor

Is there anything else needed besides resolving merge conflicts and having it successfully run the QMK CI to have this be on the way to being merged?

@drashna
Copy link
Member

drashna commented Oct 7, 2022

Well, ideally, a bunch of the bodges to ensure that keycodes match with VIA should be removed (such as the mousekeys, and midi), which also requires the via_ensure_keycode.h file to be updated as well.

There has also been added a good number of keycodes added that should be supported tooand added to the "ensure" file, too

@adophoxia
Copy link
Contributor

Ah ok. That's actually good to know.

@drashna
Copy link
Member

drashna commented Oct 7, 2022

Also, the macro section should be merged. via's macros and user keycodes could be safely moved to this block:

// Dedicated macro keys for Configurator and VIA
MACRO_0,
MACRO_1,
MACRO_2,
MACRO_3,
MACRO_4,
MACRO_5,
MACRO_6,
MACRO_7,
MACRO_8,
MACRO_9,
MACRO_10,
MACRO_11,
MACRO_12,
MACRO_13,
MACRO_14,
MACRO_15,
MACRO_16,
MACRO_17,
MACRO_18,
MACRO_19,
MACRO_20,
MACRO_21,
MACRO_22,
MACRO_23,
MACRO_24,
MACRO_25,
MACRO_26,
MACRO_27,
MACRO_28,
MACRO_29,
MACRO_30,
MACRO_31,

@wilba
Copy link
Contributor Author

wilba commented Oct 7, 2022

VIA developers are working on a way to version keycodes etc. so that VIA can be backward compatible with all existing firmware (i.e. their use of the existing keycode values) and accommodate changes to the keycode values. Refactoring how VIA works with respect to keycodes is not trivial and not something that can be done without careful consideration and development time. The decision was made to not include those changes in this PR, causing it to be delayed even more than it has been.

There are plenty of things that can be refactored, improved, etc. and the VIA developers are committed to working on these things. They've put an incredible amount of effort into VIA, including the transition to a web app, so I hope that people can understand the decision to not allow feature creep to delay this PR, which is specifically to do with a change in VIA's protocol and the ability to define custom UI in VIA to support the thing most asked for by users (which is custom lighting).

Refactoring keycodes is definitely top of the list of things in the next breaking change PR, and everyone's suggestions are welcome.

@tzarc
Copy link
Member

tzarc commented Oct 8, 2022

VIA developers are working on a way to version keycodes etc. so that VIA can be backward compatible with all existing firmware (i.e. their use of the existing keycode values) and accommodate changes to the keycode values. Refactoring how VIA works with respect to keycodes is not trivial and not something that can be done without careful consideration and development time. The decision was made to not include those changes in this PR, causing it to be delayed even more than it has been.

Good to hear. QMK has also been working in this area, so in order to ensure you guys have alignment with our direction, we've fast-tracked #18643 to give you ample time to work it in with your plans. I'll ask for a VIA version bump in that PR so that you've got something to check from a compatibility perspective.

quantum/via.h Outdated Show resolved Hide resolved
quantum/via.h Outdated Show resolved Hide resolved
quantum/via.h Outdated Show resolved Hide resolved
quantum/via.h Outdated Show resolved Hide resolved
quantum/via.h Outdated Show resolved Hide resolved
quantum/via.h Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
@drashna drashna requested a review from a team October 8, 2022 16:33
@drashna drashna requested a review from a team October 25, 2022 05:43
@wilba
Copy link
Contributor Author

wilba commented Oct 31, 2022

Is there anything else required to get this merged this breaking change round?

@tzarc
Copy link
Member

tzarc commented Nov 1, 2022

Is there anything else required to get this merged this breaking change round?

Not really, it'll likely get included once reviews take place. I'll label it accordingly.
As previously mentioned, #18643 is going to go in this cycle too, so it might be wise sticking a version check on the webapp so as to prevent using older-value keycodes with newer versioned firmware.

I'll try and sort out a review later today.

@zvecr
Copy link
Member

zvecr commented Nov 1, 2022

Is there anything else required to get this merged this breaking change round?

Its still planned for this cycle, with the aim to be merged before the Nov 12 deadline outlined in the docs.

As for what can be done to help it along, all non via related changes to user code should be reverted.

keyboards/work_louder/micro/info.json Outdated Show resolved Hide resolved
keyboards/work_louder/work_board/rules.mk Outdated Show resolved Hide resolved
@wilba
Copy link
Contributor Author

wilba commented Nov 2, 2022

As for what can be done to help it along, all non via related changes to user code should be reverted.

The user code changes are to ensure the keyboard firmware with VIA enabled will build and not be too large. Sometimes this involved fixing little things unrelated to the changes in Core (i.e. it is already broken) but I thought that was worth doing anyway, since those changes will most likely be redundant and merge cleanly if done in another commit.

Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also pinging @yoichiro -- new VIA version, new macro format/capabilities.
As per the other discussion over on #18643, should probably sort out version checks on both VIA and Remap.

keyboards/cannonkeys/satisfaction75/satisfaction75.h Outdated Show resolved Hide resolved
keyboards/custommk/evo70/keymaps/via/config.h Outdated Show resolved Hide resolved
keyboards/custommk/evo70/keymaps/via/keymap.c Outdated Show resolved Hide resolved
keyboards/geekboards/macropad_v2/keymaps/via/config.h Outdated Show resolved Hide resolved
users/talljoe/talljoe.h Outdated Show resolved Hide resolved
users/talljoe/talljoe_keymap.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
quantum/via.c Outdated Show resolved Hide resolved
@wilba
Copy link
Contributor Author

wilba commented Nov 8, 2022

VIA has been updated to use the new int values from #18643, still to do is the refactoring to the changed keycode names and adding new keycodes to the UI.

#18643 changed the VIA protocol version to 11, which was merged to develop. Because of this, for firmware built from develop right now, VIA will now load the V3 definitions and use the changed command handling that is in this PR. Thus currently, things are a bit broken until this PR gets merged.

@tzarc
Copy link
Member

tzarc commented Nov 8, 2022

VIA has been updated to use the new int values from #18643, still to do is the refactoring to the changed keycode names and adding new keycodes to the UI.

#18643 changed the VIA protocol version to 11, which was merged to develop. Because of this, for firmware built from develop right now, VIA will now load the V3 definitions and use the changed command handling that is in this PR. Thus currently, things are a bit broken until this PR gets merged.

I'd recommend making it dynamic if possible -- until the next develop->master merge we might find that some need reordering. Only once it hits master you should consider it to be properly "locked in".

There's some API work coming that will officially set up the publishing pipeline, so you should be able to pull from that URL as required.

@tzarc tzarc merged commit bc6f8dc into qmk:develop Nov 9, 2022
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
elpekenin pushed a commit to elpekenin/qmk_firmware that referenced this pull request Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants