Skip to content

Enhance USB HID handling and documentation for boot-keyboard sensitivity#2

Open
chrisgleissner wants to merge 4 commits into
mainfrom
fix/keyboard
Open

Enhance USB HID handling and documentation for boot-keyboard sensitivity#2
chrisgleissner wants to merge 4 commits into
mainfrom
fix/keyboard

Conversation

@chrisgleissner

@chrisgleissner chrisgleissner commented Apr 22, 2026

Copy link
Copy Markdown
Owner
  • Added checks to preserve boot-keyboard functionality in the presence of sibling report-keyboard interfaces.
  • Updated AGENTS.md and SKILL.md with findings on boot-keyboard behavior during HID carryover.
  • Revised README.md to clarify warranty and responsibility regarding custom firmware usage.
  • Introduced a new PLANS.md for the Forge Help System, outlining phases for a hierarchical help system.
  • Documented worklog entries detailing progress and validation for the help system implementation.

Copilot AI review requested due to automatic review settings April 22, 2026 22:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the Spiffy USB HID interface selection logic to better preserve boot-keyboard behavior on C64U when devices expose sibling report-keyboard interfaces, and adds supporting repo documentation (help-system planning/worklog + carryover guidance + warranty note).

Changes:

  • Prefer boot-keyboard selection whenever a boot keyboard interface is present, regardless of sibling report-keyboard state.
  • Prevent descriptor-only keyboard interfaces from claiming keyboard input when a sibling boot-keyboard interface exists.
  • Add/refresh documentation: warranty/responsibility, help-system plan + worklog, and carryover guidance for boot-keyboard sensitivity.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
patches/spiffy-ultimate/software/io/usb/usb_hid_selection.h Adjusts HID interface selection to always prefer boot keyboard when present.
patches/spiffy-ultimate/software/io/usb/usb_hid.cc Adds sibling boot-keyboard detection and suppresses descriptor-only keyboard claiming when a boot sibling exists.
WORKLOG.md Adds detailed worklog entries for the Forge help-system implementation/validation.
README.md Adds warranty/responsibility clarification around custom firmware usage.
PLANS.md Introduces a phased plan for a hierarchical ./forge help system.
AGENTS.md Documents carryover findings/guidance for boot-keyboard-sensitive HID behavior on c64u.
.github/skills/spiffy-carryover/SKILL.md Extends the carryover skill with explicit boot-keyboard sibling/interface-order guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +51 to +54
// Preserve Spiffy's original boot-keyboard path whenever a boot keyboard is
// present. Sibling report-keyboard interfaces can appear first and should
// not block the working c64u built-in keyboard path based on install order.
if (current.boot_keyboard) {

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

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

Inside usb_hid_select_interface(), sibling_active_report_keyboard is no longer referenced after the boot-keyboard preference change. This may cause unused-parameter warnings with some toolchains; consider removing the parameter (and updating the call site) or explicitly marking it unused.

Copilot uses AI. Check for mistakes.
Comment on lines +910 to +917
if (descriptor_keyboard &&
!current_capabilities.boot_keyboard &&
usb_hid_device_has_sibling_boot_keyboard(dev, interface)) {
// Keep descriptor-only companion interfaces from stealing the c64u
// built-in keyboard away from the working boot-keyboard path.
descriptor_keyboard = false;
current_capabilities.descriptor_keyboard = false;
}

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

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

The new suppression of descriptor_keyboard is based solely on the presence of a sibling boot-keyboard interface descriptor. If that sibling boot interface later fails to install (e.g., missing interrupt IN endpoint / pipe allocation failure), this interface will never claim the keyboard, potentially leaving the device with no keyboard input. Consider keying this suppression off a successfully installed/active sibling boot-keyboard driver (or adding a fallback when no boot keyboard ends up active).

Suggested change
if (descriptor_keyboard &&
!current_capabilities.boot_keyboard &&
usb_hid_device_has_sibling_boot_keyboard(dev, interface)) {
// Keep descriptor-only companion interfaces from stealing the c64u
// built-in keyboard away from the working boot-keyboard path.
descriptor_keyboard = false;
current_capabilities.descriptor_keyboard = false;
}

Copilot uses AI. Check for mistakes.
- Added checks to preserve boot-keyboard functionality in the presence of sibling report-keyboard interfaces.
- Updated AGENTS.md and SKILL.md with findings on boot-keyboard behavior during HID carryover.
- Revised README.md to clarify warranty and responsibility regarding custom firmware usage.
- Introduced a new PLANS.md for the Forge Help System, outlining phases for a hierarchical help system.
- Documented worklog entries detailing progress and validation for the help system implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants