hid_host_send_set_report wrongly returns ERROR_CODE_COMMAND_DISALLOWED

[Slion]

Pico W - develop branch

hid_host_send_set_report wrongly returns ERROR_CODE_COMMAND_DISALLOWED

When commenting out the following lines it works just fine suggesting the connection state is messed up.

    if (connection->state != HID_HOST_CONNECTION_ESTABLISHED){
        return ERROR_CODE_COMMAND_DISALLOWED;
    } 

set-report-fails.zip

Searching the code base for hid_host_send_set_report yields no results outside of declaration and definition suggesting this functionality had little testing done.

I added some logs and basically it works once after connection and then fails cause the connection state is set to HID_HOST_W4_SET_REPORT_RESPONSE which is 12. HID_HOST_CONNECTION_ESTABLISHED is 8.

Extra logs sample:

[00:00:20.480] LOG -- l2cap.c.349: Stop Monitor timer
[00:00:20.484] CMD => 35 0C 05 01 0B 00 01 00 
[00:00:21.138] LOG -- hid_host.c.1367: hid_host_send_set_report - connection state 8
[00:00:21.143] ACL => 0B 00 07 00 03 00 4A 00 52 01 02 
[00:00:21.404] EVT <= 13 05 01 0B 00 01 00 
[00:00:24.035] ACL <= 0B 20 07 00 03 00 42 00 A1 05 C9 
[00:00:24.040] CMD => 35 0C 05 01 0B 00 01 00 
[00:00:24.044] EVT <= 14 06 00 0B 00 02 0C 00 
[00:00:24.046] LOG -- hci.c.4112: HCI_EVENT_MODE_CHANGE, handle 0x000b, mode 2
[00:00:25.762] LOG -- hid_host.c.1367: hid_host_send_set_report - connection state 12
[00:00:28.490] LOG -- hid_host.c.1367: hid_host_send_set_report - connection state 12

[Slion]

The issue was that the handshake event was never sent from the device side thus HID_SUBEVENT_SET_REPORT_RESPONSE was never fired and the connection state was messed up. I think we can close this unless we somehow want to be able to recover from misbehaving clients. We ought to check what the specs says too.

[mringwal]

HID Spec v1.1.1: "A Bluetooth HID Host shall not have more than one Control channel transfer simultaneously outstanding to a given Bluetooth HID device. An exception is that a Bluetooth HID Host or Bluetooth HID device may send a HID_CONTROL message that specifies VIRTUAL_CABLE_UNPLUG event irrespective of whether another Control channel transfer is in progress. "

There's no mention of a timeout. Given the spec, we're not allowed to send another request if the previous one hasn't been answered ( there are no message sequence numbers either)

[Slion]

On the Android side the onSetReport documentation suggest you should only send a response in case of error which is apparently wrong then. All I did to fix it was sending back the success error code zero.

    /**
     * From [BluetoothHidDevice.Callback]
     */
    override fun onSetReport(device: BluetoothDevice?, type: Byte, reportId: Byte, data: ByteArray?) {
        super.onSetReport(device, type, reportId, data)
        Timber.d("onSetReport - Device: $device - Type: $type - Id: $reportId - Data: ${data?.toHexString()}")
        // Here we notably process keyboard LED reports

        // Possible response as defined in btstack from the specs
//        HID_HANDSHAKE_PARAM_TYPE_SUCCESSFUL = 0x00,        // This code is used to acknowledge requests. A device that has correctly received SET_REPORT, SET_IDLE or SET_PROTOCOL payload transmits an acknowledgment to the host.
//        HID_HANDSHAKE_PARAM_TYPE_NOT_READY,                // This code indicates that a device is too busy to accept data. The Bluetooth HID Host should retransmit the data the next time it communicates with the device.
//        HID_HANDSHAKE_PARAM_TYPE_ERR_INVALID_REPORT_ID,    // Invalid report ID transmitted.
//        HID_HANDSHAKE_PARAM_TYPE_ERR_UNSUPPORTED_REQUEST,  // The device does not support the request. This result code shall be used if the HIDP message type is unsupported.
//        HID_HANDSHAKE_PARAM_TYPE_ERR_INVALID_PARAMETER,    // A parameter value is out of range or inappropriate for the request.
//        HID_HANDSHAKE_PARAM_TYPE_ERR_UNKNOWN = 0x0E,       // Device could not identify the error condition.
//        HID_HANDSHAKE_PARAM_TYPE_ERR_FATAL = 0x0F,         // Restart is essential to resume functionality

        // Report success, this is needed otherwise btstack on Pico W get stuck waiting for the handshake
        hid?.reportError(device,0)
    }

[XZCE]

I think this is the same issue I had with setting the LED status more than once with a Pi Pico on my connected BT keyboard:

https://github.com/bluekitchen/btstack/issues/600#issuecomment-2160101550

It's fine if my cheap keyboard doesn't implement the protocol correctly, but it does actually work OK when connected to other computers, rather than my Pi Pico.

[mringwal]

The Android doc does not say that you don't need to send a response, but I agree that it's not precise.

Here's another quote from the HID spec.

"A Control SET_* request sends information to the Bluetooth HID device.

If the SET_* payload exceeds the MTU then the Bluetooth HID Host shall not transmit the report.

Following the reception of the SET_* message the Bluetooth HID device shall return a HANDSHAKE message with result code of SUCCESSFUL if no errors were detected. "