Re: [PATCH 2/3] HID: suppress the second input for the Apple Magic Mouse.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Mickael, sorry for taking so long to respond.

Here is what I get from my very poor experience in hid hacking.

Le 09/03/2010 14:06, Michael Poole a écrit :
Rantanplan<rantanplanlechien@xxxxxxx>  writes:

[snip]
Then I also saw that contrary to the driver I saw before (hid-stantum
and hid-ntrig) the hid-magicmouse uses raw event instead of higher
level events. I thought it was not very clear (at least for me as I do
not speak very well bit fields) and I started to write a work-around.
That was mostly a code compactness issue: It was not clear to me how to
generate an appropriate set of field descriptors for the multi-touch
records, but it looked like it would not be easy.  It is still not clear
to me -- from hid-ntrig, hid-stantum, hid-core and your patch -- how a
variable-length report from the device gets translated to multiple
events for the touch fields.  (The key words there are "to me": I am
probably overlooking something.)

The previous test I made for the magic mouse (and that was working, though not the way you want I think) was the following: injecting the most of the data I may receive.
To sum up, in our case:
- I injected in the field descriptor the values of the relatives axes (as I made in the patch I nearly submitted yesterday)
- Then I injected the first multitouch data by incrementing the collection by 1
- Then I repeated the injection of the other multitouch data, still increasing the collection by one (to have 5 mmultitouch collections at the end)

When it came to sending the events, if the data do not contain all the collections (only one touch for instance), hid calls all the events declared in the report with 0 as values.
It is working, as it sends in the order relatives axes and then multitouch, but is not very satisfactory as it implies some head over as functions are called when they should not.

I don't know if the protocol used by the magic mouse is standard, in a sense that the stantum always gives all the data it registers.


The other notable part of using raw events is that the middle-button
emulation requires watching button state in both the standard mouse
report and the multi-touch report.  Matching button-down and -up events
may be sent in different report types.

If you keep a structure to store the events, it should not be a problem.
Moreover, when using hid to process event, you have only one function for all the report types, thus, you won't miss any button-up -down.


The last point that was worrying me was the line (after the call of
hid_register_report):
'report->size = 6;'
The driver registered a new report, tells it has an arbitrary size (or
maybe not?) whereas it has no field initialized.
Six bytes is the minimum length of the touch report -- although now that
I look at it, report->size was meant to count bits.  (I am not sure why
the mouse sometimes decides to send a report with zero touches rather
than the standard mouse report, but it does.)

So I wrote first a big patch to avoid all the 'problems' listed above.
I did not send it immediately and I was a little bit overload at work.
However, when I saw that the driver had been included in the next
branch of the kernel, I took time to work on the patch again. But, I
only tried to tackle the problem of the second input device.

To sum up, the advantages of my patch are, I think, the following
(though I do not want to seem to be competing with Micheal):
- It's documenting the data the magic mouse sends as the mouse does
not send the descriptor.
- It will allow another patch to avoid the use of the raw events
instead of high level events (don't know if it is really a benefit)

As Micheal said there are huge pitfalls:
- It heavy regards to its aim
- I insert fields into the descriptor that I use them later with a
slightly change in the meaning (some keywords are not well choose and
the whole meaning is split between the registering of the fields and
the mapping)
- coding style (not really a problem though)
- raw events have the benefits of giving the number of touches into
the data size, something we don't have with high level events only.


Maybe a solution would just be to add a comment documenting the bit
fields in the data? (as now, most of the problems had been solved by
the last patch of Michael)
To what end?  User space should see the same data from either approach.
In the kernel, the decoding of the multi-touch reports is in five lines
near the top of magicmouse_emit_touch() and five to seven lines in
magicmouse_raw_event()'s case TOUCH_REPORT_ID.  There are ten known
fields (X, Y, buttons, and six touch fields) in the data, so it seems
hard to do better than ten (sane) lines.  Using some other description
may be easier to find, but I'm not sure it will be much clearer.

Michael
I just intended a comment above the functions, written with a sort of ascii art, to explicit the definition of the data. But maybe I'm just not enough skilled in bit-fields.

Benjamin

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux