Extension of the USB HID Usages

[x-ist]

The following patch advances the definitions in /trunk/src/add-ons/kernel/drivers/input/usb_hid of HID Usage Pages along with their Usage IDs to a large extend.

To provide a better overview and accessibility of the defined Usage IDs I decided to move them out of USB_hid.h into separate files, all being included by USB_hid.h. This prepares also for future implementations (e.g. of datastructures possibly required for specific HID Usage Pages).

Usage Pages start with HID_USAGE_PAGE_... Usage IDs start with HID_UID_<pn>_ Here <pn> represents a short version of the usage page.

[x-ist]

Extension of USB HID Usages

[x-ist]

Sorry for resending multiple times! WebPositive has tricked me..

Correction: The location of USB_hid.h I meant was /trunk/headers/os/drivers/usb

[axeld]

Thanks for your work!

Just a few remarks:

• The changes of the HID usage IDs probably require an updated HID driver?
• No blank line between the copyright header, and the header guard.
• The references and other comments do not belong into the copyright header.
• We usually don't add authors to public headers; they specify the system API, and even though someone actually took the time to wrote it, everyone needs to be okay with them - it's your call in the end, though.
• Two blank lines between sections, ie. after the header guard, and before the last #endif.
• If those should really be public headers, we should add the B_ prefix to all constants.
• I find the header names a bit odd: why is hid_page lower case and Keyboard upper case? I would prefer lower case (with an extra _ for alphanumeric_display)
• In general, the constant names look good IMO.

Maybe mmlr has some other comments, as he is more into the subject.

[x-ist]

Have fixed the style-related objections above. So what about adding the "B_" prefix now? BTW, the updated HID driver will be submitted too.

[axeld]

Great! I would be all for adding the "B_" prefix now, too.

[phoudoin]

Replying to axeld:

Great! I would be all for adding the "B_" prefix now, too.

I'm against: all USB definitions are from external specifiers, either USB-IF or vendors. Neither Be Inc at his time neither us can influence USB standards specification. We made some effort to keep these definitions names from being as crypted as seen on other platforms, witch is great, but prefixing with B_ wont add any value but could make someone think it's specific to BeOS/Haiku.

On the other side, I'm for having one single prefix for every USB constants in our USB_*.h, which we don't have yet. Same should applied for typenames too (usb_*).

Anyway, if you choose to prefix all USB constants, there is several others USB_*.h and components including them to fix too for consistency.

To be short:

1. I'm against prefixing with B_,
2. but I'm for always prefixing with USB_

;-)

[x-ist]

Replying to phoudoin:

I'm against: all USB definitions are from external specifiers, either USB-IF or vendors. Neither Be Inc at his time neither us can influence USB standards specification. We made some effort to keep these definitions names from being as crypted as seen on other platforms, witch is great, but prefixing with B_ wont add any value but could make someone think it's specific to BeOS/Haiku.

That's my opinion too.

On the other side, I'm for having one single prefix for every USB constants in our USB_*.h, which we don't have yet. Same should applied for typenames too (usb_*).

IMO, when working with USB one definitely knows about HID, such that using USB_ wouldn't povide more meaning to the name.

Here is an example of a name lend from the spec as is, with alternative prefixes:

HID_UID_WD_SCALE_STATUS_STABLE_AT_CENTER_OF_ZERO B_HID_UID_WD_SCALE_STATUS_STABLE_AT_CENTER_OF_ZERO USB_HID_UID_WD_SCALE_STATUS_STABLE_AT_CENTER_OF_ZERO

Even without the USB_ there is a bunch of prefixes and I would try to keep the prefixes smaller than the name though :-) Are there more points of view?

[phoudoin]

Considering that the only other place where HID terminoly is used that I know of is Bluetooth, which is a wrapper to USB HID standard, I eventually agree with your position: HID_ current prefix is fine.

[axeld]

The point of "B_" is not to make it Be/Haiku specific, the point is not to pollute the global name space. I'm aware that Be Inc. did not care about that for ie. the PCI definitions, but that doesn't mean we should follow here.

More opinions are, as always welcome :-)

[bonefish]

Replying to phoudoin:

Replying to axeld:

Great! I would be all for adding the "B_" prefix now, too.

I'm against: all USB definitions are from external specifiers, either USB-IF or vendors. Neither Be Inc at his time neither us can influence USB standards specification. We made some effort to keep these definitions names from being as crypted as seen on other platforms, witch is great, but prefixing with B_ wont add any value but could make someone think it's specific to BeOS/Haiku.

If the headers are to be public and the defined interface doesn't belong to some standard (e.g. POSIX) there is really nothing to discuss. They have to be prefixed "B_". Which doesn't have anything to do with whether Be, Inc. influences standards, just with what namespace they chose for BeOS's public API. The simple reason for using this prefix is to avoid clashes with third party software.

[idefix]

I spotted two spelling mistakes:

In USB_hid_page_led.h:

B_HID_UID_LED_SURROND_ON,

In USB_hid_page_alphanumeric_display.h:

B_HID_UID_AD_CHAR_ATTR_ENAHNCE,

[idefix]

Some more:

In USB_hid_page_keyboard.h:

B_HID_UID_KB_PERIOF_AND_RCHEVRON,
B_HID_UID_KP_3_AND_PADE_DOWN,

In USB_hid_page_led.h:

B_HID_UID_LED_CAMERA_OF,

In USB_hid_page_telephony.h:

B_HID_UID_TEL_TELPEHONY_KEY_PAD,
B_HID_UID_TEL_READIAL,
B_HID_UID_TEL_RING_EANBLE,
B_HID_UID_TEL_CONFRIMATION_TONE_1,

In USB_hid_page_bar_code_scanner.h:

B_HID_UID_BCS_PREIODICAL = 0xa9,
B_HID_UID_BCS_PREIODICAL_AUTO_DISCRIMINATE_PLUS_2,
B_HID_UID_BCS_PREIODICAL_ONLY_DECODE_WITH_PLUS_2,
B_HID_UID_BCS_PREIODICAL_IGNORE_PLUS_2,
B_HID_UID_BCS_PREIODICAL_AUTO_DISCRIMINATE_PLUS_5,
B_HID_UID_BCS_PREIODICAL_ONLY_DECODE_WITH_PLUS_5,
B_HID_UID_BCS_PREIODICAL_IGNORE_PLUS_5,
B_HID_UID_BCS_TRASNMIT_START_STOP = 0xd3,

In USB_hid_page_power_device.h:

B_HID_UID_POW_DELAY_BEOFRE_STARTUP,
B_HID_UID_POW_AWAITNING_POWER,

In USB_hid_page_pid.h:

B_HID_UID_PID_POOL_ALIGNEMNT,

In: USB_hid_page_battery_system.h:

B_HID_UID_BAT_USENEXT,
B_HID_UID_BAT_NEED_REAPLCEMENT,
B_HID_UID_BAT_AT_RATE_TOME_TO_EMPTY,
B_HID_UID_BAT_MAXERROR,
B_HID_UID_BAT_RUN_TIM_TO_EMPTY,
B_HID_UID_BAT_AVERGAE_TIME_TO_FULL,
B_HID_UID_BAT_IDEVICE_CHEMISTERY,

[idefix]

In USB_hid_page_sport_controls.h:

B_HID_UID_SPO_LOFT_WADGE,
B_HID_UID_SPO_POWER_WADGE,

In USB_hid_page_consumer.h:

B_HID_UID_CON_FUNCITION_BUTTONS,
B_HID_UID_CON_ROOM_TERMPERATURE,
B_HID_UID_CON_AL_TELELPHONY_DIALER,
B_HID_UID_CON_AC_MAXIMZE,
B_HID_UID_CON_AC_MIRROE_VERTICAL,
B_HID_UID_CON_AC_NUMERED_LIST,

In USB_hid_page_alphanumeric_display.h:

B_HID_UID_AD_UNCODE_CHARACTER,

[x-ist]

I expected there would be some typos, but not this much ... ;-) BTW, B_HID_UID_BAT_USENEXT in USB_hid_page_battery_system.h is correct with regard to the spec. Same for B_HID_UID_BAT_IDEVICE_CHEMISTERY, which I corrected nevertheless.

[idefix]

Yeah, I didn't check with the specs but I expected those to be B_HID_UID_BAT_USE_NEXT (just as B_HID_UID_BAT_MAXERROR -> B_HID_UID_BAT_MAX_ERROR) and B_HID_UID_BAT_IDEVICE_CHEMISTRY.

[x-ist]

Discovered Typos corrected.

[x-ist]

USB HID Driver adapted to the new names.

[x-ist]

Is this one still being reviewed?

[mmadia]

re-assigning to axeld. (hoping for review of updated patch).

[stippi]

I've read the patches (not every page header in detail) and they look fine to me.

[stippi]

I have applied the patch in hrev39874. I proof-read the parts that affected the USB HID driver, to confirm no constants had been changed, and made sure everything compiles. Thanks a lot for your work and sorry for the delay!