[haiku-bugs] Re: [Haiku] #7354: usb_hid tablet support

  • From: "mmlr" <trac@xxxxxxxxxxxx>
  • Date: Mon, 14 Mar 2011 18:46:30 -0000

#7354: usb_hid tablet support
---------------------------+-----------------------
   Reporter:  lt_henry     |      Owner:  mmlr
       Type:  enhancement  |     Status:  new
   Priority:  normal       |  Milestone:  R1
  Component:  Drivers/USB  |    Version:  R1/alpha2
 Resolution:               |   Keywords:  usb_hid
 Blocked By:               |   Blocking:
Has a Patch:  1            |   Platform:  All
---------------------------+-----------------------

Comment (by mmlr):

 Replying to [comment:5 caz_haiku]:
 > In HIDParser.cpp when an ITEM_TYPE_LOCAL is detected and then the
 item->tag is an ITEM_TAG_LOCAL_USAGE should the code be as follows,
 > :)[[BR]]
 > value->u.s.usage_page = globalState.usage_page;[[BR]]
 > value->u.s.usage_id = data;
 >
 > The code as it is now is[[BR]]
 > value->u.extended = data;

 No this is not correct. The item can define a usage id only, or an
 extended usage (combined usage page and usage id). The is_extended flag
 holds that information and is initialized based on the item size. The
 transformation from usage id to extended usage is done prior to adding a
 main item, but I think I see where you're coming from (see below).

 > As far as i understand when an ITEM_TAG_LOCAL_USAGE is declared then it
 should have the usage page of the previously declared global usage page(Am
 i right ?).

 The local usage can override the global usage page if it is an extended
 usage, see the HID 1.1 specs page 51.

 > USAGE_PAGE(Generic Desktop) type global[[BR]]
 > USAGE(GamePad) type local[[BR]]
 > Then when an item of ITEM_TAG_MAIN_COLLECTION is found for the first
 time,  the localState.usage_stack is passed to HIDCollection but it's
 empty because the localState.usage_stack hasn't been initialized to point
 to usageStack.[[BR]]

 You are right, and it is even worse: not only is the initialization of the
 usage stack missing, but also the whole other preprocessing like the
 potential conversion of usage ids to extended usages and the designator
 and string initialization. That is most probably also the reason why you
 thought what you wrote in the above case was necessary, as with the
 missing usage conversion you might've ended up with a collection that only
 gets a usage id istead of a full extended usage.

 > I've modified the code as follows,
 > if (item->tag == ITEM_TAG_MAIN_COLLECTION) {[[BR]]
 > localState.usage_stack = usageStack;[[BR]]
 > localState.usage_stack_used = usageStackUsed;[[BR]]
 > // then the code continues as normal.[[BR]]
 >
 > By doing this modification the HIDCollection now at least has access to
 the correct USAGE_PAGE and USAGE_ID for the devices type we are trying to
 get.

 Basically yes, but it should just do the same preprocessing of the local
 state as is done for adding a main item. I probably simply forgot about
 doing this processing also for collections, as back then I didn't use them
 for anything. I'll correct that in the driver now, thanks for the
 pointers!

 A few remarks about this report: It would make it a lot easier to
 understand what you did by simply attaching a patch instead of describing
 the changes, as with the description you have to fully go through the code
 at hand first and then guess where exactly you might have put the changes.
 Also it'd be helpful if you stated how you arrived at the conclusions you
 did, for example if you based that on the code, on code of other projects
 or on the specs. The HID driver as it is now is completely "by the book"
 based on the specs. Adding code because it works instead of because one
 understood the intentions of the specs would be a very bad thing, as this
 can easily break things subtly which is later hard to track down.

-- 
Ticket URL: <http://dev.haiku-os.org/ticket/7354#comment:6>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.

Other related posts: