-
-
Notifications
You must be signed in to change notification settings - Fork 40.7k
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
Port VIA Support over to core feature #4910
Conversation
@Wilba6582 If this is stepping on your toes, I will apologize up front. Otherwise, this should allow a Proton C type conversion for VIA boards. It should be very seamless, actually. Though, this needs testing, and I do have one question: Also, this needs documentation, before it can be merged. |
@drashna yes, it is stepping on my toes.
Basically, VIA Configurator (and the QMK changes to support it) are a work in progess, I've already implemented the generic parts like raw HID messaging and dynamic keymaps and pushed them up into core so that they are not coupled and can be used in flexible ways on a per-keyboard basis. I've started the work to generalize things so that common code between keyboard implementations is shared rather than redundant duplication. Even I have not even finished refactoring how this works with the keyboard implementations that I wrote (Zeal60, M60-A, KOYU, M6-A, M6-B, M10-B, WT8-A, WT60-A, WT65-A, WT80-A). The recent proof-of-concept of making a Furthermore, no one cared about my dynamic keymaps code when it was just in Zeal60 and didn't have a pretty GUI front-end. Now that people see VIA and the original vision I had for this way of using QMK, everyone is in such a mad rush to make it work for their keyboards. @olivia and I are perhaps half way done, there's still a lot to do on the VIA Configurator side, it's not even close to being feature complete, meanwhile, I'm not done refactoring this into generic, decoupled bits that can integrate nicely into any keyboard. Certainly the way forward is not to just copy-paste my code into core just because you don't like referencing |
@Wilba6582 I apologize for that. It was not my intent to upset you. In retrospect, I should have contacted you before opening the PR, first. Though, I wasn't really sure of how to contact you to do so, let alone what is the best method of doing so. As for the dynamic keymap code, I know it's something that Jack has wanted to do for a while now. there is a branch that has all of the ifdefs removed from the keycode list, explicitly to that end. And some other stuff. But yes, you are the one that has put most (if not all) of the work into this, thus far. However, it's absolutely understandable that nobody wanted to use it until there was actually a GUI frontend, .... because those can be very complicated. But that people are rushing towards it is a good thing. And that rush is big part of what inspired this. I can't speak for anyone else, but ... I really do want to see the RAW HID, dynamic keymaps and VIA well supported. I think it's a fantastic thing, in general. And I would like to see as much support for it added, as possible. So I apologize that I got overzealous in wanting to bring that support to the core code. Though, referencing keyboard code is a very bad idea. It means that any changes to your code can break who knows how many other keyboards. And while that's not your responsibility, it brings up some of the exact same issues with the split keyboard code. However, I do understand your feelings on this subject. And if there is anything that I can do to help, I would love to be able to do so. Even if that is to do nothing, at all. |
I'm not upset.
??? This makes no sense. You can @Wilba6582 in an issue. You can message me on Discord. I'm in the QMK Discord server, even. My WT-xx keyboard implementations link to my website http://wilba.tech which has contact links at the bottom. If complete strangers who wants to commission me for PCBs can contact me, I am sure you could have found a way.
The only keyboards that reference my code in And what's up the top of that guide?
You seem to have ignored the last point of my last reply, it is still in There is some code in @olivia and I have plans for VIA. We are happy that people like it and want to use it themselves. We also have the long term goal of supporting other PCBs. This will involve refactoring and QMK core changes, which need to be well thought out, not rushed copy paste jobs, or multiple iterations, that would break keyboards just like you said my code would if I changed it in For example, what you are doing in #3113 would be a useful prerequisite to dealing with keyboard level code requiring specific initialization as well as VIA initialization. Ideally, I also need to refactor how the message handling works, splitting it apart, so common code in Core can handle the generic stuff like dynamic keymaps, and other messages are passed to the keyboard level so they can be handled differently per keyboard. Even then, it won't be good enough for every use case, for example, split keyboard implementations in which the master could be swapped will need to route keymap changes on the master to the slave, in case that slave becomes the master. That requires the keyboard level to intercept the keymap change messages. The ability to override existing QMK functionality and do specific things at the keyboard level is what makes QMK great. I want to continue this paradigm with the VIA support code, not lock everything up in Core.
That's nice, but why are people all working on things like this without even mentioning it to me or @olivia? I have had absolutely no contact from the QMK contributors about VIA or the QMK code I wrote to support it, at best, some replies to comments I make in PRs about things that might affect VIA working with the latest revision of QMK. I wrote the original implementation of dynamic keymaps and added the raw HID support years ago, pushed them into Core so that others could use them in whatever way they wanted. To be frank, QMK contributors have had their chance to develop their own system of host-configurable run-time keymap configuration. Now that my implementation is what VIA uses, it seems like QMK contributors think it's fair game to assimilate it into Core without even discussing it with me, or even considering that the protocol I wrote is incomplete, and changes to it would require matching changes to VIA, which is beyond the scope of QMK and requires discussion with @olivia and myself. Yes, this is criticism of the communication or lack thereof leading up to this point, and the associated perception we have that there's been a lack of respect. I think it would make sense to raise an issue for conversing on the topic. I welcome ideas and feedback on how to approach this refactoring work. |
Some of that may be projection, and grasping at straws, because this has me very stressed out, because your contributions have been awesome, if not downright amazing. And I don't want to be the reason that you pull away from QMK FIrmware. However, I think it's good that a lot of this is being talked about, though the circumstance is far from ideal (about as far from it, as one can get). To be honest, I think that we need more discussion. Or at least somewhere where a game plan (or whatever) is documented, so that we all know what is going on, what's "in progress", what needs to happen, etc. And by "we", I mean QMK people, VIA people, etc. If only to point people to something for why something is or isn't happening. For instance, #3113 being very useful for VIA support. Or ... the entire issue with the Iris (such as #4893). I also think that "we" (again, QMK and VIA people) need a place to talk about development and direction of both projects. Be it public, semi-public, or private. And not the #firmware channel on discord. Maybe a #via channel. or even a different server. And yes, I know that I screwed up here. And I want to do whatever I can to make things right. |
Description
This Pull Request decouples the VIA support config from the Wilba Tech and Zeal keyboards, and moves it into the quantum code, as a core feature.
This introduces a
VIA_SUPPORT_ENABLE
makefile setting that enables both RAW HID, and Dynamic Keymap support, to facilitate the ability to easy enable support.Additionally, this converts the kb functions (matrix_int, and process_record) into core functions, so they no longer replace existing functions. They have been moved to
via_init
andprocess_via
respectively.Also, this changes the Travis CI build scripts to compile with VIA support, and to copy those keymaps to qmk.fm
Types of changes
Issues Fixed or Closed by this PR
Checklist: