[0.6.x] Add Wheel Support #3497
Conversation
|
Turns out i'm gonna need to provide at least empty specs to pass Unit Tests anyway |
|
Will need some F in the chat for either I or James having to deal with a conflict if the attribute overhaul get merged with 0.6.x |
|
Damn you james, owned by removed '0' decimals of specs |
|
Originally put the Intuos 5 wheel parsing in the Intuos4 file instead (cause i didn't look at the file name at some point). But anyway, added support for the Intuos 5 since i had one on hand and other tried it a while back (using my 0.5.x plugin). |
|
This could use a review whenever possible. |
|
This will need a rebase when #3522 is merged (and will likely happen before this PR is merged, for logistics reasons). You no longer need to define null variables in configurations after that PR. |
|
I did see that null handling was set to ignore (which i doubt does much for deserializing, mostly serializing). |
|
Just noticed but is #3522 got merged, but it doesn't seem like tests failed, shouldn't need to apply config changes, do i? |
|
You can rebase out the config changes when the PR is backported. CC: @X9VoiD |
|
Right, they need to be backported first, that PR targeted master |
gonX
left a comment
There was a problem hiding this comment.
generally LGTM, will be looking at merging this for 0.6.7 (the release following next release)
gonX
left a comment
There was a problem hiding this comment.
Code LGTM, need further testing after 0.6.6 release.
Kuuuube
left a comment
There was a problem hiding this comment.
This looks good to me and it works great in my testing. Just needs conflicts resolved.
I assume this wouldn't support tablets with two wheels yet? Or they would have to act as one wheel? The wacom PTK-670 and PTK-870 for example.
I'm okay merging as is but something that would be needed in the future.
|
It used to be an array, was told to not make it an array. |
I think I forgot to respond to this, but I prefer having wheel buttons as a separate property for parsers to set. Including it in the normal aux buttons array is confusing for a varying amount of reasons, but primarily the current assumption a lot of places in the code and GUI just expects buttons to be a part of the normal pad buttons, and a lot of parsers (except for the earliest ones) have the wheel button marked as a commented bit of code. Having this assumption simplifies implementation of button passthrough as seen in #3074 and #4004 |
I looked through the issue, and couldn't find this conversation. I think it would be best to have support for this now that the new Wacom models have 2 wheels, as not accounting for this now may require some configuration migration code in the future. |
…tDriver into 0.6.x-wheel-support
I lied.
I got triggered seeing multiple basically dead PRs targeting master, which is not going to be ready anytime soon, so i took the opportunity to backport #2669.
This should then allow Parsing of the Wheel on PTK 440 to 840 as well as the Huion HS610.
Let me know if you want me to also include the specs for known supported tablets inside configs.
(These maxes can also be acquired via calibration, which i do in my 0.5.x Wheel Addon plugin)
One more question to be asked : PTK x40 tablets have a wheel button, and a concern was made here
Before my PR, the wheel button was already parser in the Intuos4Report, would this be the time to move it out to the
IAbsoluteWheelReportor a newIWheelSupport(that would be used for relative & absolute)