Conversation
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds transient error-throttling and StatusMessage relocation in the PWM controller; bumps two firmware latest revisions in SystemUpgrade.vue; consolidates bootloader memory type constants into a BTL_MEMORY mapping and uses it in usbMcmLin.js. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/views/pwm/PwmController.vue (1)
34-39:⚠️ Potential issue | 🟠 Major
errorCountcountdown never runs whenonMountedsetup fails — error message persists forever.
pollTimer = setInterval(updateStatus, 250)is inside thetryblock (Line 34) and is only reached when the entire setup succeeds. IfenableSlavePower()orsetup()throws, execution jumps to thecatchat Line 35, which setserrorCount = 10and shows the error message — butpollTimerremainsnull, soupdateStatusis never scheduled. The decrement/clear logic at Lines 90–95 never executes, and the error banner stays on screen until the component is unmounted.Move
pollTimerinitialization out of thetryblock so the countdown runs regardless of whether setup succeeded:🐛 Proposed fix
onMounted(async () => { try { await mcm.enableSlavePower(); await mcm.setup(); await updateDutyCycle(); await updateFrequency(); await updateStatus(); - - pollTimer = setInterval(updateStatus, 250); } catch (error) { isErrorMsg.value = true; errorMsg.value = error.message; errorCount = 10; } + + pollTimer = setInterval(updateStatus, 250); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/pwm/PwmController.vue` around lines 34 - 39, The pollTimer assignment is inside the try so if enableSlavePower() or setup() throws the interval never starts and the error banner never auto-clears; move the pollTimer = setInterval(updateStatus, 250) call out of the try/catch (e.g., start it before calling enableSlavePower()/setup() or immediately in onMounted regardless of success) so updateStatus runs even on setup failure and the errorCount decrement/clear logic (which uses updateStatus) can run; ensure pollTimer is declared in the same scope as updateStatus and cleared appropriately when errorCount hits zero or on unmount.
🧹 Nitpick comments (1)
src/views/pwm/PwmController.vue (1)
90-95:elseclears error state on every poll tick even when no error is active.When
errorCount === 0(i.e., normal operation), theelsebranch unconditionally writesisErrorMsg.value = falseanderrorMsg.value = ''every 250 ms. Vue 3 short-circuits on same-value primitive writes so this is a no-op for rendering, but the intent is cleaner as a one-time transition when the counter reaches zero:♻️ Proposed refactor
- if (errorCount > 0) { - errorCount -= 1; - } else { - isErrorMsg.value = false; - errorMsg.value = ''; - } + if (errorCount > 0) { + errorCount -= 1; + if (errorCount === 0) { + isErrorMsg.value = false; + errorMsg.value = ''; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/pwm/PwmController.vue` around lines 90 - 95, The else branch is clearing isErrorMsg.value and errorMsg.value on every poll tick even when nothing changed; update the logic around errorCount in the polling handler so you only clear the error state once when the counter transitions to zero (e.g., decrement errorCount when > 0, and if that decrement results in 0 then set isErrorMsg.value = false and errorMsg.value = ''), referencing the existing errorCount, isErrorMsg.value and errorMsg.value symbols to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/views/pwm/PwmController.vue`:
- Around line 34-39: The pollTimer assignment is inside the try so if
enableSlavePower() or setup() throws the interval never starts and the error
banner never auto-clears; move the pollTimer = setInterval(updateStatus, 250)
call out of the try/catch (e.g., start it before calling
enableSlavePower()/setup() or immediately in onMounted regardless of success) so
updateStatus runs even on setup failure and the errorCount decrement/clear logic
(which uses updateStatus) can run; ensure pollTimer is declared in the same
scope as updateStatus and cleared appropriately when errorCount hits zero or on
unmount.
---
Nitpick comments:
In `@src/views/pwm/PwmController.vue`:
- Around line 90-95: The else branch is clearing isErrorMsg.value and
errorMsg.value on every poll tick even when nothing changed; update the logic
around errorCount in the polling handler so you only clear the error state once
when the counter transitions to zero (e.g., decrement errorCount when > 0, and
if that decrement results in 0 then set isErrorMsg.value = false and
errorMsg.value = ''), referencing the existing errorCount, isErrorMsg.value and
errorMsg.value symbols to locate and modify the code.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
public/mcm-81339-v0-28-0.binis excluded by!**/*.binpublic/mcm-81339-v0-29-0.binis excluded by!**/*.binpublic/mcm-81349-v0-28-0.binis excluded by!**/*.binpublic/mcm-81349-v0-29-0.binis excluded by!**/*.bin
📒 Files selected for processing (2)
src/views/pwm/PwmController.vuesrc/views/system/SystemUpgrade.vue
Coderabitai suggestion to write once
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/views/pwm/PwmController.vue (1)
25-39:⚠️ Potential issue | 🟠 MajorError message never clears when
onMountedsetup failsWhen
mcm.enableSlavePower()ormcm.setup()throws, thecatchblock setserrorCount = 10, butpollTimer = setInterval(updateStatus, 250)at line 34 is inside thetryblock and is never reached.updateStatus— which houses the countdown logic — is therefore never scheduled, soerrorCountstays at 10 and the error message is displayed indefinitely. This directly contradicts the PR's stated goal of clearing errors after a timeout.The simplest consistent fix is to move the
setIntervalcall outside thetry/catchso it always starts;updateStatus's polling errors are already silently swallowed, and the countdown will then work identically for all three error sources:🐛 Proposed fix
onMounted(async () => { try { await mcm.enableSlavePower(); await mcm.setup(); await updateDutyCycle(); await updateFrequency(); await updateStatus(); - - pollTimer = setInterval(updateStatus, 250); } catch (error) { isErrorMsg.value = true; errorMsg.value = error.message; errorCount = 10; } + + pollTimer = setInterval(updateStatus, 250); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/pwm/PwmController.vue` around lines 25 - 39, The onMounted try/catch currently prevents scheduling the status poll when mcm.enableSlavePower() or mcm.setup() throws, so move the pollTimer = setInterval(updateStatus, 250) call outside (after) the try/catch block so updateStatus always runs and can decrement errorCount and clear isErrorMsg/errorMsg; keep the try/catch to set isErrorMsg.value, errorMsg.value and errorCount = 10 on failure, and ensure pollTimer, updateStatus, and setInterval are the same symbols used so the countdown logic runs regardless of setup success.
🧹 Nitpick comments (1)
src/views/pwm/PwmController.vue (1)
14-14: Extract the magic number10into a named constant
errorCount = 10appears three times (lines 38, 56, 66). Extracting it to a named constant makes the intended timeout duration self-documenting and the only place to update if the poll interval changes.♻️ Proposed refactor
+const ERROR_DISPLAY_POLLS = 10; // 10 × 250 ms = 2.5 s let errorCount = 0;- errorCount = 10; + errorCount = ERROR_DISPLAY_POLLS;(apply the same substitution in
updateDutyCycleandupdateFrequencycatch blocks)Also applies to: 38-38, 56-56, 66-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/pwm/PwmController.vue` at line 14, Extract the magic number 10 used to reset errorCount into a named constant (e.g., MAX_POLL_ERRORS or POLL_ERROR_THRESHOLD) at the top of the module and replace all occurrences where errorCount is set to 10; update the references in the catch blocks of fetch/poll logic and in the functions updateDutyCycle and updateFrequency to use that constant instead of the literal 10 so the threshold is self-documenting and centralized for future changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/views/pwm/PwmController.vue`:
- Around line 25-39: The onMounted try/catch currently prevents scheduling the
status poll when mcm.enableSlavePower() or mcm.setup() throws, so move the
pollTimer = setInterval(updateStatus, 250) call outside (after) the try/catch
block so updateStatus always runs and can decrement errorCount and clear
isErrorMsg/errorMsg; keep the try/catch to set isErrorMsg.value, errorMsg.value
and errorCount = 10 on failure, and ensure pollTimer, updateStatus, and
setInterval are the same symbols used so the countdown logic runs regardless of
setup success.
---
Nitpick comments:
In `@src/views/pwm/PwmController.vue`:
- Line 14: Extract the magic number 10 used to reset errorCount into a named
constant (e.g., MAX_POLL_ERRORS or POLL_ERROR_THRESHOLD) at the top of the
module and replace all occurrences where errorCount is set to 10; update the
references in the catch blocks of fetch/poll logic and in the functions
updateDutyCycle and updateFrequency to use that constant instead of the literal
10 so the threshold is self-documenting and centralized for future changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/js/usbMcmLin.js`:
- Around line 4-8: BTL_MEMORY currently maps strings to numeric codes but
invalid or mis-cased `memory` values (e.g., 'Flash_CS') will produce undefined
which coerces to 0 when written into a Uint8Array, silently selecting nvram; fix
by normalizing and validating the lookup: when you compute the mapped value (use
a local like `const memCode = BTL_MEMORY[memory.toLowerCase()]`), check that
memCode is a valid number (e.g., typeof memCode === 'number' and not NaN) before
assembling the payload or writing into the Uint8Array, and if invalid throw or
return an explicit error; update all usages (including the payload assembly site
that writes the memory code into the buffer) to use this guarded `memCode`
variable.
In `@src/views/pwm/PwmController.vue`:
- Around line 36-40: The catch block in the onMounted setup sets
isErrorMsg.value/errorMsg.value and errorCount = ERROR_DISPLAY_POLLS but never
schedules updateStatus because pollTimer is only assigned at the end of the try;
fix by ensuring the countdown runs on error: either (A) in the catch, start a
one-shot timer (e.g., setTimeout) that calls updateStatus (or decrements
errorCount) after the same interval used by pollTimer so the error banner will
clear, or (B) assign pollTimer inside the catch to the same recurring timer used
in the try so updateStatus continues running; update the catch to reference
pollTimer, updateStatus, isErrorMsg, errorMsg, and ERROR_DISPLAY_POLLS
accordingly.
1 participant
Upgrade 813x9 fw to v0.29.0, clear error after timeout
Summary by CodeRabbit
Bug Fixes
Chores
New Features