🆕 Add connection timer #4562
🆕 Add connection timer #4562
Conversation
|
Warning Rate limit exceeded@haslinghuis has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 21 seconds before requesting another review. ⌛ 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)
WalkthroughA connection time indicator feature was introduced. A new reactive Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SerialBackend
participant VueApp
participant StatusBar
User->>SerialBackend: Initiates connection
SerialBackend->>SerialBackend: Set connectionTimestamp = Date.now()
SerialBackend->>VueApp: Update CONNECTION.timestamp after 100ms
VueApp->>StatusBar: Pass connectionTimestamp as prop
StatusBar->>StatusBar: Start interval to update elapsed time
StatusBar->>User: Display formatted connection time (updates every second)
User->>SerialBackend: Disconnects
SerialBackend->>SerialBackend: Clear connectionTimestamp
SerialBackend->>VueApp: Update CONNECTION.timestamp to null after 100ms
StatusBar->>User: Hide or reset connection time display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/init.js (1)
17-21: Document the semantics of the new reactive storeA quick code comment / JSDoc block will help future maintainers understand that
timestampis expected to hold aDate.now()value (milliseconds) while a connection is active andnullwhen disconnected.-// Connection tracking object -const CONNECTION = reactive({ - timestamp: null, -}); +/** + * Global reactive container for connection state. + * `timestamp` is `null` while disconnected, or a millisecond Unix-epoch + * value (`Date.now()`) set by the serial backend when a connection is opened. + */ +const CONNECTION = reactive({ + timestamp: null, +});
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
locales/en/messages.json(1 hunks)src/components/init.js(2 hunks)src/components/status-bar/StatusBar.vue(4 hunks)src/index.html(1 hunks)src/js/serial_backend.js(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: in the betaflight-configurator codebase, the virtualfc.js aux_config array must stay synchronized wi...
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.
Applied to files:
src/components/init.jssrc/js/serial_backend.js
📚 Learning: in the betaflight configurator repository, only english messages should be added directly to `locale...
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4490
File: locales/en/messages.json:1534-1545
Timestamp: 2025-05-26T11:37:09.758Z
Learning: In the Betaflight Configurator repository, only English messages should be added directly to `locales/en/messages.json`; other language translations are handled via Crowdin automation and should not be manually updated in PRs.
Applied to files:
locales/en/messages.json
📚 Learning: in webserial.js, there's a timing issue where the cached `this.ports` array doesn't immediately refl...
Learnt from: blckmn
PR: betaflight/betaflight-configurator#4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Applied to files:
src/js/serial_backend.js
📚 Learning: in the betaflight configurator, fc.config.buildkey is defaulted to an empty string when not present,...
Learnt from: DavidAnson
PR: betaflight/betaflight-configurator#4450
File: src/js/tabs/setup.js:0-0
Timestamp: 2025-05-01T05:41:03.282Z
Learning: In the Betaflight Configurator, FC.CONFIG.buildKey is defaulted to an empty string when not present, making the length check (FC.CONFIG.buildKey.length === 32) safe without additional type checking.
Applied to files:
src/js/serial_backend.js
📚 Learning: betaflight configurator only connects with local embedded devices (flight controllers), not external...
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4379
File: src-tauri/tauri.conf.json:12-14
Timestamp: 2025-06-07T23:27:01.841Z
Learning: Betaflight Configurator only connects with local embedded devices (flight controllers), not external web resources, which affects the security threat model and CSP requirements.
Applied to files:
src/js/serial_backend.js
🧬 Code Graph Analysis (1)
src/js/serial_backend.js (3)
src/js/gui_log.js (1)
gui_log(7-21)src/js/localization.js (1)
i18n(7-7)src/js/fc.js (1)
FC(130-993)
🔇 Additional comments (13)
src/components/init.js (1)
38-38: LGTM – new state plugged into the global modelAdding
CONNECTIONtobetaflightModelkeeps the pattern consistent with the other global stores and ensures reactivity in all descendants.
Nothing further to change here.locales/en/messages.json (1)
724-727: New i18n key looks correct and follows existing naming/format conventions
No further action required.src/index.html (1)
173-173: LGTM! Clean property binding for connection timer feature.The addition of
:connection-timestamp="CONNECTION.timestamp"correctly binds the global reactive connection timestamp to the StatusBar component, following Vue.js naming conventions (kebab-case in template, camelCase in JavaScript).src/js/serial_backend.js (6)
33-33: LGTM! Clean module-level variable declaration.The
connectionTimestampvariable is appropriately declared at module level to track connection state across function calls.
363-369: LGTM! Consistent timestamp handling for virtual connections.The implementation correctly sets the connection timestamp and updates the Vue component state with a reasonable delay to ensure the Vue app is mounted.
535-543: LGTM! Consistent timestamp tracking pattern.The timestamp setting in
processUidfollows the same pattern asonOpenVirtual, ensuring consistency across different connection flows (virtual vs real connections).
672-678: LGTM! Proper cleanup on connection close.The timestamp is correctly cleared and the Vue state is updated when the connection closes, ensuring the UI reflects the disconnected state.
930-932: LGTM! Useful utility function.The exported
getConnectionTimestamp()function provides clean external access to the current connection timestamp, which could be useful for other parts of the application.
95-95: Minor whitespace cleanup - LGTM!Trailing whitespace removal improves code cleanliness.
src/components/status-bar/StatusBar.vue (4)
4-7: LGTM! Clean conditional rendering with proper localization.The template correctly shows the connection time only when
connectionTimestampis present, uses proper localization with$t(), and maintains consistent styling with existing status bar elements.
22-22: LGTM! Proper Vue 3 Composition API imports.All necessary composition API functions are imported for the reactive time tracking implementation.
42-45: LGTM! Well-defined prop with appropriate type and default.The
connectionTimestampprop is properly typed asNumberwithnulldefault, which correctly handles both connected and disconnected states.
79-114: LGTM! Excellent reactive time tracking implementation.The setup function demonstrates good Vue 3 practices:
- Reactive
currentTimeupdated every second for live time display- Proper lifecycle management with interval cleanup in
onUnmounted- Clean computed property that formats elapsed time as MM:SS
- Handles edge case when
connectionTimestampis nullThe time calculation logic is correct and the formatting ensures consistent two-digit display.
haslinghuis
force-pushed
the
connection-timer
branch
from
1608e1e to
2d50dc2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
locales/en/messages.json(1 hunks)src/components/init.js(2 hunks)src/components/status-bar/StatusBar.vue(4 hunks)src/index.html(1 hunks)src/js/serial_backend.js(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- locales/en/messages.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/index.html
- src/components/init.js
- src/components/status-bar/StatusBar.vue
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: in webserial.js, there's a timing issue where the cached `this.ports` array doesn't immediately refl...
Learnt from: blckmn
PR: betaflight/betaflight-configurator#4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Applied to files:
src/js/serial_backend.js
📚 Learning: in the betaflight configurator, fc.config.buildkey is defaulted to an empty string when not present,...
Learnt from: DavidAnson
PR: betaflight/betaflight-configurator#4450
File: src/js/tabs/setup.js:0-0
Timestamp: 2025-05-01T05:41:03.282Z
Learning: In the Betaflight Configurator, FC.CONFIG.buildKey is defaulted to an empty string when not present, making the length check (FC.CONFIG.buildKey.length === 32) safe without additional type checking.
Applied to files:
src/js/serial_backend.js
🪛 Biome (2.1.2)
src/js/serial_backend.js
[error] 95-95: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 95-95: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 96-96: Expected a JSX attribute but instead found '.'.
Expected a JSX attribute here.
(parse)
[error] 96-96: Expected a JSX attribute but instead found '.'.
Expected a JSX attribute here.
(parse)
[error] 97-97: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 99-99: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 99-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 100-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 100-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 100-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 100-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 100-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 100-100: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 250-250: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 251-251: expected } but instead found if
Remove if
(parse)
[error] 252-252: expected } but instead found ;
Remove ;
(parse)
[error] 253-253: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 254-254: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 248-248: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
[error] 378-378: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 379-379: expected } but instead found if
Remove if
(parse)
[error] 380-380: expected } but instead found ;
Remove ;
(parse)
[error] 381-381: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 382-382: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 376-376: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
[error] 552-552: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 553-553: expected } but instead found if
Remove if
(parse)
[error] 554-554: expected } but instead found ;
Remove ;
(parse)
[error] 555-555: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 556-556: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 559-560: Expected a JSX Expression, a Element, or a text but instead found '<<<<<<'.
Expected a JSX Expression, a Element, or a text here.
(parse)
[error] 560-562: Expected a JSX attribute but instead found '=======
await'.
Expected a JSX attribute here.
(parse)
[error] 562-562: Expected a JSX attribute but instead found '();'.
Expected a JSX attribute here.
(parse)
[error] 563-563: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 563-563: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 563-563: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 563-563: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 563-563: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 563-563: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 550-550: Wrap comments inside children within braces.
Unsafe fix: Wrap the comments with braces
(lint/suspicious/noCommentText)
[error] 940-940: expected } but instead found return
Remove return
(parse)
[error] 940-941: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 941-941: expected < but instead the file ends
the file ends here
(parse)
🪛 GitHub Actions: Preview Deployment
src/js/serial_backend.js
[error] 99-99: Build failed: Invalid JavaScript syntax detected during import analysis. Possible unresolved merge conflict markers present in the code (lines 98-100). Rename file to .jsx or .tsx if using JSX syntax.
🔇 Additional comments (4)
src/js/serial_backend.js (4)
248-254: LGTM! Connection timestamp clearing logic is well implemented.The implementation correctly clears the connection timestamp on reset and safely updates the Vue component with appropriate null-checking and timing considerations.
376-382: LGTM! Virtual connection timestamp logic is consistent.The implementation correctly sets the connection timestamp for virtual connections using the same safe pattern as other timestamp updates.
548-556: LGTM! Connection timestamp setting in processUid is correct.The timestamp logic properly records the connection time when UID is processed, following the established pattern.
939-941: LGTM! Clean API for accessing connection timestamp.The exported getter function provides proper encapsulation and external access to the connection timestamp.
haslinghuis
requested review from
blckmn,
KarateBrot,
SupaflyFPV,
nerdCopter and
Copilot
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a connection timer feature that displays a live counter in the status bar showing how long the connection has been active. The timer updates in real-time and is visible whenever a connection is established.
Key changes:
- Connection timestamp tracking for both virtual and real connections
- Live timer display in the status bar with reactive updates
- Proper cleanup of timer resources and connection state
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/js/serial_backend.js | Adds connection timestamp tracking and Vue state updates for connection establishment and closure |
| src/index.html | Passes connection timestamp prop to the status bar component |
| src/components/status-bar/StatusBar.vue | Implements the connection timer display with reactive time formatting |
| src/components/init.js | Adds CONNECTION reactive object to global Vue state |
| locales/en/messages.json | Adds localization string for the connection time label |
|
|
Preview URL: https://6723bb10.betaflight-configurator.pages.dev |
Summary by CodeRabbit