Skip to content
This repository has been archived by the owner on Dec 14, 2023. It is now read-only.

Demo of boot protocol working #50

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Demo of boot protocol working #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9997b29
Fix switching between boot protocol and report protocol
gedankenexperimenter May 21, 2022
3606015
Add debugging output for keyboard reports
gedankenexperimenter May 21, 2022
b8001cd
Comment the relevant line where it seems things should fail
gedankenexperimenter May 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/BootKeyboard/BootKeyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ bool BootKeyboard_::setup(USBSetup& setup) {
if (request == HID_GET_PROTOCOL) {
// TODO improve
#ifdef __AVR__
// This is where the `protocol` variable is used to set...something, but I don't
// know what. With the change that I've made to use `boot_protocol_` instead, this
// would always be set to `HID_REPORT_PROTOCOL`, even when sending boot protocol
// reports (successfully). It doesn't seem correct, but it works on macOS and Linux
// (or at least, my old Ubuntu machine).
UEDATX = protocol;
Copy link

Choose a reason for hiding this comment

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

This is an optimization for AVR to queue a single byte for transmission on the control pipe. In this particular case, it is the single byte response to the HID_GET_PROTOCOL request.

#endif
#ifdef ARDUINO_ARCH_SAM
Expand Down Expand Up @@ -237,15 +242,29 @@ uint8_t BootKeyboard_::getLeds() {
}

uint8_t BootKeyboard_::getProtocol() {
return protocol;
if (boot_protocol_) {
return HID_BOOT_PROTOCOL;
} else {
return HID_REPORT_PROTOCOL;
}
Copy link
Author

Choose a reason for hiding this comment

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

This reports the protocol based on the new boot_protocol_ variable instead of protocol. This is ultimately what controls which type of report is sent, in the HID adaptor code. See this line in Kaleidoscope-HIDAdaptor-KeyboardioHID for details.

}

void BootKeyboard_::setProtocol(uint8_t protocol) {
this->protocol = protocol;
if (protocol == HID_BOOT_PROTOCOL) {
boot_protocol_ = true;
} else {
boot_protocol_ = false;
}
Copy link
Author

Choose a reason for hiding this comment

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

Instead of setting the protocol variable here, I leave it alone and set the new boolean boot_protocol_. The protocol variable stays set to HID_REPORT_PROTOCOL, set in the constructor:

BootKeyboard_::BootKeyboard_(void) : PluggableUSBModule(1, 1, epType), protocol(HID_REPORT_PROTOCOL), idle(1), leds(0) {

}

int BootKeyboard_::sendReport() {
if (memcmp(&last_report_, &report_, sizeof(report_))) {
Serial.print(F("sending boot report: "));
for (byte i{0}; i < 8; ++i) {
Serial.print(F(" "));
Serial.print(report_.bytes[i], HEX);
}
Serial.println();
Copy link
Author

Choose a reason for hiding this comment

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

Serial debug output showing the details of any report sent in boot protocol mode.

// if the two reports are different, send a report
int returnCode = USB_Send(pluggedEndpoint | TRANSFER_RELEASE, &report_, sizeof(report_));
HIDReportObserver::observeReport(HID_REPORTID_KEYBOARD, &report_, sizeof(report_), returnCode);
Expand Down
1 change: 1 addition & 0 deletions src/BootKeyboard/BootKeyboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class BootKeyboard_ : public PluggableUSBModule {

EPTYPE_DESCRIPTOR_SIZE epType[1];
uint8_t protocol;
bool boot_protocol_{false};
Copy link
Author

@gedankenexperimenter gedankenexperimenter Mar 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

true => boot protocol
false => report (NKRO) protocol

Copy link

Choose a reason for hiding this comment

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

This adds a state variable whose only purpose is to track changes requested by Kaleidoscope, and to feed them back to Kaleidoscope via getProtocol. I'm not sure it belongs here. On the other hand, setProtocol is of dubious value as well, because (before these changes) it can force the interface to answer GET_PROTOCOL with a different protocol than the host previously selected, which might cause the host to decide that the interface is misbehaving.

uint8_t idle;

uint8_t leds;
Expand Down
6 changes: 6 additions & 0 deletions src/MultiReport/Keyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ void Keyboard_::end() {
}

int Keyboard_::sendReportUnchecked() {
Serial.print(F("sending nkro report:"));
for (byte i{0}; i < 29; ++i) {
Serial.print(F(" "));
Serial.print(report_.allkeys[i], HEX);
}
Serial.println();
Copy link
Author

Choose a reason for hiding this comment

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

This is all just debug output, making it clear when the keyboard is in report protocol mode.

return HID().SendReport(HID_REPORTID_NKRO_KEYBOARD,
&last_report_, sizeof(last_report_));
}
Expand Down