Setup
I was just looking through the code in the HIDS module in more detail, now that I have built a few demo HIDS apps I want to make sure that I understand the HIDS module inside and out. I was looking at the Input and Output report structures and saw this:
/**@brief HID Service Input Report characteristic init structure. This contains all options and
* data needed for initialization of one Input Report characteristic. */
typedef struct
{
uint16_t max_len; /**< Maximum length of characteristic value. */
ble_srv_report_ref_t rep_ref; /**< Value of the Report Reference descriptor. */
ble_srv_cccd_security_mode_t security_mode; /**< Security mode for the HID Input Report characteristic, including cccd. */
uint8_t read_resp : 1; /**< Should application generate a response to read requests. */
} ble_hids_inp_rep_init_t;
/**@brief HID Service Output Report characteristic init structure. This contains all options and
* data needed for initialization of one Output Report characteristic. */
typedef struct
{
uint16_t max_len; /**< Maximum length of characteristic value. */
ble_srv_report_ref_t rep_ref; /**< Value of the Report Reference descriptor. */
ble_srv_cccd_security_mode_t security_mode; /**< Security mode for the HID Output Report characteristic, including cccd. */
uint8_t read_resp : 1; /**< Should application generate a response to read requests. */
} ble_hids_outp_rep_init_t;
If you then look at the definition of ble_srv_report_ref_t you get this:
/**@brief Value of a Report Reference descriptor.
*
* @details This is mapping information that maps the parent characteristic to the Report ID(s) and
* Report Type(s) defined within a Report Map characteristic.
*/
typedef struct
{
uint8_t report_id; /**< Non-zero value if there is more than one instance of the same Report Type */
uint8_t report_type; /**< Type of Report characteristic (see @ref BLE_HIDS_REPORT_TYPE) */
} ble_srv_report_ref_t;
The problem
That report_type field can be INPUT, OUTPUT or FEATURE respectively...but it is used inside of a struct that is specific to input and output reports. Given that these are unique types, I suspect that something like this would be invalid state:
ble_hids_inp_rep_init_t input_report_array[1];
memset((void *)input_report_array, 0, sizeof(ble_hids_inp_rep_init_t));
// Initialize HID Service
p_input_report = &input_report_array[0];
p_input_report->max_len = INPUT_REPORT_KEYS_MAX_LEN;
p_input_report->rep_ref.report_id = INPUT_REP_REF_ID;
p_input_report->rep_ref.report_type = BLE_HIDS_REP_TYPE_OUTPUT;
// ^ ERROR: How can you put an output type in an input report?
So either one of two things is true:
- Either it is possible to create an input report with an output report type; which seems invalid. OR
- It is completely valid to have an input report with an output report type and I just don't understand why yet.
This is my question: which of these is true?
Suggestion for a cleaner API in the first case
If it is the first case, then I would propose eliminating the need for the ble_srv_report_ref_t in future versions of the HIDS APIs and just do this instead:
/**@brief HID Service Input Report characteristic init structure. This contains all options and
* data needed for initialization of one Input Report characteristic. */
typedef struct
{
uint16_t max_len; /**< Maximum length of characteristic value. */
uint8_t report_id; /**< Non-zero value if there is more than one instance of the same Report Type */
ble_srv_cccd_security_mode_t security_mode; /**< Security mode for the HID Input Report characteristic, including cccd. */
uint8_t read_resp : 1; /**< Should application generate a response to read requests. */
} ble_hids_inp_rep_init_t;
That way it is impossible to make a mistake when initialising your BLE HID Service.
