Add controller LED color API and Blockly block#596
Add controller LED color API and Blockly block#596tracygardner wants to merge 8 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds XR controller LED color control: new async API methods to set controller LED color (including a WebHID helper), Blockly block and generator, toolbox entry, and English/Spanish localization; also exposes the API to the iframe sandbox. Changes
Sequence Diagram(s)sequenceDiagram
participant Block as Blockly Block
participant Runner as Runtime (iframe/host)
participant flockXR as flockXR API
participant XRInput as XR input sources
participant GP as navigator.getGamepads()
participant HW as Controller Hardware / WebHID
Block->>Runner: execute generated code
Runner->>flockXR: await setControllerLedColor(index, "#RRGGBB")
flockXR->>XRInput: query XR input sources
flockXR->>GP: query navigator.getGamepads()
flockXR->>flockXR: select gamepad by normalized index
flockXR->>HW: try lightIndicator.setColor / leds[0].setColor
alt supported
HW-->>flockXR: success
else unsupported
flockXR->>HW: setPlayStationControllerLedViaHid(gamepad, rgb)
HW-->>flockXR: success/failure
end
flockXR-->>Runner: return (void)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 39 minutes and 34 seconds.Comment |
Deploying flockxr with
|
| Latest commit: |
cf2b3de
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ae0d8f12.flockxr.pages.dev |
| Branch Preview URL: | https://codex-add-block-to-control-g.flockxr.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/xr.js`:
- Around line 54-57: The code in setControllerLedColor uses
navigator?.getGamepads which throws if navigator is undeclared; change the guard
to check existence with typeof navigator !== "undefined" (e.g., typeof navigator
!== "undefined" && typeof navigator.getGamepads === "function") before calling
getGamepads so the fallback to [] runs safely; update the gamepads assignment
and the retrieval of gamepad (gamepads?.[Math.trunc(Number(controllerIndex))])
accordingly to ensure no ReferenceError in non-browser environments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e33c9714-ac55-48cf-8ab3-1e21928ba465
📒 Files selected for processing (6)
api/xr.jsblocks/xr.jsgenerators/generators-scene.jslocale/en.jslocale/es.jstoolbox.js
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/xr.js (1)
61-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
typeof navigator !== "undefined"guards instead ofnavigator?.…at root.Line [61] and Line [144] can still throw in environments where
navigatoris undeclared, so the fallback path is not guaranteed.🔧 Proposed fix
- const browserGamepads = Array.from(navigator?.getGamepads?.() ?? []).filter( - Boolean, - ); + const browserGamepads = Array.from( + typeof navigator !== "undefined" && + typeof navigator.getGamepads === "function" + ? navigator.getGamepads() + : [], + ).filter(Boolean);- if (!navigator?.hid) return false; + if (typeof navigator === "undefined" || !navigator.hid) return false;#!/bin/bash # Verify remaining risky optional-chaining root usages in JS files. rg -nP --type=js '\bnavigator\?\.' -C2Also applies to: 144-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/xr.js` at line 61, Replace root-level optional chaining on navigator with an explicit typeof check: update the browserGamepads initialization (the Array.from(navigator?.getGamepads?.() ?? []) expression) to first confirm typeof navigator !== "undefined" and typeof navigator.getGamepads === "function" before calling getGamepads, falling back to an empty array otherwise; apply the same pattern to the other occurrence referenced around line 144 so no call or property access uses navigator?.… at module/root scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/xr.js`:
- Around line 95-109: The hex-only validator hexToRgb in api/xr.js rejects named
colors (causing setControllerLedColor to no-op when generators send 'red');
update hexToRgb (or add a new normalizeColor helper used before calling
hexToRgb) to accept common CSS color names (at least 'red', 'green', 'blue',
'black', 'white') by mapping them to their hex equivalents, or use a small
parsing fallback that converts named colors to hex before parseInt runs, then
ensure the call site that logs "[setControllerLedColor] invalid color format"
uses the normalized value so generators/generators-scene.js can continue to pass
names without causing silent failures.
---
Duplicate comments:
In `@api/xr.js`:
- Line 61: Replace root-level optional chaining on navigator with an explicit
typeof check: update the browserGamepads initialization (the
Array.from(navigator?.getGamepads?.() ?? []) expression) to first confirm typeof
navigator !== "undefined" and typeof navigator.getGamepads === "function" before
calling getGamepads, falling back to an empty array otherwise; apply the same
pattern to the other occurrence referenced around line 144 so no call or
property access uses navigator?.… at module/root scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32c68768-87f1-471e-9f96-f33eb2fb33bd
📒 Files selected for processing (3)
api/xr.jsflock.jsgenerators/generators-scene.js
🚧 Files skipped from review as they are similar to previous changes (1)
- generators/generators-scene.js
| const match = trimmed.match(/^#?([0-9a-fA-F]{6})$/); | ||
| if (!match) return null; | ||
| const value = match[1]; | ||
| return { | ||
| r: parseInt(value.slice(0, 2), 16), | ||
| g: parseInt(value.slice(2, 4), 16), | ||
| b: parseInt(value.slice(4, 6), 16), | ||
| }; | ||
| }; | ||
|
|
||
| const rgb = hexToRgb(color); | ||
| if (!rgb) { | ||
| console.log("[setControllerLedColor] invalid color format", { color }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Generator/API default color mismatch can cause silent no-op.
This API only accepts hex at Line [95], but generators/generators-scene.js currently falls back to 'red' when COLOR is missing. That path will always hit invalid-format return at Line [106]-Line [109].
🔧 Suggested cross-file adjustment
- const color =
- javascriptGenerator.valueToCode(
- block,
- "COLOR",
- javascriptGenerator.ORDER_NONE,
- ) || "'red'";
+ const color =
+ javascriptGenerator.valueToCode(
+ block,
+ "COLOR",
+ javascriptGenerator.ORDER_NONE,
+ ) || "'#ff0000'";🧰 Tools
🪛 GitHub Actions: Prettier
[warning] Prettier reported formatting/style issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/xr.js` around lines 95 - 109, The hex-only validator hexToRgb in
api/xr.js rejects named colors (causing setControllerLedColor to no-op when
generators send 'red'); update hexToRgb (or add a new normalizeColor helper used
before calling hexToRgb) to accept common CSS color names (at least 'red',
'green', 'blue', 'black', 'white') by mapping them to their hex equivalents, or
use a small parsing fallback that converts named colors to hex before parseInt
runs, then ensure the call site that logs "[setControllerLedColor] invalid color
format" uses the normalized value so generators/generators-scene.js can continue
to pass names without causing silent failures.
1 participant
Motivation
Description
setControllerLedColor(controllerIndex, color)toapi/xr.jswhich accepts a controller index and a hex color string, converts it to RGB, and attempts to set the color using common controller properties (lightIndicator.setColororleds[0].setColor).set_controller_led_colorinblocks/xr.jsand a corresponding JavaScript generator ingenerators/generators-scene.jsthat emitssetControllerLedColor(...)calls.toolbox.jswith default shadow inputs for index and color.locale/en.jsandlocale/es.jsfor the block and tooltip text.Testing
Codex Task
Summary by CodeRabbit
New Features
Localization