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

Add support for Wacom Movink #673

Merged
merged 11 commits into from
May 2, 2024

Conversation

flying-elephant
Copy link
Contributor

Those commits are ready for Wacom Movink announced yesterday.

This is a complete new layout for Wacom Movink

Signed-off-by: Tatsunosuke Tobita <[email protected]>
This is a new tablet file for Wacom Movink

Signed-off-by: Tatsunosuke Tobita <[email protected]>
This changes support Wacom One pen on Wacom Movink.

Signed-off-by: Tatsunosuke Tobita <[email protected]>
Copy link
Member

@whot whot left a comment

Choose a reason for hiding this comment

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

Thanks for the quick PR - a few questions but otherwise this is almost ready.

data/movink.tablet Outdated Show resolved Hide resolved
Comment on lines 41 to 42
Left=A;C
Right=B;C
Copy link
Member

Choose a reason for hiding this comment

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

We can't have one button in two locations but based on the drawing this should be Top=C, right?

Also, after looking at some marketing pictures: are those real buttons or emulated by the driver (in userspace)? or rather: do we get those events under linux?

Copy link
Contributor Author

@flying-elephant flying-elephant Apr 25, 2024

Choose a reason for hiding this comment

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

Top=C, right?

I'll work on this, thanks!
There are two buttons on each sides, so I thought they should be the sides.
"Tpo=C" makes much sense.

Yes, one of the physical buttons can be assigned for an express key, but it also can be other functionalities than the express key; one of them is "Touch on/off".

Copy link
Member

Choose a reason for hiding this comment

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

can be assigned for an express key,

so these are software/firmware controlled buttons? that's going to be ... interesting, libwacom doesn't have a good way to express these for now.

Might be best to add a comment in the file here and then figure out how to really handle this later when we have the consumers (e.g. mutter + gnome-control-center) actually wanting deal with these buttons.

data/movink.tablet Show resolved Hide resolved
Co-authored-by: Peter Hutterer <[email protected]>
@whot whot added the sysinfo needed Used to indicate sysinfo is missing from the PR label Apr 29, 2024
Copy link

Hi, there. I'm a bot and have been asked to have a look at this.

Please run the sysinfo script from https://github.com/linuxwacom/wacom-hid-descriptors and
file a new issue in that repo with the resulting sysinfo tarball. The README in that repo
has more information and instructions.

The .tablet files in this PR should then link to the sysinfo in this form:

# sysinfo.abCdE1234.tar.gz
# https://github.com/linuxwacom/wacom-hid-descriptors/issues/1234

A git grep sysinfo will show how this is currently used in existing data files.

See also https://github.com/linuxwacom/libwacom/wiki/Adding-a-new-device


This is an automated comment created by a bot. Responding to the bot or mentioning it won't have any effect.

data/libwacom.stylus Show resolved Hide resolved
data/movink.tablet Outdated Show resolved Hide resolved
Comment on lines 41 to 42
Left=A;C
Right=B;C
Copy link
Member

Choose a reason for hiding this comment

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

can be assigned for an express key,

so these are software/firmware controlled buttons? that's going to be ... interesting, libwacom doesn't have a good way to express these for now.

Might be best to add a comment in the file here and then figure out how to really handle this later when we have the consumers (e.g. mutter + gnome-control-center) actually wanting deal with these buttons.

Copy link
Member

@whot whot left a comment

Choose a reason for hiding this comment

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

Thanks for the update. CI still fails so at least two changes are still needed - please make sure to run meson test -C builddir locally, that'll run (most of) the test suite locally so it'll speed up finding the various nitpicks my human eyes won't see :)

@whot whot changed the title New feature 042024 Add support for Wacom Movink May 1, 2024
Buttons line added for the UD pen eraser, 0x1000A
Modified to add information of sysinfo and required changed by  the review
@flying-elephant
Copy link
Contributor Author

I modified the codes and updated them.
However, I got the attached log by running the suggestion, "meson test -C".
Do you know what is wrong exactly?
The log looks like the lines which describe about the eraser button, but I'm not sure what I should do.
"UD pen" could have an eraser button, so it has to stick with it somehow.
testlog.txt

Copy link
Member

@whot whot left a comment

Choose a reason for hiding this comment

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

I think the eraser parsing fails, fixing that should fix the CI

data/libwacom.stylus Outdated Show resolved Hide resolved
data/libwacom.stylus Outdated Show resolved Hide resolved
data/movink.tablet Outdated Show resolved Hide resolved
data/libwacom.stylus Outdated Show resolved Hide resolved
data/movink.tablet Show resolved Hide resolved
data/movink.tablet Show resolved Hide resolved
@whot whot merged commit 03a9a24 into linuxwacom:master May 2, 2024
13 checks passed
@whot
Copy link
Member

whot commented May 2, 2024

All good now, let's get this merged! Thanks heaps for the effort

@whot whot removed the sysinfo needed Used to indicate sysinfo is missing from the PR label May 2, 2024
Copy link

github-actions bot commented May 2, 2024

Hi, there. I'm a bot and have been asked to have a look at this.

Thanks for providing the sysinfo. Someone will be along shortly to review the PR.


This is an automated comment created by a bot. Responding to the bot or mentioning it won't have any effect.

whot pushed a commit to whot/libwacom that referenced this pull request Aug 2, 2024
Signed-off-by: Tatsunosuke Tobita <[email protected]>
(cherry picked from commit 03a9a24)
whot pushed a commit that referenced this pull request Aug 5, 2024
Signed-off-by: Tatsunosuke Tobita <[email protected]>
(cherry picked from commit 03a9a24)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants