Add BLE support - #46
Conversation
|
Warning Review limit reached
Next review available in: 36 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds typed Socket.IO contracts and related app/store wiring, introduces new device models and controls, and updates CI/lint/tooling plus broad quote-format normalization across the frontend. ChangesFrontend feature and typing changes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Backend
participant AppSocket
participant App
participant DevicesStore
participant DeviceNotificationsStore
participant SettingsStore
Backend-->>AppSocket: connect / device events / settingsChanged
AppSocket-->>App: optional-chained listeners
App->>DevicesStore: add/remove/clear/update device state
App->>DeviceNotificationsStore: dispatch/remove/clear notifications
App->>SettingsStore: getServerSettings()
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/helper/DeviceCommunicator.ts (1)
8-29: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRestore the
receiveUpdatessuppression around optimistic updates.
setAttribute()now emits immediately, butsrc/stores/devices.ts:updateDevice()still deep-merges incoming payloads wheneverreceiveUpdatesis true. Since nothing flips that flag off here anymore, a fast server echo can overwrite the just-set local value and cause flicker until the device catches up.🤖 Prompt for 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. In `@src/helper/DeviceCommunicator.ts` around lines 8 - 29, The optimistic update path in DeviceCommunicator.setAttribute is missing the receiveUpdates suppression, so a fast server echo can still be merged back by updateDevice in src/stores/devices.ts and overwrite the local value. Restore the temporary receiveUpdates=false/true guard around the immediate emit in setAttribute, and make sure the existing receiveUpdates check in updateDevice continues to skip incoming echoes while the optimistic local update is in flight.src/stores/health.ts (1)
62-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid stacking
healthMetricslisteners on reconnect.App.vuecallshealthStore.init(io)on every'connect', andinit()adds a newsocket.on('healthMetrics', ...)handler each time. Remove the previous listener before registering a new one, or register this handler once outside the reconnect path, to prevent duplicate chart updates and a growing listener leak.🤖 Prompt for 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. In `@src/stores/health.ts` around lines 62 - 94, The health metrics socket handler is being registered repeatedly because init() adds a new socket.on('healthMetrics', ...) listener every time App.vue reconnects. Update init(socket) in health store to either remove any existing healthMetrics listener before attaching a new one, or make the listener registration happen only once so state.value and chartData updates do not fire multiple times after reconnects. Use the init function and the healthMetrics event handler as the main touchpoints.
🧹 Nitpick comments (2)
src/components/device/DebouncedTextField.vue (1)
41-64: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNo cleanup of
debounceTimeron unmount.If the component unmounts while a debounce timer is pending,
emitValuefires after unmount (harmless no-op, but wasteful and inconsistent with clean teardown). Consider clearing the timer inonBeforeUnmount.🤖 Prompt for 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. In `@src/components/device/DebouncedTextField.vue` around lines 41 - 64, The DebouncedTextField component leaves a pending debounce timeout alive after unmount, so clear it during teardown to avoid `emitValue` running after the component is destroyed. Update the `DebouncedTextField.vue` logic around `debounceTimer` by adding cleanup in `onBeforeUnmount`, and make sure any pending timer is cancelled alongside the existing timeout handling in the debounced emit flow.src/components/automation/MonacoEditor.vue (1)
61-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
DeviceEventType/DeviceEventlook unused after theonEventoverload rework.The new
onEventoverloads passDevice(andDevice, DeviceNotification) directly rather than aDeviceEventwrapper, leavingDeviceEventType/DeviceEventas dead ambient types in the injectedfacts.d.tsfor automation scripts. Low impact (editor-only typings) but worth pruning for clarity.🤖 Prompt for 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. In `@src/components/automation/MonacoEditor.vue` around lines 61 - 96, Prune the dead ambient event typings in MonacoEditor.vue by removing the unused DeviceEventType and DeviceEvent declarations from the injected facts.d.ts block, since onEvent now passes Device directly (and Device, DeviceNotification for notifications). Keep the remaining DeviceAttribute and onEvent overloads intact, and make sure only the symbols still referenced by automation scripts remain declared.
🤖 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 `@src/App.vue`:
- Around line 36-39: The disconnect handler in App.vue clears devicesStore but
leaves deviceNotificationsStore state behind, so stale notifications can survive
reconnects. Update the io?.on('disconnect') callback to also reset
deviceNotificationsStore, using the new clear/remove action proposed in
deviceNotificationsStore so its latest map is emptied alongside
devicesStore.clear().
- Line 34: The call to healthStore.init in App.vue is passing a possibly null
AppSocket, which does not match the expected non-null type. Narrow io before
invoking healthStore.init, or use a non-null assertion only if the socket is
guaranteed to exist at that point, and make sure the init call in the App
component only runs with a valid AppSocket instance.
In `@src/components/automation/CreateForm.vue`:
- Around line 14-29: The script name validation in scriptNameRules is rejecting
exactly 64-character names because the length check uses a strict less-than
comparison. Update the length rule in CreateForm.vue so the script name
validator allows values up to and including 64 characters, keeping the existing
error message and the rest of the rules unchanged.
In `@src/components/device/control/DeviceAiroticControl.vue`:
- Around line 41-52: The Cancel flow in DeviceAiroticControl.vue only closes the
menus and leaves the live v-model state in localRestColor and localBreathInColor
unchanged. Update the cancel/dismiss handling for the restColorMenu and
breathInColorMenu dialogs to restore those refs from the device’s current color
before setting the menu closed, so reopening the picker shows the actual device
value. Apply the same reset behavior in the related rest/breath-in color
handlers and keep applyRestColor/applyBreathInColor using the refreshed local
refs.
- Around line 157-163: The color trigger blocks in DeviceAiroticControl.vue are
click-only divs, so make the rest color and breath-in triggers keyboard
accessible by treating them as buttons. Add button semantics and focusability to
the trigger containers, and wire keyboard activation (Enter/Space) to the same
handlers that set restColorMenu or the breath-in dialog state; reuse the
existing trigger markup around restColorCss and the breath-in equivalent so both
behave consistently.
- Around line 23-27: The parseColor helper in DeviceAiroticControl.vue does not
handle malformed color strings because Number conversion can produce NaN and the
current nullish fallback will not replace it. Update parseColor to validate each
parsed component after splitting the value, and default any non-finite or
missing channel to 0 before returning the { r, g, b } object. Keep the fix
localized to parseColor so callers that rely on its output never receive rgb
values with NaN components.
- Around line 88-95: The `watch(latestNotification, ...)` in
`DeviceAiroticControl` only updates `breathState` on future notifications, so it
can be stale on mount if the store already contains a matching `colorChange`
event. Update the component to initialize `breathState` from the current
`latestNotification` (or use an immediate watch) so the existing notification
state is applied as soon as the control panel loads, while keeping the current
`colorChange`/`breathInColor` mapping logic.
In `@src/components/device/control/virtual/DeviceVirtualTtsControl.vue`:
- Around line 57-64: Restore the dynamic queuing label in
DeviceVirtualTtsControl.vue by using the existing queuing state instead of the
hardcoded switch text. Update the v-switch label to reflect
props.device.attributes.queuing.value via the removed queuingLabel computed
logic (or an equivalent computed/helper), and keep changeQueuing tied to the
same switch so the label always matches the current enabled/disabled state.
In `@src/components/device/DebouncedTextField.vue`:
- Around line 17-19: The value emitted from DebouncedTextField remains a string
for number inputs, which causes downstream device attribute updates to receive
the wrong type. Update emitValue() in DebouncedTextField.vue to detect
props.type === 'number' and coerce the value to a number before calling
emit('update:modelValue', ...). Keep the change localized to the
DebouncedTextField component so GenericDeviceControl.vue continues to receive
correctly typed int/float values.
In `@src/shims-vue.d.ts`:
- Around line 1-8: The $socket type is declared inconsistently as Socket/typeof
Socket while the plugin actually provides an AppSocket instance, and the
augmentation is split across multiple declaration files. Update the
ComponentCustomProperties.$socket declaration to use AppSocket and consolidate
the Vue module augmentation into a single place, referencing the existing
AppSocket and ComponentCustomProperties symbols so this.$socket exposes the
correct event contracts everywhere.
In `@src/stores/deviceNotifications.ts`:
- Around line 16-33: The deviceNotifications store has no way to remove stale
entries, so add a cleanup action in useDeviceNotificationsStore (such as clear()
and/or remove(deviceId)) alongside dispatch and getLatest, and make App.vue call
it from the disconnect/deviceDisconnected flow just like devicesStore.clear().
Ensure the new action deletes outdated latest records so getLatest() cannot
return notifications for disconnected devices or future deviceId re-use.
In `@src/utils/utils.ts`:
- Around line 55-57: `isStringDeviceAttribute` is misnamed and has an incorrect
type predicate because it returns true for `float` and `int` attributes as well
as `str`. Update the guard in `utils.ts` so `isStringDeviceAttribute` only
matches the `str` type, or otherwise rename/refactor it to reflect the broader
intent if it is meant to cover all value-bearing attributes. Keep the predicate
aligned with its declared return type `StrDeviceAttribute` and the callers in
`GenericDeviceControl.vue`.
In `@src/views/SettingsView.vue`:
- Around line 45-58: The SettingsView.vue editor setup in storeEditorInstance()
adds a global window resize handler but never cleans it up, which can leave
stale listeners active after the view is left. Keep a reference to the resize
callback used in the editor.layout logic, and register a matching removal in
onBeforeUnmount() so the handler is unregistered when the component unmounts.
Make sure the cleanup is tied to the same handler instance created in
storeEditorInstance() to avoid stacking listeners on repeated visits.
---
Outside diff comments:
In `@src/helper/DeviceCommunicator.ts`:
- Around line 8-29: The optimistic update path in
DeviceCommunicator.setAttribute is missing the receiveUpdates suppression, so a
fast server echo can still be merged back by updateDevice in
src/stores/devices.ts and overwrite the local value. Restore the temporary
receiveUpdates=false/true guard around the immediate emit in setAttribute, and
make sure the existing receiveUpdates check in updateDevice continues to skip
incoming echoes while the optimistic local update is in flight.
In `@src/stores/health.ts`:
- Around line 62-94: The health metrics socket handler is being registered
repeatedly because init() adds a new socket.on('healthMetrics', ...) listener
every time App.vue reconnects. Update init(socket) in health store to either
remove any existing healthMetrics listener before attaching a new one, or make
the listener registration happen only once so state.value and chartData updates
do not fire multiple times after reconnects. Use the init function and the
healthMetrics event handler as the main touchpoints.
---
Nitpick comments:
In `@src/components/automation/MonacoEditor.vue`:
- Around line 61-96: Prune the dead ambient event typings in MonacoEditor.vue by
removing the unused DeviceEventType and DeviceEvent declarations from the
injected facts.d.ts block, since onEvent now passes Device directly (and Device,
DeviceNotification for notifications). Keep the remaining DeviceAttribute and
onEvent overloads intact, and make sure only the symbols still referenced by
automation scripts remain declared.
In `@src/components/device/DebouncedTextField.vue`:
- Around line 41-64: The DebouncedTextField component leaves a pending debounce
timeout alive after unmount, so clear it during teardown to avoid `emitValue`
running after the component is destroyed. Update the `DebouncedTextField.vue`
logic around `debounceTimer` by adding cleanup in `onBeforeUnmount`, and make
sure any pending timer is cancelled alongside the existing timeout handling in
the debounced emit flow.
🪄 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: 7e43deb1-95a6-4ba2-ba45-051667d3e8fb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (81)
.github/workflows/release.yml.github/workflows/test.ymlcypress/integration/example.spec.tscypress/support/index.tseslint.config.tspackage.jsonsrc/App.vuesrc/components/DeviceInfo.vuesrc/components/ServerStatusOverlay.vuesrc/components/__tests__/HelloWorld.spec.tssrc/components/automation/CreateForm.vuesrc/components/automation/LogViewer.vuesrc/components/automation/MonacoEditor.vuesrc/components/chart/StreamLineChart.vuesrc/components/device/DebouncedSlider.vuesrc/components/device/DebouncedTextField.vuesrc/components/device/DeviceCard.vuesrc/components/device/DeviceControl.vuesrc/components/device/control/DeviceAiroticControl.vuesrc/components/device/control/DeviceEstim2bControl.vuesrc/components/device/control/DeviceZc95Control.vuesrc/components/device/control/GenericDeviceControl.vuesrc/components/device/control/slvctrlplus/DeviceAirValveControl.vuesrc/components/device/control/slvctrlplus/DeviceDisplayControl.vuesrc/components/device/control/slvctrlplus/DeviceDistanceControl.vuesrc/components/device/control/slvctrlplus/DeviceEt312Control.vuesrc/components/device/control/slvctrlplus/DeviceNogasmControl.vuesrc/components/device/control/slvctrlplus/DeviceStrikerMk2Control.vuesrc/components/device/control/virtual/DeviceVirtualDisplayControl.vuesrc/components/device/control/virtual/DeviceVirtualPiperTtsControl.vuesrc/components/device/control/virtual/DeviceVirtualRandomGeneratorControl.vuesrc/components/device/control/virtual/DeviceVirtualTtsControl.vuesrc/components/icons/DeviceIcon.vuesrc/helper/ChartHelper.tssrc/helper/DeviceCommunicator.tssrc/helper/TimeoutHelper.tssrc/layouts/LayoutWithMenu.vuesrc/main.tssrc/model/MapRule.tssrc/model/devices/Device.tssrc/model/devices/airotic/DeviceAirotic.tssrc/model/devices/estim2b/DeviceEstim2b.tssrc/model/devices/slvctrl/DeviceAirValve.tssrc/model/devices/slvctrl/DeviceDisplay.tssrc/model/devices/slvctrl/DeviceDistance.tssrc/model/devices/slvctrl/DeviceEt312.tssrc/model/devices/slvctrl/DeviceNogasm.tssrc/model/devices/slvctrl/DeviceStrikerMk2.tssrc/model/devices/virtual/VirtualDeviceDisplay.tssrc/model/devices/virtual/VirtualDevicePiperTts.tssrc/model/devices/virtual/VirtualDeviceTts.tssrc/model/devices/virtual/VirtualRandomGenerator.tssrc/model/devices/zc95/DeviceZc95.tssrc/plugins/vueSocketIOClient.tssrc/plugins/vuetify.tssrc/plugins/webfontloader.tssrc/router/index.tssrc/shims-vue.d.tssrc/shims-vuetify.d.tssrc/stores/app.tssrc/stores/automation.tssrc/stores/backend.tssrc/stores/deviceNotifications.tssrc/stores/devices.tssrc/stores/health.tssrc/stores/settings.tssrc/types/socket.tssrc/types/utility.tssrc/utils/apiFetch.tssrc/utils/utils.tssrc/views/AutomationView.vuesrc/views/BackendUrl.vuesrc/views/DevicesView.vuesrc/views/MissionControlSingleView.vuesrc/views/MissionControlView.vuesrc/views/SettingsView.vuesrc/views/SystemHealthView.vuesrc/vueSocketIOClient.d.tstsconfig.app.jsonvite.config.tsvue.config.js
💤 Files with no reviewable changes (2)
- src/helper/TimeoutHelper.ts
- tsconfig.app.json
Summary by CodeRabbit
New Features
Bug Fixes
Chores