raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.65k stars 909 forks source link

host_hid mouse data displaced #442

Closed dshadoff closed 2 years ago

dshadoff commented 3 years ago

Some wired mice (such as the official Raspberry Pi mouse) work with the host_hid USB example code in pico-examples - in fact, it seems that at the moment, only wired mice are recognized (although this is not the issue I wish to report here).

At least some wired mice which have side buttons (i.e. back/forward) have displaced data:

(note: the mouse works fine on a RPi 400)

I am reporting this here as pico-sdk holds a non-current version of tinyusb, and it doesn't seem to be straightforward to test with the current tinyusb on pico hardware at the moment.

This may be solved with the newer version of tinyusb in pico-sdk 1.2.0 ... this issue can serve as a placeholder until that is available for test; if pico-sdk and tinyusb are in sync at that time (and if the issue is not yet resolved), I will be happy to report the issue upstream. I am also happy to gather any diagnostic information required (if the process is explained).

For the moment, the existence of a report may be helpful to other users, to indicate current known system limitations.

dshadoff commented 3 years ago

I tested with several mice - including the two mentioned earlier in this thread - and they all seemed to work, and to provide consistent results. Thank You ! I will be happy to close this issue when the code gets integrated into pico-sdk (until then, perhaps it can stay open in order to inform other users about status).

The only thing I noticed, was surrounding the disconnection and reconnection of mouse devices - but that appears to be the same as issue #483 opened by @v9938 , so it can be tracked there.

I will continue to test with a variety of other devices I have; if I find any issues, I will open separate issues for tracking.

hathach commented 3 years ago

thank for the feedback, look like I nailed it this time. For the release/update, it may take a bit of time, since I still want to do more tweak to host API but have been kept busy with other works.

hathach commented 3 years ago

@v9938 let me know if the new code fox issue with your devices

v9938 commented 3 years ago

@hathach Sorry for the late feedback. Unfortunately, there was no change in the behavior. If there is anything I can do to help, please reply.

Model No:C-U007

HID device address = 1, instance = 0 is mounted
HID Interface Protocol = Keyboard
usbh_edpt_xfer 386: ASSERT FAILED
config_get_report_desc_complete 395: ASSERT FAILED

Model No:C-U008

*** PANIC ***

ep 0 in was already available
hathach commented 3 years ago

@v9938 thanks for your feedback, seem like there is still issue :( . Could you also attach both log with LOG=1 and LOG=3 for each of your device.

v9938 commented 3 years ago

@hathach The LOG files are attached below.

Model No:C-U007 LOG=1 U007_LV1.log / LOG=3 U007_LV3.log , U007_LV3_case2.log

Model No:C-U008 LOG=1 U008_LV1.log / LOG=3 U008_LV3.log

dshadoff commented 3 years ago

@v9938 , these logs are related to the disconnect/reconnect issue, issue #483 ... shouldn't they be tracked there instead ? I interpreted @hathach 's comments to be "do you see issues with the data - OTHER THAN those tracked in issue 483 ?"

hathach commented 3 years ago

indeed @v9938 this seems to be issue with connection, which can be either hardware and/or connection issue. To further troubleshoot it, I would suggest you to post picture of your hw set-up, also have shortest otg cable as possible etc... I don't see any abnormal connection issue with mine. I think people could then help you more with troubleshoot if it is hardware issue. I think my work here is done. Thanks both of you for testing out and giving feedback.

Update: more-host-hid pr is merged to master.

v9938 commented 3 years ago

@hathach Sorry for the late response. There were several problems and it took a long time to prepare. I have considered the possibility of my board being broken. I tried the same experiment again with a new pico board. The result was no change :-(

The next picture shows the OTG cable. In both cables, the result was the same result.

The experimental setup was as shown in the next photo.

Maybe there is nothing wrong with the environment.

hathach commented 3 years ago

I am not good at hardware, here is my set up that work relatively well. I use another pico as picoprobe (above) also providing 5V to target (below) via pin 40. IMG_1489

dshadoff commented 2 years ago

SDK version 1.3.0 release and the corresponding TinyUSB version are working fine for this. I will close the issue now.

kilograham commented 2 years ago

cc @liamfraser

dupontgu commented 1 year ago

Want to add my experience for anyone who is still facing this - I am seeing the exact same issue as the original post, but I think I figured out what my problem was (And I don't know if it was teased out in all the comments above).

I was mostly following this example, and there seems to be an edge case where tuh_hid_interface_protocol returns HID_ITF_PROTOCOL_MOUSE, even if the reports from that instance should be treated as "composite" reports. So in my case, process_mouse_report was getting run with the raw report, even though the first byte of that report was actually a report_id, NOT the start of the mouse data (As OP was seeing, the report ID was 0x01, making it seem like the mouse left button was always pressed). So I changed my callback to look like this (forgive the Google formatting), which checks the instance's report descriptor to see if we should expect a report_id to show up in the report. Not terribly efficient, seems to work? Constants pulled from the (HID usage tables)[https://usb.org/document-library/hid-usage-tables-14].

void tuh_hid_report_received_cb(uint8_t dev_addr, uint8_t instance,
                                uint8_t const* report, uint16_t len) {
  uint8_t itf_protocol = HID_ITF_PROTOCOL_NONE;
  uint8_t const* report_offset = report;

  if (hid_info[instance].report_count > 1) {
    // we know this is a composite report, so extract the id and start the reading the data one byte over
    report_offset = report + 1;
    uint8_t id = report[0];

    // find the tuh_hid_report_info_t that matches the id of the report we just received 
    for (size_t i = 0; i < hid_info[instance].report_count; i++) {
      tuh_hid_report_info_t info = hid_info[instance].report_info[i];
      if (info.report_id == id) {
        if (info.usage_page == GENERIC_DESKTOP_USAGE_PAGE) {
          if (info.usage == USAGE_MOUSE) {
            itf_protocol = HID_ITF_PROTOCOL_MOUSE;
          } else if (info.usage == USAGE_KEYBOARD) {
            itf_protocol = HID_ITF_PROTOCOL_KEYBOARD;
          }
        }
        // TODO handle other usage pages, Consume Control, etc?
        break;
      }
    }
  } else {
    itf_protocol = tuh_hid_interface_protocol(dev_addr, instance);
  }

  switch (itf_protocol) {
    case HID_ITF_PROTOCOL_KEYBOARD:
      process_kbd_report(dev_addr, (hid_keyboard_report_t const*)report_offset);
      break;

    case HID_ITF_PROTOCOL_MOUSE:
      process_mouse_report(dev_addr, (hid_mouse_report_t const*)report_offset);
      break;

    default:
      break;
  }
  //...
}

I am testing with a little portable keyboard/touchpad like this, and it appears to send mouse and Consumer Control data via the same "instance", so report_id's are used to distinguish between mouse reports and CC reports.