Cache driver acceleration configuration - #127
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe driver now prepares acceleration and curve parameters before runtime use. A seqlock cache refreshes these values after parameter updates. Rotation event insertion uses a dedicated helper. The package version changes to 0.5.10. ChangesAcceleration pipeline
Package release metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ParameterCallbacks
participant refresh_cached_args
participant SeqlockCache
participant collect_args
participant f_accelerate_prepared
ParameterCallbacks->>refresh_cached_args: refresh after successful parameter update
refresh_cached_args->>SeqlockCache: publish prepared_accel_args
collect_args->>SeqlockCache: read prepared_accel_args with retry protection
SeqlockCache-->>collect_args: return stable prepared arguments
f_accelerate_prepared->>collect_args: use cached prepared arguments
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
driver/input_handler.h (1)
70-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix asymmetric event-count bookkeeping and misleading debug log.
The X-insertion block (lines 83-94) advances
syn_pos,end, andcounttogether on success. The Y-insertion block (lines 96-105) advances onlycount, notsyn_posorend. In the current function this has no live effect, because neither variable is read after the Y block. But it leaves the two blocks inconsistent. A future change that reorders the blocks, adds a third insertion, or reusesend/syn_posafter the Y block will silently use stale values and desyncmemmoveranges fromcount.Also, in both blocks,
dbg(...)sits outsideif (syn_pos). Whensyn_posisNULL(noSYN_REPORTfound), the function logs a successful injection message even though nothing was inserted. This produces a misleading debug trace exactly when a user is troubleshooting rotation behavior with a debug build.♻️ Proposed fix for symmetry and log accuracy
if (injected_x && synthetic_x_val != NONE_EVENT_VALUE && count < max) { if (syn_pos) { memmove(syn_pos + 1, syn_pos, (end - syn_pos) * sizeof(*syn_pos)); syn_pos->type = EV_REL; syn_pos->code = REL_X; syn_pos->value = synthetic_x_val; syn_pos++; end++; count++; + dbg("rotation: injected synthetic REL_X = %d", synthetic_x_val); } - dbg("rotation: injected synthetic REL_X = %d", synthetic_x_val); } if (injected_y && synthetic_y_val != NONE_EVENT_VALUE && count < max) { if (syn_pos) { memmove(syn_pos + 1, syn_pos, (end - syn_pos) * sizeof(*syn_pos)); syn_pos->type = EV_REL; syn_pos->code = REL_Y; syn_pos->value = synthetic_y_val; + syn_pos++; + end++; count++; + dbg("rotation: injected synthetic REL_Y = %d", synthetic_y_val); } - dbg("rotation: injected synthetic REL_Y = %d", synthetic_y_val); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@driver/input_handler.h` around lines 70 - 109, Make the Y-insertion block in inject_rotation_events mirror the successful X-insertion bookkeeping by advancing syn_pos and end along with count after memmove. Move each rotation debug message inside its corresponding if (syn_pos) block so it is emitted only when the synthetic event was actually inserted.driver/accel.h (1)
50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
staticto the new file-scope constants.
NORMALIZED_DPIandDEG_TO_RAD_FACTORare defined withoutstatic, unlike every other symbol in this header, which usesstatic inline. Withoutstatic, these objects have external linkage. If this header is ever included from more than one translation unit that is linked together, the linker reports duplicate symbols.Add
staticto match the file's existing internal-linkage convention.As per coding guidelines, "Adhere to existing project conventions, mimic surrounding code style and patterns."
🔧 Proposed fix
-const fpt NORMALIZED_DPI = fpt_fromint(1000); -const fpt DEG_TO_RAD_FACTOR = fpt_xdiv(FIXEDPT_PI, fpt_rconst(180)); +static const fpt NORMALIZED_DPI = fpt_fromint(1000); +static const fpt DEG_TO_RAD_FACTOR = fpt_xdiv(FIXEDPT_PI, fpt_rconst(180));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@driver/accel.h` around lines 50 - 51, Update the file-scope constants NORMALIZED_DPI and DEG_TO_RAD_FACTOR to use static, matching the header’s existing internal-linkage convention and preventing duplicate symbols across translation units.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@driver/accel.h`:
- Around line 50-51: Update the file-scope constants NORMALIZED_DPI and
DEG_TO_RAD_FACTOR to use static, matching the header’s existing internal-linkage
convention and preventing duplicate symbols across translation units.
In `@driver/input_handler.h`:
- Around line 70-109: Make the Y-insertion block in inject_rotation_events
mirror the successful X-insertion bookkeeping by advancing syn_pos and end along
with count after memmove. Move each rotation debug message inside its
corresponding if (syn_pos) block so it is emitted only when the synthetic event
was actually inserted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 059b4a6e-f4d6-4142-be4e-570263d9b012
📒 Files selected for processing (9)
PKGBUILDdriver/accel.hdriver/accel/linear.hdriver/accel/natural.hdriver/accel/synchronous.hdriver/accel_k.hdriver/input_handler.hdriver/maccel.cdriver/params.h
1 participant
Summary
Performance
maccel_events: 10,744 bytes down to 611 bytes, with the 7,579-byte acceleration routine outlinedVerification
make buildmake testcargo test --allRuntime hardware verification was intentionally omitted because it requires loading the kernel module.
Summary by CodeRabbit
New Features
Bug Fixes
Chores