Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[Stefan Berzl]

Hello!

>> >From f9bb82e400effd3aea37b2be710add9e2bb832da Mon Sep 17 00:00:00 2001
>> From: Stefan Berzl <stefanberzl@gmail.com>
>> Date: Fri, 11 Mar 2022 04:04:30 +0100
>> Subject: [PATCH] hid: Add support for the xp-pen deco mini7 tablet
>>
>> ...
> 
> Stefan,
> 
> sorry for not responding earlier, but this patch somehow fell in between 
> cracks. Please for your further submissions do not forget to at least CC 
> some of the relevant malinglists as well.

Will do... I am still new to the process and tend to forget things, but 
I'll do my best.

> Quite a lot has changed in uclogic driver since then, as José has been 
> pushing quite a lot of code there from DIGImend project ... could you 
> please update your patch on top of current codebase and resend it?
> 
> Thanks,
> 

I think you'll be pleased to find that José's work is basically a 
superset of mine, therefore eliminating the need for my patch 
altogether. When running his newest deco l code, the deco mini 7 is 
fully supported as well. I should have given a tested-by or something, 
but since I am still new to this, I was kinda hesitant with the big 
changes introduced by the newest digimend work. As you know, there is no 
sort of registry to differentiate devices by ability, so one or two
useless evdev devices are generated for hardware the tablet may or may 
not have, like "Touch Ring" or "Touch Strip". I thought that this might 
cause confusion and would have to be amended.

There is however one caveat that seems to be unique to the mini7, which 
is the ack packet that is sent when switching to the vendor defined 
usage. It doesn't do much though, as currently it gets interpreted as a 
pen report and since it doesn't have useful values, causes the cursor to 
go to the top left screen position. Since the ack packet is only sent 
once, it ought to be of little consequence.

I would of course fix this, but I don't really know what's the preferred 
way. One can obviously simply set up an urb to catch this, but it would 
have to be a special corner case for the mini 7, as José assures me that 
none of his tablets display similar behavior. Is this acceptable?
José already had a look at some firmware device descriptor string that 
reports the number of buttons and what not, but as far as I know, it 
doesn't say anything about ack packets (right José? Does it say 
anything about touch strips or similar?).

Everything said, I think this can be closed
https://patchwork.kernel.org/project/linux-input/patch/b401e453-9c66-15e3-1a1d-21f33b7a64e8@gmail.com/

Also I made this patch, which is very trivial and simplifies the hot path
https://patchwork.kernel.org/project/linux-input/patch/17153eb3-0eb9-cc05-4b65-9c0f4e8d3c90@gmail.com/

Thanks for your work on the linux input subsystem!


Kind Regards
Stefan Berzl


PS: notice how I put the right CCs and stuff :)

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[José Expósito]

Hi Stefan,

On Wed, Jun 22, 2022 at 11:31:05PM +0200, Stefan Berzl wrote:
> Hello!
> 
> >> >From f9bb82e400effd3aea37b2be710add9e2bb832da Mon Sep 17 00:00:00 2001
> >> From: Stefan Berzl <stefanberzl@gmail.com>
> >> Date: Fri, 11 Mar 2022 04:04:30 +0100
> >> Subject: [PATCH] hid: Add support for the xp-pen deco mini7 tablet
> >>
> >> ...
> > 
> > Stefan,
> > 
> > sorry for not responding earlier, but this patch somehow fell in between 
> > cracks. Please for your further submissions do not forget to at least CC 
> > some of the relevant malinglists as well.
> 
> Will do... I am still new to the process and tend to forget things, but 
> I'll do my best.
> 
> > Quite a lot has changed in uclogic driver since then, as José has been 
> > pushing quite a lot of code there from DIGImend project ... could you 
> > please update your patch on top of current codebase and resend it?
> > 
> > Thanks,
> > 
> 
> I think you'll be pleased to find that José's work is basically a 
> superset of mine, therefore eliminating the need for my patch 
> altogether. When running his newest deco l code, the deco mini 7 is 
> fully supported as well. I should have given a tested-by or something, 
> but since I am still new to this, I was kinda hesitant with the big 
> changes introduced by the newest digimend work. As you know, there is no 
> sort of registry to differentiate devices by ability, so one or two
> useless evdev devices are generated for hardware the tablet may or may 
> not have, like "Touch Ring" or "Touch Strip". I thought that this might 
> cause confusion and would have to be amended.

Useless event nodes should not be created. At the moment, the driver
disables all interfaces that are not used and, after switching to "raw"
mode, it should only create a node for each valid device.

At the moment, there are only HID descriptors for the frame and the pen
so, if your tablet is creating a touch ring device, something is not
working as expected.

Running "sudo libinput record" should display only the frame and the
pen. Does it show something different in your case?

	$ sudo libinput record
	[...]
	/dev/input/event21:	Hanvon Ugee Technology Co.,Ltd Deco L
	/dev/input/event22:	Hanvon Ugee Technology Co.,Ltd Deco L Pad

 
> There is however one caveat that seems to be unique to the mini7, which 
> is the ack packet that is sent when switching to the vendor defined 
> usage. It doesn't do much though, as currently it gets interpreted as a 
> pen report and since it doesn't have useful values, causes the cursor to 
> go to the top left screen position. Since the ack packet is only sent 
> once, it ought to be of little consequence.
> 
> I would of course fix this, but I don't really know what's the preferred 
> way. One can obviously simply set up an urb to catch this, but it would 
> have to be a special corner case for the mini 7, as José assures me that 
> none of his tablets display similar behavior. Is this acceptable?

My tablets also send an ACK packet, but in my case it does not have any
visible effects. Maybe it is related to the DE environment used. I
tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
ACK is ignored... But catching it is fine, we can include the code you
suggest.

> José already had a look at some firmware device descriptor string that 
> reports the number of buttons and what not, but as far as I know, it 
> doesn't say anything about ack packets (right José? Does it say 
> anything about touch strips or similar?).

In the devices I tested, the ACK packet is always present, so it should
be fine to catch it. I'll test your patch in all the devices I own to
be safe.

Best wishes,
Jose

> Everything said, I think this can be closed
> https://patchwork.kernel.org/project/linux-input/patch/b401e453-9c66-15e3-1a1d-21f33b7a64e8@gmail.com/
> 
> Also I made this patch, which is very trivial and simplifies the hot path
> https://patchwork.kernel.org/project/linux-input/patch/17153eb3-0eb9-cc05-4b65-9c0f4e8d3c90@gmail.com/
> 
> Thanks for your work on the linux input subsystem!
> 
> 
> Kind Regards
> Stefan Berzl
> 
> 
> PS: notice how I put the right CCs and stuff :)

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[Nikolai Kondrashov]

On 6/23/22 20:51, José Expósito wrote:
>> I would of course fix this, but I don't really know what's the preferred
>> way. One can obviously simply set up an urb to catch this, but it would
>> have to be a special corner case for the mini 7, as José assures me that
>> none of his tablets display similar behavior. Is this acceptable?
> 
> My tablets also send an ACK packet, but in my case it does not have any
> visible effects. Maybe it is related to the DE environment used. I
> tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
> ACK is ignored... But catching it is fine, we can include the code you
> suggest.
> 
>> José already had a look at some firmware device descriptor string that
>> reports the number of buttons and what not, but as far as I know, it
>> doesn't say anything about ack packets (right José? Does it say
>> anything about touch strips or similar?).
> 
> In the devices I tested, the ACK packet is always present, so it should
> be fine to catch it. I'll test your patch in all the devices I own to
> be safe.

I think it's OK to just ignore the first packet for these devices, even if the 
ACK packet is not sent for some of them. Even with the report rate of 20 years 
ago nobody would've noticed if you dropped one packet.

Nick

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[Stefan Berzl]

Hi!

On 23/06/2022 20:01, Nikolai Kondrashov wrote:
> On 6/23/22 20:51, José Expósito wrote:
>>> I would of course fix this, but I don't really know what's the preferred
>>> way. One can obviously simply set up an urb to catch this, but it would
>>> have to be a special corner case for the mini 7, as José assures me that
>>> none of his tablets display similar behavior. Is this acceptable?
>>
>> My tablets also send an ACK packet, but in my case it does not have any
>> visible effects. Maybe it is related to the DE environment used. I
>> tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
>> ACK is ignored... But catching it is fine, we can include the code you
>> suggest.
>>
>>> José already had a look at some firmware device descriptor string that
>>> reports the number of buttons and what not, but as far as I know, it
>>> doesn't say anything about ack packets (right José? Does it say
>>> anything about touch strips or similar?).
>>
>> In the devices I tested, the ACK packet is always present, so it should
>> be fine to catch it. I'll test your patch in all the devices I own to
>> be safe.
> 
> I think it's OK to just ignore the first packet for these devices, even if the ACK packet is not sent for some of them. Even with the report rate of 20 years ago nobody would've noticed if you dropped one packet.
> 
> Nick

Sounds good indeed. Does it also work if the user presses a button first?
The way I get it, we would only receive the button up event then, not the
button down?

Faithfully
Stefan Berzl

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[Nikolai Kondrashov]

On 6/24/22 01:46, Stefan Berzl wrote:
> Hi!
> 
> On 23/06/2022 20:01, Nikolai Kondrashov wrote:
>> On 6/23/22 20:51, José Expósito wrote:
>>>> I would of course fix this, but I don't really know what's the preferred
>>>> way. One can obviously simply set up an urb to catch this, but it would
>>>> have to be a special corner case for the mini 7, as José assures me that
>>>> none of his tablets display similar behavior. Is this acceptable?
>>>
>>> My tablets also send an ACK packet, but in my case it does not have any
>>> visible effects. Maybe it is related to the DE environment used. I
>>> tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
>>> ACK is ignored... But catching it is fine, we can include the code you
>>> suggest.
>>>
>>>> José already had a look at some firmware device descriptor string that
>>>> reports the number of buttons and what not, but as far as I know, it
>>>> doesn't say anything about ack packets (right José? Does it say
>>>> anything about touch strips or similar?).
>>>
>>> In the devices I tested, the ACK packet is always present, so it should
>>> be fine to catch it. I'll test your patch in all the devices I own to
>>> be safe.
>>
>> I think it's OK to just ignore the first packet for these devices, even if the ACK packet is not sent for some of them. Even with the report rate of 20 years ago nobody would've noticed if you dropped one packet.
>>
>> Nick
> 
> Sounds good indeed. Does it also work if the user presses a button first?
> The way I get it, we would only receive the button up event then, not the
> button down?

It's basically humanly impossible to register a pen's button press without 
first producing some movement reports. You have to hold the pen within range 
for it to register, and it's hard to hold it still enough within range to not 
produce movement reports, while also trying to press a button. I think we're 
safe here.

If there are any buttons on the frame, then it's likely possible to 
distinguish their reports from the pen ones by looking at the report contents 
before discarding.

Nick

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[Nikolai Kondrashov]

On 6/24/22 01:46, Stefan Berzl wrote:
> Hi!
> 
> On 23/06/2022 20:01, Nikolai Kondrashov wrote:
>> On 6/23/22 20:51, José Expósito wrote:
>>>> I would of course fix this, but I don't really know what's the preferred
>>>> way. One can obviously simply set up an urb to catch this, but it would
>>>> have to be a special corner case for the mini 7, as José assures me that
>>>> none of his tablets display similar behavior. Is this acceptable?
>>>
>>> My tablets also send an ACK packet, but in my case it does not have any
>>> visible effects. Maybe it is related to the DE environment used. I
>>> tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
>>> ACK is ignored... But catching it is fine, we can include the code you
>>> suggest.
>>>
>>>> José already had a look at some firmware device descriptor string that
>>>> reports the number of buttons and what not, but as far as I know, it
>>>> doesn't say anything about ack packets (right José? Does it say
>>>> anything about touch strips or similar?).
>>>
>>> In the devices I tested, the ACK packet is always present, so it should
>>> be fine to catch it. I'll test your patch in all the devices I own to
>>> be safe.
>>
>> I think it's OK to just ignore the first packet for these devices, even if the ACK packet is not sent for some of them. Even with the report rate of 20 years ago nobody would've noticed if you dropped one packet.
>>
>> Nick
> 
> Sounds good indeed. Does it also work if the user presses a button first?
> The way I get it, we would only receive the button up event then, not the
> button down?

It's basically humanly impossible to register a pen's button press without 
first producing some movement reports. You have to hold the pen within range 
for it to register, and it's hard to hold it still enough within range to not 
produce movement reports, while also trying to press a button. I think we're 
safe here.

If there are any buttons on the frame, then it's likely possible to 
distinguish their reports from the pen ones by looking at the report contents 
before discarding.

Nick

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[José Expósito]

Hi Nickolai,

Nikolai Kondrashov wrote:
> I think it's OK to just ignore the first packet for these devices, even if the
> ACK packet is not sent for some of them. Even with the report rate of 20 years
> ago nobody would've noticed if you dropped one packet.

A bit more of context about this initial packet:

These XP-PEN devices need to receive a packet of data to be fully functional.
The driver sends it:

	02 b0 04 00 00 00 00 00 00 00

And in response to the activation packet, the tablet sends an ACK:

	02 b1 04 00 00 00 00 00 00 00

In my case the packet is ignored but on Stepfan's tablet, this packet sends to
mouse pointer to the 0,0 coordinates.

Looking at the data he added to his last email, his tablet ACK has 2 extra
bytes, making it match the size of a pen report.
Because the ACK packet starts with 02 it looks like it is  interpreted as a pen
report with all values set to 0, including X and Y.

We are not worried about a packet being dropped, we would like to filter the
ACK so it does not get handled as a pen report. This should allow to
avoid sending the pointer to 0,0 on device connection.

It is not a super anoying bug, but it'll be nice if we could avoid it.

Best wishes,
Jose

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[Stefan Berzl]

Hello there!

On 23/06/2022 19:51, José Expósito wrote:
> Hi Stefan,
> 
> On Wed, Jun 22, 2022 at 11:31:05PM +0200, Stefan Berzl wrote:
>> Hello!
>>
>>>> >From f9bb82e400effd3aea37b2be710add9e2bb832da Mon Sep 17 00:00:00 2001
>>>> From: Stefan Berzl <stefanberzl@gmail.com>
>>>> Date: Fri, 11 Mar 2022 04:04:30 +0100
>>>> Subject: [PATCH] hid: Add support for the xp-pen deco mini7 tablet
>>>>
>>>> ...
>>>
>>> Stefan,
>>>
>>> sorry for not responding earlier, but this patch somehow fell in between 
>>> cracks. Please for your further submissions do not forget to at least CC 
>>> some of the relevant malinglists as well.
>>
>> Will do... I am still new to the process and tend to forget things, but 
>> I'll do my best.
>>
>>> Quite a lot has changed in uclogic driver since then, as José has been 
>>> pushing quite a lot of code there from DIGImend project ... could you 
>>> please update your patch on top of current codebase and resend it?
>>>
>>> Thanks,
>>>
>>
>> I think you'll be pleased to find that José's work is basically a 
>> superset of mine, therefore eliminating the need for my patch 
>> altogether. When running his newest deco l code, the deco mini 7 is 
>> fully supported as well. I should have given a tested-by or something, 
>> but since I am still new to this, I was kinda hesitant with the big 
>> changes introduced by the newest digimend work. As you know, there is no 
>> sort of registry to differentiate devices by ability, so one or two
>> useless evdev devices are generated for hardware the tablet may or may 
>> not have, like "Touch Ring" or "Touch Strip". I thought that this might 
>> cause confusion and would have to be amended.
> 
> Useless event nodes should not be created. At the moment, the driver
> disables all interfaces that are not used and, after switching to "raw"
> mode, it should only create a node for each valid device.
> 
> At the moment, there are only HID descriptors for the frame and the pen
> so, if your tablet is creating a touch ring device, something is not
> working as expected.
> 
> Running "sudo libinput record" should display only the frame and the
> pen. Does it show something different in your case?
> 
> 	$ sudo libinput record
> 	[...]
> 	/dev/input/event21:	Hanvon Ugee Technology Co.,Ltd Deco L
> 	/dev/input/event22:	Hanvon Ugee Technology Co.,Ltd Deco L Pad

This is certainly true for the newer xppen devices we are working on. 
However, while waiting for the xppen stuff to gain support, I bought a
tablet  that's already supported, the Gaomon S620. Executing libinput 
record or any other command that lists the devices, like evemu-describe, 
gives:

/dev/input/event15: GAOMON Gaomon Tablet
/dev/input/event16: GAOMON Gaomon Tablet Pad
/dev/input/event17: GAOMON Gaomon Tablet Touch Strip
/dev/input/event18: GAOMON Gaomon Tablet Dial

>> There is however one caveat that seems to be unique to the mini7, which 
>> is the ack packet that is sent when switching to the vendor defined 
>> usage. It doesn't do much though, as currently it gets interpreted as a 
>> pen report and since it doesn't have useful values, causes the cursor to 
>> go to the top left screen position. Since the ack packet is only sent 
>> once, it ought to be of little consequence.
>>
>> I would of course fix this, but I don't really know what's the preferred 
>> way. One can obviously simply set up an urb to catch this, but it would 
>> have to be a special corner case for the mini 7, as José assures me that 
>> none of his tablets display similar behavior. Is this acceptable?
> 
> My tablets also send an ACK packet, but in my case it does not have any
> visible effects. Maybe it is related to the DE environment used. I
> tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
> ACK is ignored... But catching it is fine, we can include the code you
> suggest.

Can the contents maybe differ?

This is the ack the mini 7 gives me:
02 b1 04 00 00 00 00 00 00 00 00 00

While this is a button:
02 f0 00 00 00 00 00 00 00 00 00 00

And here we have pen movement:
02 a1 59 23 ef 32 b8 0e 00 00 00 00

>> José already had a look at some firmware device descriptor string that 
>> reports the number of buttons and what not, but as far as I know, it 
>> doesn't say anything about ack packets (right José? Does it say 
>> anything about touch strips or similar?).
> 
> In the devices I tested, the ACK packet is always present, so it should
> be fine to catch it. I'll test your patch in all the devices I own to
> be safe.
> 
> Best wishes,
> Jose
> 

Yours truly
Stefan Berzl

Re: [PATCH] hid: Add support for the xp-pen deco mini7 tablet

[José Expósito]

On Fri, Jun 24, 2022 at 12:24:09AM +0200, Stefan Berzl wrote:
> On 23/06/2022 19:51, José Expósito wrote:
> > At the moment, there are only HID descriptors for the frame and the pen
> > so, if your tablet is creating a touch ring device, something is not
> > working as expected.
> > 
> > Running "sudo libinput record" should display only the frame and the
> > pen. Does it show something different in your case?
> > 
> > 	$ sudo libinput record
> > 	[...]
> > 	/dev/input/event21:	Hanvon Ugee Technology Co.,Ltd Deco L
> > 	/dev/input/event22:	Hanvon Ugee Technology Co.,Ltd Deco L Pad
> 
> This is certainly true for the newer xppen devices we are working on. 
> However, while waiting for the xppen stuff to gain support, I bought a
> tablet  that's already supported, the Gaomon S620. Executing libinput 
> record or any other command that lists the devices, like evemu-describe, 
> gives:
> 
> /dev/input/event15: GAOMON Gaomon Tablet
> /dev/input/event16: GAOMON Gaomon Tablet Pad
> /dev/input/event17: GAOMON Gaomon Tablet Touch Strip
> /dev/input/event18: GAOMON Gaomon Tablet Dial

Ah OK, I though you were talking about the XP-PEN device. I don't know why
those extra event nodes are created, sorry.
 
> >> There is however one caveat that seems to be unique to the mini7, which 
> >> is the ack packet that is sent when switching to the vendor defined 
> >> usage. It doesn't do much though, as currently it gets interpreted as a 
> >> pen report and since it doesn't have useful values, causes the cursor to 
> >> go to the top left screen position. Since the ack packet is only sent 
> >> once, it ought to be of little consequence.
> >>
> >> I would of course fix this, but I don't really know what's the preferred 
> >> way. One can obviously simply set up an urb to catch this, but it would 
> >> have to be a special corner case for the mini 7, as José assures me that 
> >> none of his tablets display similar behavior. Is this acceptable?
> > 
> > My tablets also send an ACK packet, but in my case it does not have any
> > visible effects. Maybe it is related to the DE environment used. I
> > tested it on elementary OS (Ubuntu) and Fedora 36, in both cases the
> > ACK is ignored... But catching it is fine, we can include the code you
> > suggest.
> 
> Can the contents maybe differ?
> 
> This is the ack the mini 7 gives me:
> 02 b1 04 00 00 00 00 00 00 00 00 00
> 
> While this is a button:
> 02 f0 00 00 00 00 00 00 00 00 00 00
> 
> And here we have pen movement:
> 02 a1 59 23 ef 32 b8 0e 00 00 00 00

Yes, the contents are different. My ACK does not contain the last 2 bytes.
As mentioned in my previous email, I think that because the ACK of your
tablet matches the size of a pen report and starts with 02 it is handled as
a pen report.

Jose