Port Hurra-v2 review fixes to v3 (desc_capture, act_click, ferrum km.mask) - #5
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Backports three Hurra-v2 fixes into Hurra v3’s shared-lineage code to improve descriptor replay correctness, prevent stuck mouse buttons in click scheduling, and make Ferrum’s km.mask() read path report real state—along with host-native unit tests to lock the behavior in.
Changes:
- Make captured USB descriptors internally consistent when truncated (patch
wTotalLength, HIDwDescriptorLength, stringbLength; clear invalid HID report descriptors on fetch failure). - Rework
act_clickto cancel/unstick overlapping click sequences and drive all click releases via a unifiedact_click_tick()pump fromkmbox_cmd_poll(). - Implement Ferrum
km.mask(<key>)read getter viaact_kb_mask_get(), and add targeted host tests for the new wiring.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ferrum_mask_test.c | Adds host-native coverage for Ferrum km.mask() read/write round-trip. |
| test/desc_capture_test.c | Adds host-native descriptor truncation/consistency tests (includes isolated patch helper tests). |
| test/actions_test.c | Adds host-native tests for click scheduler stuck-button fix and act_kb_mask_get(). |
| src/kmbox_cmd.c | Pumps the new click tick scheduler each poll loop iteration (V3F). |
| src/ferrum.c | Implements km.mask(key) read variant using act_kb_mask_get(). |
| src/desc_capture.c | Adds truncation-consistency patching for config/BOS/HID/string descriptors and clears invalid HID report descriptors. |
| src/actions.h | Exposes act_click_tick() and act_kb_mask_get(). |
| src/actions.c | Implements unified click tick scheduler and adds act_kb_mask_get(). |
| Makefile | Wires new tests into make test (including ASan for desc_capture_test). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ramseymcgrath
force-pushed
the
feat/port-v2-review-fixes
branch
from
July 8, 2026 04:22
a76010e to
c5faa5b
Compare
ramseymcgrath
force-pushed
the
feat/port-v2-review-fixes
branch
from
July 8, 2026 04:30
c5faa5b to
a297bc8
Compare
…mask) Backport three fixes from Hurra-v2's deferred-review branch that predated the v3 fork. Each is host-unit-tested; `make test`, `make all`, and `make PROTOCOL=ferrum all` all build/pass clean. desc_capture: patch descriptor length fields (config/BOS wTotalLength, HID wDescriptorLength, string bLength) when a captured descriptor is truncated to fit its buffer or the device short-transfers, and set has_hid_desc=false on a failed HID report fetch instead of replaying an invalid descriptor. Reject malformed string descriptors (short or wrong-type) rather than storing them, and bound the config walk in patch_hid_report_desc_len() so a truncated trailing byte cannot be read past. Size caps left unchanged: RAM and cross-core serialize-transport headroom were verified, and the patching makes truncation internally consistent at any cap. act_click: cancel an in-flight click sequence and release a still-held button when a new click targets a different button; unify single- and multi-click through act_click_tick(), with a wrap-safe deadline check. Both command pumps (kmbox_cmd_poll and the two_board.c BOARD_ROLE_HOST stage) step motion+click via a shared act_tick() helper, so neither can drift and strand a click by pumping only one. Adds act_kb_mask_get(). g_click_sched/g_buttons confirmed single-core (V3F), so no lock is needed. ferrum: wire the km.mask(<key>) read path to act_kb_mask_get() instead of the always-0 stub. lock_*/g_lock_mask left intact -- they are live in v3's default hurra.c protocol, unlike v2 which removed them. Tests: add test/desc_capture_test.c, test/actions_test.c, and test/ferrum_mask_test.c to the `make test` target, including wrap-around and malformed-descriptor regression cases.
ramseymcgrath
force-pushed
the
feat/port-v2-review-fixes
branch
from
July 8, 2026 04:39
a297bc8 to
ed676fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backports three fixes from Hurra-v2's deferred-review branch that landed after the v3 fork and never made it across. All three touch shared-lineage code; each is host-unit-tested and both firmware profiles build clean.
Fixes
1.
desc_capture— descriptor truncation consistencyWhen a captured descriptor is truncated to fit its buffer, patch the length fields so the replayed descriptor stays internally consistent: config/BOS
wTotalLength, the HIDwDescriptorLengthinside the config descriptor, and stringbLength. On a failed HID report-descriptor fetch, sethas_hid_desc=falseinstead of replaying an invalid descriptor. This matters directly for v3's descriptor-passthrough MITM role.captured_descriptors_tis statically allocated in several places and is serialized cross-core over the SPI frame transport (something v2 lacks). RAM and transport headroom were verified to exist, but the patching makes truncation safe at any cap, and v3's real targets sit far under the current caps — so bumping four large statics buys ~nothing. Documented for a future large-composite-device need.2.
act_click— stuck-button fix + unified tick schedulerGuard invalid button/
count==0; when a new click targets a different button while a prior sequence still holds one down, release the old button first. Single- and multi-click now run throughact_click_tick(), pumped fromkmbox_cmd_poll(). Addsact_kb_mask_get().g_click_sched/g_buttonsare touched only on V3F (command parse + poll run in the same non-preempted loop, on that image's own copy), so no lock/barrier is needed — same discipline as the existingg_motionpath.3.
ferrum—km.mask()read getterThe
km.mask(<key>)read path was a stub that always emitted0. It now reports the real masked state viaact_kb_mask_get().lock_*/g_lock_maskleft intact (v2 deleted them). They are live in v3's defaulthurra.cprotocol (lock_listener+ 7 registered TinyFrame listeners), so v3 intentionally diverges from v2 here.Tests
Adds
test/desc_capture_test.c(ASan),test/actions_test.c, andtest/ferrum_mask_test.ctomake test, each written TDD-first (red → green).make test→ all green, 0 failuresmake all(Hurra binary) → builds; V3F + V5F + merged imagemake PROTOCOL=ferrum all→ builds clean (exercises theferrum.cchange on-target)Not included (follow-ups)
TF_USE_SOF_BYTE: investigated and deliberately not ported — v3 firmware andhurra-appalready agree (both SOF-off, per the TinyFrame default); flipping only the firmware would break interop. Tracked separately as a docs-vs-implementation reconciliation.two_board.cBOARD_ROLE_HOST(non-default build) also pumps the input loop and would want anact_click_tick()call for symmetry.