New extended data for usage report#5465
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughreportUpgradeEvent now reuses the passed Changes
Sequence Diagram(s)sequenceDiagram
participant Code as reportUpgradeEvent
participant Device as Local Device API (/json/cfg)
participant Server as usage.wled.me
Code->>Device: GET /json/cfg
Device-->>Code: cfg JSON
Code->>Code: assemble upgradeData (info + cfg, map bus types & IR)
Code->>Server: POST /api/usage/upgrade (upgradeData)
Server-->>Code: 200 / error
alt success
Code->>Code: updateVersionInfo(...) + success toast
else failure
Code->>Code: show failure toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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. Comment |
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 `@wled00/data/index.js`:
- Around line 3416-3427: The IR_TYPES object in wled00/data/index.js is
misaligned with the ir.cpp remote type enum; update IR_TYPES so its keys match
ir.cpp: keep 0 as null, then set 1 -> "24-key", 2 -> "24-key-v2" (CW/WW/CT
control), 3 -> "40-key", 4 -> "44-key", 5 -> "21-key", 6 -> "6-key", 7 ->
"9-key", and 8 -> "json-remote" and remove the unused 9 entry so reported
telemetry matches the ir.cpp switch cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…s, peripherals, and integrations
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/data/index.js (1)
3525-3528:⚠️ Potential issue | 🟠 MajorDon't mark failed reports as handled.
This shared
catchalso covers the fresh/json/infoand/json/cfgreads. CallingupdateVersionInfo(...)here suppresses future prompts even when nothing reached the telemetry API, which also contradicts Line 3522. Only persist the new version after a successful POST or an explicit user opt-out.Possible fix
.catch(e => { console.log('Failed to report upgrade', e); showToast('Report failed', true); - updateVersionInfo(info.ver, false, !!alwaysReport); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/index.js` around lines 3525 - 3528, The catch handler that logs 'Failed to report upgrade' currently calls updateVersionInfo(info.ver, false, !!alwaysReport) which incorrectly marks the version as handled even when telemetry POST or subsequent /json/info and /json/cfg reads failed; modify the error path in the promise chain around the POST (the .catch(e => { ... }) that contains showToast and console.log) to remove the updateVersionInfo(...) call so it is only invoked after a successful POST response or when the user explicitly opts out, or alternatively add a success flag and call updateVersionInfo(info.ver, false, !!alwaysReport) only inside the successful POST response handler (not in the shared catch) to avoid suppressing future prompts; ensure references to updateVersionInfo, showToast and the failing .catch are updated accordingly.
♻️ Duplicate comments (1)
wled00/data/index.js (1)
3416-3427:⚠️ Potential issue | 🟠 MajorFix the
IR_TYPESlookup; it's still shifted by one.
cfgData.hw?.ir?.typeis used verbatim here, so every non-zero IR remote is reported under the wrong name today.json-remotealso belongs on code8, not9.Possible fix
const IR_TYPES = { - 1: "3-key", - 2: "24-key", - 3: "24-key-v2", - 4: "40-key", - 5: "44-key", - 6: "21-key", - 7: "6-key", - 8: "9-key", - 9: "json-remote", + 1: "24-key", + 2: "24-key-v2", + 3: "40-key", + 4: "44-key", + 5: "21-key", + 6: "6-key", + 7: "9-key", + 8: "json-remote", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/data/index.js` around lines 3416 - 3427, The IR_TYPES lookup is misaligned with cfgData.hw?.ir?.type so non-zero IR types are reported wrong; update IR_TYPES (the constant named IR_TYPES) so the values align with their numeric codes — specifically move "json-remote" to key 8 and ensure "9-key" is at key 9 (i.e., correct the shifted mapping so cfgData.hw?.ir?.type indexes the right label).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/data/index.js`:
- Around line 3470-3478: The peripherals array collapses button topology to a
boolean via .some(b => b.type !== 0); instead compute the actual physical button
count from cfgData.hw?.btn?.ins (e.g. const buttonCount = (cfgData.hw?.btn?.ins
?? []).filter(b => b.type !== 0).length), use buttonCount > 0 to include
"buttons" in the peripherals list, and expose the count as a separate field
(e.g. buttonCount or buttons_count) in the payload so single-button and
multi-button installs remain distinguishable; update references to
cfgData.hw?.btn?.ins and remove the .some(...) usage.
- Around line 3429-3434: Replace the parallel Promise.all fetches with a
serialized flow: use the existing info argument if present (the local info
variable/parameter) or fetch getURL('/json/info') first, await its JSON, then
fetch getURL('/json/cfg') and await its JSON; remove Promise.all usage (and any
references to infoData/cfgData from the parallel result) and instead use
sequential variables (e.g., info and cfg) so the /json/cfg request only runs
after /json/info completes to avoid overlapping startup requests.
- Around line 3458-3467: The gamma feature is being detected by comparing
cfgData.light?.gc?.col > 1.0 which treats the boolean as numeric and prevents
"gamma" from being reported; update the ledFeatures construction (the array
assigned to ledFeatures) to treat gamma flags as booleans — check
cfgData.light?.gc?.col || cfgData.light?.gc?.bri (or similar boolean fields like
gammaCorrectCol/gammaCorrectBri) instead of numeric comparison so that "gamma"
is included when either gamma correction flag is true.
---
Outside diff comments:
In `@wled00/data/index.js`:
- Around line 3525-3528: The catch handler that logs 'Failed to report upgrade'
currently calls updateVersionInfo(info.ver, false, !!alwaysReport) which
incorrectly marks the version as handled even when telemetry POST or subsequent
/json/info and /json/cfg reads failed; modify the error path in the promise
chain around the POST (the .catch(e => { ... }) that contains showToast and
console.log) to remove the updateVersionInfo(...) call so it is only invoked
after a successful POST response or when the user explicitly opts out, or
alternatively add a success flag and call updateVersionInfo(info.ver, false,
!!alwaysReport) only inside the successful POST response handler (not in the
shared catch) to avoid suppressing future prompts; ensure references to
updateVersionInfo, showToast and the failing .catch are updated accordingly.
---
Duplicate comments:
In `@wled00/data/index.js`:
- Around line 3416-3427: The IR_TYPES lookup is misaligned with
cfgData.hw?.ir?.type so non-zero IR types are reported wrong; update IR_TYPES
(the constant named IR_TYPES) so the values align with their numeric codes —
specifically move "json-remote" to key 8 and ensure "9-key" is at key 9 (i.e.,
correct the shifted mapping so cfgData.hw?.ir?.type indexes the right label).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
New extended data for usage report
Fixes #5444
Summary by CodeRabbit
New Features
Bug Fixes