Fix Aura interface selection and effect persistence for GX651AR Zephyrus Duo - #285
Conversation
…1ce6 device ID & fix syntax
Some laptops (e.g. GX651AR Zephyrus Duo) have no sysfs keyboard backlight class. set_led_mode_data called set_brightness()? right after a successful color write, so a brightness failure aborted the whole operation -- including config.write(), meaning color changes never persisted to disk even though they succeeded on the hardware.
Previous entry (from earlier experimentation) claimed PerKey lighting, a borrowed per-key layout, and 7 additional effects that were never tested against real hardware. Corrected to only the modes actually verified: Static, Breathe, RainbowCycle, Pulse.
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds GX651AR Aura support, recognizes keyboard identifier ChangesAura device support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@asusd/src/aura_laptop/trait_impls.rs`:
- Around line 182-184: Update the public set_led_mode setter to handle
self.0.set_brightness failures with the same warning-and-continue behavior
already used by set_led_mode_data, so execution reaches config.write() and
persists the mode. Prefer extracting the shared brightness-update handling into
a helper reused by both setters.
In `@asusd/src/aura_manager.rs`:
- Around line 262-268: Correct the comments near the 0b05:1ce6 selector to
consistently identify the USB interface as 1.2, replacing the incorrect 1.3 and
interface 3 references. Keep hidraw3 only as the current hidraw node
description.
- Around line 262-301: Extract the 0b05:1ce6 interface filtering and ASUS HID
parent deduplication from init_all_hid into a shared helper, then invoke it from
both init_all_hid and the hidraw add handling in new before init_hid_devices.
Ensure hotplug events skip non-Aura 1ce6 interfaces and apply the same
seen_usb_parents behavior as startup.
In `@rog-aura/src/keyboard/power.rs`:
- Line 2: Rewrite both model-ID comments with one canonical, verified product-ID
list, removing duplicate 0x1ce6 entries and inconsistent pipe/comma separators.
Ensure the list matches the IDs handled by AuraDeviceType::from and the
corresponding support entries, and clearly identifies models using the newer bit
layout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 70e85e65-3e9e-4903-9a0d-1db6d9d6e629
📒 Files selected for processing (5)
asusd/src/aura_laptop/trait_impls.rsasusd/src/aura_manager.rsrog-aura/data/aura_support.ronrog-aura/src/keyboard/power.rsrog-aura/src/lib.rs
📜 Review details
🔇 Additional comments (2)
rog-aura/src/lib.rs (1)
104-104: LGTM!rog-aura/data/aura_support.ron (1)
1163-1171: LGTM!
set_led_mode_data was fixed to not abort on brightness failures, but the sibling set_led_mode setter had the identical bug: an unhandled set_brightness()? would abort before config.write(), so mode changes via this path would not persist on backlight-less devices like the GX651AR. Applies the same warn-and-continue fix here.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@asusd/src/aura_laptop/trait_impls.rs`:
- Around line 140-142: The setters currently hold the config lock while awaiting
device operations, creating lock-order inversion with CtrlTask::create_tasks.
Update Aura::set_brightness and write_current_config_mode to copy required
config values, release config before any backlight/TUF await, then reacquire
config for state updates and config.write().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4be1be45-1fce-4ae9-a385-ed1587bcc9a3
📒 Files selected for processing (1)
asusd/src/aura_laptop/trait_impls.rs
Comments referred to interface 3 / hidraw3 from an earlier draft of this fix, but the actual code correctly targets interface 1.2. Also clarified that hidraw numbering is not stable across boots.
Earlier edit inserted 0x1ce6 multiple times with mixed pipe/comma separators. Cleaned up to a single, consistent comma-separated list.
The interface-1.2 filter previously only ran during initial device enumeration. USB reconnect/resume events go through a separate hotplug handler that called init_hid_devices directly, so a disconnect/reconnect or resume-from-sleep could re-select the wrong interface for the Zephyrus Duo. Extracted the filter into a shared is_non_aura_1ce6_interface() helper used by both paths. Verified: unbind/rebind the USB device and confirmed asusctl aura effect still works correctly afterward with no manual intervention.
Ghoul4500
left a comment
There was a problem hiding this comment.
This looks correct to me. Thank you for your contribution
Description
The ASUS ROG Zephyrus Duo keyboard (USB ID 0b05:1ce6) had two separate
bugs preventing RGB/Aura control from working.
Bug 1: Wrong HID interface selected
0b05:1ce6 is a composite HID device exposing multiple interfaces.
DeviceManager was selecting an interface that reuses report ID 0x5d
for unrelated Input-only data (keyboard media keys), rather than the
actual Aura control interface (Output/Feature capable per the HID
report descriptor), which is USB interface 1.2. This caused RGB
commands to silently fail.
Bug 2: Effect writes silently failed to persist on backlight-less devices
This device has no sysfs keyboard backlight class.
set_led_mode_datacalled
set_brightness()?immediately after a successful color write;since that call always fails on this hardware, the
?aborted therest of the function -- including
config.write(). The color changewas sent to the keyboard and visible immediately, but was never saved,
and asusctl reported an error even though the write had succeeded. On
reboot, the keyboard would revert to whatever was last saved, making
Aura control look completely broken even though the underlying HID
write worked correctly.
Fix: brightness failures are now logged as a warning instead of
aborting the effect-setting flow, so color/effect changes always
persist regardless of whether backlight control is available.
Related to #264
Tested Hardware & Environment
Verification and testing:
cargo fmt --all -- --check)cargo clippy --all -- -D warnings/cargo check --all-targets)cargo test --all)cargo cranky)Verified working via
asusctl aura effect, confirmed on real hardware:static -c <colour>,breathe --colour <c1> --colour2 <c2> --speed <speed>,rainbow-cycle --speed <speed>,pulse -c <colour>-- keyboardchanges color/effect immediately
setting a color, rebooting with no manual intervention, and
confirming both the keyboard and
/etc/asusd/aura_*.roncorrectlyretained the value
RainbowWavewas tested and did not produce a visible effect onthis hardware, so it was intentionally left out of
basic_modesNotes
layout_nameis currently borrowed fromga401q(single-zone default)since this device doesn't have a dedicated per-key layout file yet.
Keyboard brightness control itself is still not supported on this
hardware (no sysfs LED backlight class) -- querying/setting brightness
specifically will still error, which is expected and unrelated to this
fix. What's fixed is that this limitation no longer breaks color/effect
control or its persistence.