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

[Benjamin Tissoires <tissoire@cena.fr>]

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@free.fr>  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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html