Changes in UI and 3 different protocols-USB, WIFI, BLE added #40
Conversation
…ond) then close the window.
WalkthroughThis update introduces a major refactor and modularization of the bio-signal acquisition and streaming system. The legacy subprocess-based and stateful connection management for USB, WiFi, and BLE devices is replaced by a unified, asynchronous, and object-oriented approach. New modules ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI
participant FlaskAPI
participant ConnectionManager
participant Device (USB/WiFi/BLE)
participant LSL
participant AppScript
User->>WebUI: Click protocol button (USB/WiFi/BLE)
WebUI->>FlaskAPI: /set_csv (CSV toggle state)
WebUI->>FlaskAPI: /connect or /connect_ble (with params)
FlaskAPI->>ConnectionManager: connect_usb/connect_wifi/connect_ble
ConnectionManager->>Device: Establish connection, start data stream
Device-->>ConnectionManager: Stream samples
ConnectionManager->>LSL: Push samples to LSL
ConnectionManager->>FlaskAPI: Connection status
FlaskAPI->>WebUI: JSON response (success/failure)
WebUI->>WebUI: Enable app buttons, update status
User->>WebUI: Click app button
WebUI->>FlaskAPI: /run_application (app name)
FlaskAPI->>AppScript: Start app subprocess
AppScript-->>FlaskAPI: Running status
FlaskAPI->>WebUI: JSON response (app running)
User->>WebUI: Click disconnect
WebUI->>FlaskAPI: /stop_application (all apps), /disconnect
FlaskAPI->>ConnectionManager: Cleanup connections
ConnectionManager->>Device: Stop stream, cleanup
ConnectionManager->>LSL: Close outlet
FlaskAPI->>WebUI: JSON response (disconnected)
Possibly related PRs
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (16)
emgenvelope.py (1)
132-138: Timer stops but inlet is left open – consider explicit cleanupAfter detecting a disconnection you stop the
QTimerand close the window, but the LSLStreamInletobject remains open.
Although closing the window ends the process in__main__, if this widget were embedded inside a larger Qt application the inlet would keep running in the background.Add an explicit
self.inlet.close_stream()(or set it toNone) before shutting down to release the socket promptly.gui.py (1)
36-43: UI shuts down gracefully, but a tiny race remains
timer.stop()andwin.close()are executed from the timer thread-context.
Ifplot_lsl_data()has already returned itsappand the main thread is about to callapp.exec_(), a rapid disconnect could close the window before the event loop starts, leading to:RuntimeError: wrapped C/C++ object of type QWidget has been deletedSafest pattern is to emit a Qt signal or use
QtCore.QTimer.singleShot(0, win.close)instead of closing synchronously from the timer callback.This is optional but removes a rare crash on flaky networks.
chords_ble.py (1)
95-99:print_rate()resets the counter but never prints — dead codeThe coroutine continuously sleeps and sets
self.samples_received = 0, but the value is neither read nor logged, so sample-rate statistics are lost.
Either add aprint()(or better, a proper logger) or drop the task to avoid unnecessary context-switches.- await asyncio.sleep(1) - self.samples_received = 0 + await asyncio.sleep(1) + print(f"Samples/s: {self.samples_received}") + self.samples_received = 0chords_wifi.py (4)
5-5: Remove unused SciPy imports
butterandfiltfiltare never referenced in this module, so the extra dependency only increases install size and import time.-from scipy.signal import butter, filtfilt🧰 Tools
🪛 Ruff (0.8.2)
5-5:
scipy.signal.butterimported but unusedRemove unused import
(F401)
5-5:
scipy.signal.filtfiltimported but unusedRemove unused import
(F401)
66-74: Computed throughput metrics are unused
sps,fps, andbpsare calculated every second but never consumed. Either log them (handy for debugging) or remove the dead code to save a few µs per loop.- sps = self.calculate_rate(self.sample_size, elapsed_time) - fps = self.calculate_rate(self.packet_size, elapsed_time) - bps = self.calculate_rate(self.data_size, elapsed_time) + # Uncomment for live-rate diagnostics + # print(f"SPS={self.calculate_rate(self.sample_size, elapsed_time):.0f}, " + # f"FPS={self.calculate_rate(self.packet_size, elapsed_time):.0f}, " + # f"BPS={self.calculate_rate(self.data_size, elapsed_time):.0f}")🧰 Tools
🪛 Ruff (0.8.2)
67-67: Local variable
spsis assigned to but never usedRemove assignment to unused variable
sps(F841)
68-68: Local variable
fpsis assigned to but never usedRemove assignment to unused variable
fps(F841)
69-69: Local variable
bpsis assigned to but never usedRemove assignment to unused variable
bps(F841)
96-104: Don’t terminate the whole process from a library class
sys.exit()will kill whichever program imports this module. Prefer raising a domain-specific exception and let callers decide how to recover.- print("\nError: Sample Lost") - self.cleanup() - sys.exit(1) + raise RuntimeError("Sample lost – connection appears unreliable")(Apply the same change for the duplicate-sample branch.)
116-124: Exception is swallowed during cleanupThe bare
exceptmasks real bugs and the unused variableetriggers Ruff F841. At least log the message.- except Exception as e: - pass + except Exception as e: + print(f"Error while closing WebSocket: {e}")🧰 Tools
🪛 Ruff (0.8.2)
120-120: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
chords_serial.py (2)
61-62: Remove unusedsampling_rateassignment
sampling_rateis read fromsupported_boardsbut never referenced afterwards.- sampling_rate = self.supported_boards[self.board]["sampling_rate"]🧰 Tools
🪛 Ruff (0.8.2)
61-61: Local variable
sampling_rateis assigned to but never usedRemove assignment to unused variable
sampling_rate(F841)
148-149:cleanup()should not callsys.exit()For a reusable library, forcing the interpreter to exit prevents callers (e.g., Flask backend) from recovering or cleaning up additional resources. Return gracefully or raise an exception instead.
- print("Cleanup Completed.") - sys.exit(0) + print("Cleanup Completed.") + # Let caller decide whether to exitconnection.py (2)
7-8: Remove unusedsysimport
sysis not referenced in this file.-import sys🧰 Tools
🪛 Ruff (0.8.2)
7-7:
sysimported but unusedRemove unused import:
sys(F401)
60-68: Open the CSV file with a context managerUsing
with open(...)ensures the file handle is flushed and closed even if an exception occurs before__del__executes.- self.csv_file = open(filename, 'w', newline='') - headers = ['Counter'] + [f'Channel{i+1}' for i in range(self.num_channels)] - self.csv_writer = csv.writer(self.csv_file) - self.csv_writer.writerow(headers) + with open(filename, 'w', newline='') as f: + self.csv_writer = csv.writer(f) + headers = ['Counter'] + [f'Channel{i+1}' for i in range(self.num_channels)] + self.csv_writer.writerow(headers) + # Re-open in append mode for the streaming loop + self.csv_file = open(filename, 'a', newline='') + self.csv_writer = csv.writer(self.csv_file)🧰 Tools
🪛 Ruff (0.8.2)
63-63: Use a context manager for opening files
(SIM115)
static/style.css (3)
333-344:.header h1rules are duplicated – keep a single source of truthThese lines repeat the exact style block already defined at ~35-46. Duplicate selectors bloat the sheet and increase the risk of the two blocks getting out-of-sync in future edits.
-/* REMOVE duplicated .header h1 block (already defined above) */ -.header h1 { - font-size: 48px; - font-weight: bold; - color: #333; - margin: 0 0 15px 0; - background: linear-gradient(90deg, #ec4899, #a855f7, #3b82f6, #ec4899); - background-size: 300%; - -webkit-background-clip: text; - background-clip: text; - -webkit-text-fill-color: transparent; - animation: scroll-gradient 5s linear infinite; -}
452-475:protocol-button-group&protocol-buttons-combined buttonblocks appear twiceThe definitions at 55-62 and here are identical. Consider extracting common button styling into one block and using utility classes or CSS variables to override only what differs (e.g. size in media queries).
This will slim the stylesheet and make future tweaks less error-prone.
287-308: Back-to-back identical@media (max-width: 600px)queriesTwo consecutive media queries target the same breakpoint with overlapping rules for the same selectors. Merge them to reduce CSS size and ensure later edits remain consistent.
Example:
@media (max-width: 600px) { - /* first block … */ -} - -@media (max-width: 600px) { - /* second block … */ + /* merged declarations … */ }app.py (1)
172-176: Terminated subprocesses may linger as zombies
running_apps[app_name].terminate()sends SIGTERM but you neverwait()on the process. On POSIX systems this leaves a zombie until the parent collects the exit status.running_apps[app_name].terminate() -running_apps.pop(app_name, None) +running_apps[app_name].wait(timeout=5) +running_apps.pop(app_name, None)Optionally use
kill()after a grace period.templates/index.html (1)
136-141: CSV toggle is permanently disabled after initiating a connection
setCSVToggleState(false)is called at the start of every connect function but never reset totrueupon successful connection, so users cannot toggle CSV logging for subsequent sessions without refreshing.Add a call after a successful connection:
// inside success branch enableAllAppButtons(); setCSVToggleState(true);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
app.py(1 hunks)beetle.py(7 hunks)chords.py(0 hunks)chords_ble.py(1 hunks)chords_serial.py(1 hunks)chords_wifi.py(1 hunks)connection.py(1 hunks)emgenvelope.py(4 hunks)eog.py(3 hunks)ffteeg.py(4 hunks)gui.py(5 hunks)heartbeat_ecg.py(4 hunks)keystroke.py(2 hunks)npg-ble.py(0 hunks)static/style.css(3 hunks)templates/index.html(2 hunks)
💤 Files with no reviewable changes (2)
- chords.py
- npg-ble.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
keystroke.py (1)
eog.py (1)
detect_blinks(146-165)
chords_wifi.py (2)
chords_ble.py (2)
connect(161-172)cleanup(151-159)chords_serial.py (1)
cleanup(138-149)
🪛 Ruff (0.8.2)
chords_ble.py
75-75: Local variable channels is assigned to but never used
Remove assignment to unused variable channels
(F841)
chords_serial.py
61-61: Local variable sampling_rate is assigned to but never used
Remove assignment to unused variable sampling_rate
(F841)
connection.py
7-7: sys imported but unused
Remove unused import: sys
(F401)
63-63: Use a context manager for opening files
(SIM115)
chords_wifi.py
5-5: scipy.signal.butter imported but unused
Remove unused import
(F401)
5-5: scipy.signal.filtfilt imported but unused
Remove unused import
(F401)
67-67: Local variable sps is assigned to but never used
Remove assignment to unused variable sps
(F841)
68-68: Local variable fps is assigned to but never used
Remove assignment to unused variable fps
(F841)
69-69: Local variable bps is assigned to but never used
Remove assignment to unused variable bps
(F841)
120-120: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
135-135: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
app.py
5-5: webbrowser imported but unused
Remove unused import: webbrowser
(F401)
🔇 Additional comments (24)
ffteeg.py (4)
12-12: Added time module import for disconnection detection.Adds the time module to enable tracking timestamps for disconnection detection.
21-22: Added stream state tracking for disconnection detection.Good addition of variables to track stream state and detect disconnections:
stream_activeflag to maintain stream statuslast_data_timeto track when data was last receivedThis enables graceful shutdown on disconnection.
115-115: Updated timestamp tracking for disconnection detection.Records the current time whenever data is received from the LSL stream, supporting the disconnection detection mechanism.
137-143: Added disconnection detection and graceful shutdown.Good implementation of disconnection detection logic:
- Checks if more than 2 seconds have passed since last data reception
- Sets the stream status flag to inactive
- Provides console feedback
- Stops the UI update timer
- Gracefully closes the application window
This prevents the application from freezing or displaying stale data when the stream is lost.
beetle.py (11)
5-5: Added sys module import for error handling.The sys module is necessary for the sys.exit() calls added for proper error handling.
16-18: Added stream state tracking variables.Good addition of variables to track stream state:
last_data_timeto record timestamp of last received datastream_activeflag to maintain connection statusinletexplicit initialization to None for better error handling
22-22: Added proper error handling for missing streams.Improved error handling by properly exiting when no LSL streams are found rather than continuing execution.
37-37: Added proper error handling for connection failure.Ensured the application properly exits when unable to connect to any stream.
81-84: Added event handling for graceful exit.Good addition of event handling during message display to allow users to close the window at any time, ensuring the application exits cleanly rather than hanging.
113-113: Added global declaration for last_data_time.Ensures the global variable is properly accessed within the calibration function.
135-135: Added timestamp tracking during calibration.Records the time when data is received during calibration to support disconnection detection.
138-142: Added disconnection detection during calibration.Good implementation of disconnection detection during the critical calibration phase:
- Checks if 2+ seconds passed since last data reception
- Shows a user-friendly message
- Properly cleans up pygame resources
- Exits with an error code
144-147: Added event handling during calibration.Important addition of event handling to allow users to exit during the calibration process.
197-197: Added timestamp tracking in main game loop.Records the time when data is received in main game loop to support disconnection detection.
200-204: Added disconnection detection in main game loop.Good implementation of disconnection detection during the main game loop:
- Checks if 2+ seconds passed since last data reception
- Shows a user-friendly message
- Sets the running flag to false to exit the loop gracefully
- Uses break to immediately exit the loop
heartbeat_ecg.py (4)
10-10: Added time module import for disconnection detection.Adds the time module to enable tracking timestamps for disconnection detection.
19-20: Added stream state tracking for disconnection detection.Good addition of variables to track stream state and detect disconnections:
stream_activeflag to maintain stream statuslast_data_timeto track when data was last receivedThis enables graceful shutdown on disconnection.
114-114: Updated timestamp tracking for disconnection detection.Records the current time whenever data is received from the LSL stream, supporting the disconnection detection mechanism.
128-133: Added disconnection detection and graceful shutdown.Good implementation of disconnection detection logic:
- Checks if more than 2 seconds have passed since last data reception
- Sets the stream status flag to inactive
- Provides console feedback
- Stops the UI update timer
- Gracefully closes the application window
This prevents the application from freezing or displaying stale data when the stream is lost.
eog.py (3)
17-18: Added stream state tracking for disconnection detection.Good addition of variables to track stream state and detect disconnections:
stream_activeflag to maintain stream statuslast_data_timeto track when data was last receivedThis enables graceful shutdown on disconnection.
105-105: Updated timestamp tracking for disconnection detection.Records the current time whenever data is received from the LSL stream, supporting the disconnection detection mechanism.
139-144: Added disconnection detection and graceful shutdown.Good implementation of disconnection detection logic:
- Checks if more than 2 seconds have passed since last data reception
- Sets the stream status flag to inactive
- Provides console feedback
- Stops the UI update timer
- Gracefully closes the application window
This prevents the application from freezing or displaying stale data when the stream is lost.
static/style.css (1)
7-12: Avoidoverflow: hiddenon<body>– it prevents scrolling on small screensLocking the viewport with
height: 100vh; overflow: hidden;can make the entire UI unreachable / un-scrollable on devices where the content vertically exceeds 100 % of the viewport (mobile browsers add a dynamic URL bar, virtual keyboard, etc.).
Consider either:-body { - height: 100vh; - overflow: hidden; +body { + min-height: 100vh; /* allow taller content */ + overflow-y: auto; /* enable scrolling when needed */ }or moving the height/overflow constraints to a subordinate wrapper that truly needs to be fixed-height (e.g. a splash screen).
templates/index.html (1)
513-578: Client expects/disconnect– update JS once the backend route is addedOnce you add the Flask
/disconnectroute (see backend comment), remember to check the response here and surface errors to the user instead of refreshing regardless of failure.const resp = await fetch('/disconnect', { method: 'POST' }); const data = await resp.json(); if (data.status !== 'success') { console.error(data.message); statusDiv.textContent = data.message || 'Disconnection failed'; statusDiv.className = 'error-status'; return; // skip reload }
| global last_data_time, stream_active | ||
| if self.inlet is None or not self.stream_active: | ||
| return |
There was a problem hiding this comment.
Remove unnecessary global declaration to prevent namespace confusion
last_data_time and stream_active are instance attributes (self.last_data_time, self.stream_active).
Declaring them as global does nothing useful here and may trick future readers (or static-analysis tools) into thinking the variables live at module scope.
- global last_data_time, stream_activeNo other change is required—the rest of the method already uses the instance attributes correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| global last_data_time, stream_active | |
| if self.inlet is None or not self.stream_active: | |
| return | |
| if self.inlet is None or not self.stream_active: | |
| return |
| self.last_data_time = time.time() | ||
| for sample in samples: | ||
| self.eog_data[self.current_index] = sample[0] # Store sample in circular buffer at current position | ||
| self.current_index = (self.current_index + 1) % self.buffer_size # Update index with wrap-around using modulo | ||
|
|
||
| filtered_eog = lfilter(self.b, self.a, self.eog_data) | ||
| self.detect_blinks(filtered_eog) # Run blink detection on the filtered signal | ||
| else: | ||
| if self.last_data_time and (time.time() - self.last_data_time) > 2: | ||
| print("LSL Status: Stream disconnected. Stopping detection.") | ||
| self.stop_detection() | ||
| popup.after(0, popup.quit) # Schedule quit on main thread | ||
| break # Exit the loop | ||
| except Exception as e: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
stream_active never updated & risk of false-positive disconnect
You introduced self.last_data_time but never toggle the new self.stream_active flag; meanwhile the disconnect check executes even before the first sample arrives, which can mark the stream as “disconnected” during normal start-up latency.
Suggested fixes:
- samples, _ = self.inlet.pull_chunk(timeout=1.0, max_samples=1)
+ # Wait slightly longer on the very first pull to avoid premature time-outs
+ samples, _ = self.inlet.pull_chunk(timeout=2.0, max_samples=1)
@@
- self.last_data_time = time.time()
+ self.last_data_time = time.time()
+ self.stream_active = True
@@
- if self.last_data_time and (time.time() - self.last_data_time) > 2:
+ if self.last_data_time and (time.time() - self.last_data_time) > 2:
print("LSL Status: Stream disconnected. Stopping detection.")
+ self.stream_active = FalseKeeping stream_active in sync and giving the first pull a longer timeout will reduce spurious disconnects.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.last_data_time = time.time() | |
| for sample in samples: | |
| self.eog_data[self.current_index] = sample[0] # Store sample in circular buffer at current position | |
| self.current_index = (self.current_index + 1) % self.buffer_size # Update index with wrap-around using modulo | |
| filtered_eog = lfilter(self.b, self.a, self.eog_data) | |
| self.detect_blinks(filtered_eog) # Run blink detection on the filtered signal | |
| else: | |
| if self.last_data_time and (time.time() - self.last_data_time) > 2: | |
| print("LSL Status: Stream disconnected. Stopping detection.") | |
| self.stop_detection() | |
| popup.after(0, popup.quit) # Schedule quit on main thread | |
| break # Exit the loop | |
| except Exception as e: | |
| self.last_data_time = time.time() | |
| self.stream_active = True | |
| for sample in samples: | |
| self.eog_data[self.current_index] = sample[0] # Store sample in circular buffer at current position | |
| self.current_index = (self.current_index + 1) % self.buffer_size # Update index with wrap-around using modulo | |
| filtered_eog = lfilter(self.b, self.a, self.eog_data) | |
| self.detect_blinks(filtered_eog) # Run blink detection on the filtered signal | |
| else: | |
| if self.last_data_time and (time.time() - self.last_data_time) > 2: | |
| print("LSL Status: Stream disconnected. Stopping detection.") | |
| self.stream_active = False | |
| self.stop_detection() | |
| popup.after(0, popup.quit) # Schedule quit on main thread | |
| break # Exit the loop | |
| except Exception as e: |
| channels = [ | ||
| int.from_bytes(sample_data[1:3], byteorder='big', signed=True), | ||
| int.from_bytes(sample_data[3:5], byteorder='big', signed=True), | ||
| int.from_bytes(sample_data[5:7], byteorder='big', signed=True)] | ||
|
|
||
| self.samples_received += 1 | ||
|
|
There was a problem hiding this comment.
Unused channels variable — either publish or remove
channels is decoded but never referenced, triggering the Ruff warning (F841) and wasting CPU cycles.
If you only need the counter logic, remove the assignment; if channel data will feed another pipeline, store it or emit an event.
- channels = [
- int.from_bytes(sample_data[1:3], byteorder='big', signed=True),
- int.from_bytes(sample_data[3:5], byteorder='big', signed=True),
- int.from_bytes(sample_data[5:7], byteorder='big', signed=True)]
-
- self.samples_received += 1
+ # Channel bytes decoded here when downstream processing is added
+ self.samples_received += 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| channels = [ | |
| int.from_bytes(sample_data[1:3], byteorder='big', signed=True), | |
| int.from_bytes(sample_data[3:5], byteorder='big', signed=True), | |
| int.from_bytes(sample_data[5:7], byteorder='big', signed=True)] | |
| self.samples_received += 1 | |
| # Channel bytes decoded here when downstream processing is added | |
| self.samples_received += 1 |
🧰 Tools
🪛 Ruff (0.8.2)
75-75: Local variable channels is assigned to but never used
Remove assignment to unused variable channels
(F841)
| try: | ||
| while True: | ||
| try: | ||
| data = self.ws.recv() | ||
| self.last_data_time = time.time() # Update when data is received | ||
| except (websocket.WebSocketConnectionClosedException, ConnectionResetError) as e: | ||
| print(f"\nConnection closed: {str(e)}") | ||
| self.cleanup() | ||
| return | ||
| except Exception as e: | ||
| print(f"\n[ERROR] Connection error: {e}") | ||
| self.cleanup() | ||
| return | ||
|
|
||
| # Check for timeout (device stopped sending data) | ||
| if time.time() - self.last_data_time > self.timeout_sec: | ||
| print("\nDevice stopped sending data") | ||
| self.cleanup() | ||
| return | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
ws.recv() blocks forever, defeating the timeout check
websocket-client waits indefinitely if no data arrives, so the subsequent timeout guard is never reached.
Set a socket timeout right after connecting and handle WebSocketTimeoutException instead.
self.ws = websocket.WebSocket()
self.ws.connect(f"ws://{host_ip}:81")
+self.ws.settimeout(self.timeout_sec) # <-- keep recv responsive- except (websocket.WebSocketConnectionClosedException, ConnectionResetError) as e:
+ except (websocket.WebSocketTimeoutException,) as e:
+ # No data within timeout
+ print("\nDevice stopped sending data")
+ self.cleanup()
+ return
+ except (websocket.WebSocketConnectionClosedException, ConnectionResetError) as e:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| while True: | |
| try: | |
| data = self.ws.recv() | |
| self.last_data_time = time.time() # Update when data is received | |
| except (websocket.WebSocketConnectionClosedException, ConnectionResetError) as e: | |
| print(f"\nConnection closed: {str(e)}") | |
| self.cleanup() | |
| return | |
| except Exception as e: | |
| print(f"\n[ERROR] Connection error: {e}") | |
| self.cleanup() | |
| return | |
| # Check for timeout (device stopped sending data) | |
| if time.time() - self.last_data_time > self.timeout_sec: | |
| print("\nDevice stopped sending data") | |
| self.cleanup() | |
| return | |
| self.ws = websocket.WebSocket() | |
| self.ws.connect(f"ws://{host_ip}:81") | |
| self.ws.settimeout(self.timeout_sec) # <-- keep recv responsive | |
| try: | |
| while True: | |
| try: | |
| data = self.ws.recv() | |
| self.last_data_time = time.time() # Update when data is received | |
| except (websocket.WebSocketTimeoutException,) as e: | |
| # No data within timeout | |
| print("\nDevice stopped sending data") | |
| self.cleanup() | |
| return | |
| except (websocket.WebSocketConnectionClosedException, ConnectionResetError) as e: | |
| print(f"\nConnection closed: {str(e)}") | |
| self.cleanup() | |
| return | |
| except Exception as e: | |
| print(f"\n[ERROR] Connection error: {e}") | |
| self.cleanup() | |
| return | |
| # Check for timeout (device stopped sending data) | |
| if time.time() - self.last_data_time > self.timeout_sec: | |
| print("\nDevice stopped sending data") | |
| self.cleanup() | |
| return |
| try: | ||
| print("\nConnected! (Press Ctrl+C to stop)") | ||
| while True: | ||
| data = self.wifi_connection.ws.recv() | ||
|
|
||
| if isinstance(data, (bytes, list)): | ||
| for i in range(0, len(data), self.wifi_connection.block_size): | ||
| block = data[i:i + self.wifi_connection.block_size] | ||
| if len(block) < self.wifi_connection.block_size: | ||
| continue | ||
|
|
||
| channel_data = [] | ||
| for ch in range(self.wifi_connection.channels): | ||
| offset = 1 + ch * 2 | ||
| sample = int.from_bytes(block[offset:offset + 2], byteorder='big', signed=True) | ||
| channel_data.append(sample) | ||
|
|
||
| if self.lsl_connection: # Push to LSL | ||
| self.lsl_connection.push_sample(channel_data) | ||
| if self.csv_logging and self.csv_writer: # Only log if CSV is set up | ||
| self.log_to_csv(channel_data) | ||
|
|
||
| except KeyboardInterrupt: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
No error handling for broken WebSocket inside WiFi loop
If the connection drops, ws.recv() raises WebSocketConnectionClosedException, crashing the loop. Surround the body with try/except similar to chords_wifi.process_data() or delegate to that method instead of duplicating code.
| self.wifi_connection.disconnect() | ||
| print("\nDisconnected") |
There was a problem hiding this comment.
disconnect() is undefined for Chords_WIFI
Chords_WIFI only exposes cleanup(). Calling a non-existent method will raise AttributeError on the first keyboard interrupt.
- self.wifi_connection.disconnect()
+ self.wifi_connection.cleanup()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.wifi_connection.disconnect() | |
| print("\nDisconnected") | |
| self.wifi_connection.cleanup() | |
| print("\nDisconnected") |
| if __name__ == "__main__": | ||
| app.run(debug=True) |
There was a problem hiding this comment.
Frontend calls a /disconnect endpoint that is missing here
templates/index.html → disconnectAll() issues a POST /disconnect, but no corresponding Flask route exists.
Without it, the call returns 404 and any server-side cleanup (stopping LSL stream, closing serial/BLE sockets, resetting globals) never happens.
Suggested minimal implementation:
+@app.route('/disconnect', methods=['POST'])
+def disconnect():
+ global active_connection, lsl_stream_active
+
+ try:
+ if active_connection:
+ active_connection.close() # assuming `Connection` exposes close()
+ active_connection = None
+ lsl_stream_active = False
+ return jsonify({'status': 'success'})
+ except Exception as exc:
+ return jsonify({'status': 'error', 'message': str(exc)}), 500Be sure the Connection class provides a safe close() / disconnect() method that stops background threads and releases resources.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if __name__ == "__main__": | |
| app.run(debug=True) | |
| @app.route('/disconnect', methods=['POST']) | |
| def disconnect(): | |
| global active_connection, lsl_stream_active | |
| try: | |
| if active_connection: | |
| active_connection.close() # assuming `Connection` exposes close() | |
| active_connection = None | |
| lsl_stream_active = False | |
| return jsonify({'status': 'success'}) | |
| except Exception as exc: | |
| return jsonify({'status': 'error', 'message': str(exc)}), 500 | |
| if __name__ == "__main__": | |
| app.run(debug=True) |
🧰 Tools
🪛 ast-grep (0.31.1)
[warning] 196-196: Detected Flask app with debug=True. Do not deploy to production with this flag enabled as it will leak sensitive information. Instead, consider using Flask configuration variables or setting 'debug' using system environment variables.
Context: app.run(debug=True)
Note: [CWE-489] Active Debug Code. [REFERENCES]
- https://labs.detectify.com/2015/10/02/how-patreon-got-hacked-publicly-exposed-werkzeug-debugger/
(debug-enabled-python)
| import asyncio | ||
| from connection import Connection | ||
| from threading import Thread | ||
| import webbrowser |
There was a problem hiding this comment.
Remove unused webbrowser import
webbrowser is imported but never referenced, triggering Ruff F401 and adding unnecessary memory overhead at startup.
-from threading import Thread
-import webbrowser
+from threading import Thread📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import webbrowser | |
| from threading import Thread |
🧰 Tools
🪛 Ruff (0.8.2)
5-5: webbrowser imported but unused
Remove unused import: webbrowser
(F401)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
templates/index.html (1)
6-8: Title update and icon library version
The<title>refactor looks good. For the Font Awesome import, consider upgrading to a more recent major version (5.x or 6.x) to benefit from additional icons and security patches.static/style.css (4)
7-8: Revisitoverflow: hiddenon<body>
Hiding overflow on the<body>can prevent scrolling when content (e.g., the BLE popup) exceeds viewport height on smaller screens. Consider removing or toggling it only when needed (e.g., when the popup is shown).
35-46: Duplicate.header h1styles
The gradient-animated header text rules are defined twice (lines 35–46 and 343–354). Consolidate into a single block to avoid maintenance overhead.Also applies to: 343-354
55-61: Consolidate duplicated.protocol-button-grouprules
The.protocol-button-groupselector appears twice with identical properties. Merge them into one definition to DRY up the CSS.Also applies to: 462-468
275-300: Duplicate media queries formax-width: 600px
Two nearly identical blocks target the same breakpoint. Combine these into a single@media (max-width: 600px)section to avoid inconsistencies.Also applies to: 302-323
| // Initialize CSV toggle | ||
| document.getElementById('csvToggle').addEventListener('change', function() { | ||
| fetch('/set_csv', { | ||
| method: 'POST', | ||
| headers: { |
There was a problem hiding this comment.
Missing CSV toggle element (csvToggle)
The script at DOMContentLoaded references document.getElementById('csvToggle'), but there is no element with that ID in the markup. This will throw at runtime. Either add the corresponding <input type="checkbox" id="csvToggle"> or update the JS to use the correct control (e.g., csvRecordBtn).
| // function setCSVToggleState(enabled) { | ||
| // const csvToggle = document.getElementById('csvToggle'); | ||
| // csvToggle.disabled = !enabled; | ||
| // csvToggle.parentElement.style.opacity = enabled ? '1' : '0.6'; | ||
| // csvToggle.parentElement.style.cursor = enabled ? 'pointer' : 'not-allowed'; | ||
| // } |
There was a problem hiding this comment.
Commented-out setCSVToggleState causes broken calls
You call setCSVToggleState(false) in the BLE/USB/WiFi flows but the function is commented out. Uncomment or remove these calls to avoid JavaScript errors when attempting to disable/enable the CSV toggle.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Chores