Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rtl872x] usb: HID #2659

Merged
merged 2 commits into from Jun 8, 2023
Merged

[rtl872x] usb: HID #2659

merged 2 commits into from Jun 8, 2023

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Jun 6, 2023

Description

USB HID (Mouse / Keyboard) support for P2 / Photon 2.

Same APIs as previously on Photon / P1 / Electron:
https://docs.particle.io/reference/device-os/api/keyboard/keyboard/
https://docs.particle.io/reference/device-os/api/mouse/mouse/

Caveats

  1. Mouse.moveTo() might not be moving the cursor (at least on Linux). Might need to come up with a different digitizer descriptor. Wayland uses multiple pointers, so standard pointer doesn't move. Might be the same nowadays on Windows/OSX.
  2. We are out of USB endpoints to run CDC + HID at the same time. When both are enabled we are again using a hack from Electron times where one of the CDC endpoints is in fact invalid. This works fine on Linux, double check Windows / OSX behavior, but should probably behave fine as well, as again we've done the same thing with Electron.

@avtolstoy avtolstoy requested a review from XuGuohui June 6, 2023 12:40
@avtolstoy avtolstoy added this to the 5.3.3 milestone Jun 6, 2023
} else {
return handleOutSetupRequest(req);
}
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we can remove the else here, the above if will always return directly.

return dev_->setupReply(req, nullptr, 0);
}
break;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: To be consistent, add default case. The following switch does add default.

}

int HidClassDriver::setup(SetupRequest* req) {
if (req->bmRequestTypeType != SetupRequest::TYPE_CLASS && req->bmRequestTypeType != SetupRequest::TYPE_STANDARD) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prefer to use CHECK(). Same to other checks.

hal/src/rtl872x/usbd_hid.cpp Show resolved Hide resolved
hal/src/rtl872x/usbd_hid.cpp Outdated Show resolved Hide resolved
hal/src/rtl872x/usbd_hid.h Show resolved Hide resolved
hal/src/rtl872x/usbd_hid.h Show resolved Hide resolved
hal/src/rtl872x/usbd_hid.cpp Show resolved Hide resolved
0xc0 // END_COLLECTION
};

const size_t DEFAULT_HID_DESCRIPTOR_DIGITIZER_LEN = 65;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: constexpr

@@ -34,6 +34,7 @@ using namespace particle::usbd;
namespace {

const char DEFAULT_NAME[] = HAL_PLATFORM_USB_PRODUCT_STRING " " "USB Serial";
const uint8_t DUMMY_IN_EP = 0x8a;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: constexpr

@XuGuohui XuGuohui self-requested a review June 6, 2023 15:40
@avtolstoy avtolstoy merged commit 118b47a into develop Jun 8, 2023
1 check passed
@avtolstoy avtolstoy deleted the feature/p2-usb-hid branch June 8, 2023 13:17
@scott-brust scott-brust modified the milestones: 5.3.3, 5.4.0 Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants