Skip to content

Updated UI with different protocols-USB, WIFI, BLE added #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 41 commits into from
May 9, 2025

Conversation

PayalLakra
Copy link
Member

@PayalLakra PayalLakra commented May 2, 2025

Summary by CodeRabbit

  • New Features

    • Unified connection interface for USB, WiFi, and BLE devices with protocol-specific buttons and BLE device selection modal.
    • Added disconnect button and CSV recording toggle to the UI.
    • Introduced a modular, asynchronous connection management backend replacing legacy subprocess-based device handling.
    • Added robust BLE, USB, and WiFi device communication modules with real-time data streaming and logging.
    • Dynamic application launching with status checks and error alerts integrated into the UI.
    • Enhanced real-time detection and graceful handling of device or stream disconnections across all monitoring and plotting tools.
  • Improvements

    • Redesigned and modernized UI with Tailwind CSS and Font Awesome icons, improved layout, dark mode, and responsiveness.
    • Streamlined connection and application management with clear status updates and error handling.
    • Client-driven asynchronous operations replace server-side forms and legacy SSE updates.
    • Improved stream disconnection detection with timeout-based monitoring and graceful shutdown in all real-time visualization tools.
  • Bug Fixes

    • Improved detection and handling of lost or unavailable data streams, ensuring clean shutdowns and user notifications.
  • Refactor

    • Major backend and frontend codebase restructuring for modularity, maintainability, and unified user experience.
    • Removal of legacy scripts, subprocesses, and session-based device management; introduction of modular connection and protocol classes.
    • Consolidation of connection logic into a single Connection class managing USB, WiFi, and BLE protocols.
  • Chores

    • Deprecated and removed obsolete files and scripts, consolidating device communication logic into new, maintainable modules.
    • Removed legacy CSS styling file, replaced by Tailwind CSS-based styling.
    • Removed legacy popups and replaced with modern modals and inline status messages.

Copy link

coderabbitai bot commented May 2, 2025

"""

Walkthrough

This 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 (connection.py, chords_ble.py, chords_serial.py, chords_wifi.py) encapsulate protocol-specific logic, while the Flask backend (app.py) exposes simplified API endpoints for scanning, connecting, and managing device streams and applications. The web UI (templates/index.html) is overhauled for a modern, responsive experience with protocol buttons, BLE device selection, CSV logging toggle, and app control. Disconnection detection and graceful shutdown are added across all real-time plotting and monitoring applications. The legacy CSS file was removed.

Changes

Files / Groups Change Summary
app.py Refactored to use a modular, asynchronous connection model via a new Connection class; replaced subprocess/thread-based device and stream management with thread-based, explicit JSON API endpoints for device scanning, connection, CSV logging, and app control; removed SSE, session-based device selection, and legacy error handling.
beetle.py, emgenvelope.py, eog.py, ffteeg.py,
gui.py, heartbeat_ecg.py, keystroke.py
Added LSL stream disconnection detection via timeout and graceful shutdown logic in real-time plotting/monitoring GUIs and scripts.
chords.py, npg-ble.py Deleted legacy scripts for Arduino serial and BLE device streaming; all logic now modularized and managed via new connection modules.
chords_ble.py New module for BLE device scanning, connection, and data streaming using Bleak; includes sample loss detection, async connection monitoring, and CLI interface.
chords_serial.py New module for USB serial device detection, connection, and data streaming; includes hardware handshake, packet parsing, and signal-based cleanup.
chords_wifi.py New module for WiFi/WebSocket device connection and data streaming with error detection and rate calculation.
connection.py New unified connection manager class for USB, WiFi, and BLE; handles LSL streaming, optional CSV logging, and protocol-specific connection logic; exposes CLI interface.
static/style.css Entire CSS file removed, eliminating previous styling rules for layout, buttons, popups, animations, and status indicators.
templates/index.html Overhauled UI: protocol selection, BLE device popup, CSV toggle, disconnect logic, app control buttons, and client-driven fetch-based backend interaction; removed server-side forms and SSE; restyled with Tailwind CSS and Font Awesome icons in dark mode.
requirements.txt Added new requirements file listing pinned versions of all Python dependencies used in the project.
config/apps.yaml Added new YAML configuration file defining metadata for available applications including title, icon, color, script, description, and category.
static/script.js Added new comprehensive frontend JavaScript managing app loading, protocol connection, BLE device scanning, recording control, console event streaming, UI state updates, and theme toggling.

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: /connect (with protocol, device address, CSV flag)
    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 updates
    FlaskAPI->>WebUI: JSON response (success/failure)
    WebUI->>WebUI: Enable app buttons, update status

    User->>WebUI: Click app button
    WebUI->>FlaskAPI: /launch_app (app name)
    FlaskAPI->>AppScript: Start app subprocess
    AppScript-->>FlaskAPI: Running status
    FlaskAPI->>WebUI: JSON response (app running)

    User->>WebUI: Click disconnect
    WebUI->>FlaskAPI: /disconnect
    FlaskAPI->>ConnectionManager: Cleanup connections
    ConnectionManager->>Device: Stop stream, cleanup
    ConnectionManager->>LSL: Close outlet
    FlaskAPI->>WebUI: JSON response (disconnected)
Loading

Possibly related PRs

  • upsidedownlabs/Chords-Python#38: Refactored app.py to support BLE device scanning and connection using subprocess-based NPG BLE client; this PR replaces that approach with a modular, async design and removes the subprocess-based BLE client.

Poem

🐇
Refactored wires, BLE and more,
Async connections at the core!
Protocol buttons, popups bright,
CSV toggles—logging right.
Apps now run with just a click,
Streams detected—disconnects quick!
Modular code, a hoppy delight—
The Chords are tuned and running light!

"""

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (16)
emgenvelope.py (1)

132-138: Timer stops but inlet is left open – consider explicit cleanup

After detecting a disconnection you stop the QTimer and close the window, but the LSL StreamInlet object 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 to None) before shutting down to release the socket promptly.

gui.py (1)

36-43: UI shuts down gracefully, but a tiny race remains

timer.stop() and win.close() are executed from the timer thread-context.
If plot_lsl_data() has already returned its app and the main thread is about to call app.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 deleted

Safest 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 code

The 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 a print() (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 = 0
chords_wifi.py (4)

5-5: Remove unused SciPy imports

butter and filtfilt are 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.butter imported but unused

Remove unused import

(F401)


5-5: scipy.signal.filtfilt imported but unused

Remove unused import

(F401)


66-74: Computed throughput metrics are unused

sps, fps, and bps are 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 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)


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 cleanup

The bare except masks real bugs and the unused variable e triggers 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 e is assigned to but never used

Remove assignment to unused variable e

(F841)

chords_serial.py (2)

61-62: Remove unused sampling_rate assignment

sampling_rate is read from supported_boards but never referenced afterwards.

-                    sampling_rate = self.supported_boards[self.board]["sampling_rate"]
🧰 Tools
🪛 Ruff (0.8.2)

61-61: Local variable sampling_rate is assigned to but never used

Remove assignment to unused variable sampling_rate

(F841)


148-149: cleanup() should not call sys.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 exit
connection.py (2)

7-8: Remove unused sys import

sys is not referenced in this file.

-import sys
🧰 Tools
🪛 Ruff (0.8.2)

7-7: sys imported but unused

Remove unused import: sys

(F401)


60-68: Open the CSV file with a context manager

Using 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 h1 rules are duplicated – keep a single source of truth

These 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 button blocks appear twice

The 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) queries

Two 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 never wait() 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 to true upon 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0b503e6 and 1bd152a.

📒 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_active flag to maintain stream status
  • last_data_time to track when data was last received

This 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:

  1. Checks if more than 2 seconds have passed since last data reception
  2. Sets the stream status flag to inactive
  3. Provides console feedback
  4. Stops the UI update timer
  5. 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_time to record timestamp of last received data
  • stream_active flag to maintain connection status
  • inlet explicit 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:

  1. Checks if 2+ seconds passed since last data reception
  2. Shows a user-friendly message
  3. Properly cleans up pygame resources
  4. 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:

  1. Checks if 2+ seconds passed since last data reception
  2. Shows a user-friendly message
  3. Sets the running flag to false to exit the loop gracefully
  4. 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_active flag to maintain stream status
  • last_data_time to track when data was last received

This 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:

  1. Checks if more than 2 seconds have passed since last data reception
  2. Sets the stream status flag to inactive
  3. Provides console feedback
  4. Stops the UI update timer
  5. 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_active flag to maintain stream status
  • last_data_time to track when data was last received

This 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:

  1. Checks if more than 2 seconds have passed since last data reception
  2. Sets the stream status flag to inactive
  3. Provides console feedback
  4. Stops the UI update timer
  5. 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: Avoid overflow: hidden on <body> – it prevents scrolling on small screens

Locking 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 added

Once you add the Flask /disconnect route (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
}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: Revisit overflow: hidden on <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 h1 styles
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-group rules
The .protocol-button-group selector 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 for max-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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1bd152a and 617e87e.

📒 Files selected for processing (2)
  • static/style.css (3 hunks)
  • templates/index.html (2 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (3)
templates/index.html (3)

32-34: Undefined connectDefault() function
The generic "Connect" button calls connectDefault(), but this function is not defined in your JavaScript. This will throw a runtime error. Either implement connectDefault() to delegate based on the selected protocol or remove/wire this button directly to the protocol connect functions.


133-137: Missing csvToggle element
The script references document.getElementById('csvToggle'), but there is no element with this ID in the markup. This will throw at runtime. Consider adding a corresponding <input type="checkbox" id="csvToggle"> or update the JS to use the existing control (e.g., the CSV record button).


145-150: Commented-out setCSVToggleState function
The setCSVToggleState helper is fully commented out, yet it is invoked in the connection flows. Uncomment or reimplement it to avoid undefined-function errors when toggling CSV recording state.

🧹 Nitpick comments (5)
templates/index.html (1)

37-37: Avoid inline visibility styles
The .disconnect-button container uses style="display: none;" inline. Consider moving this to a CSS rule (e.g., a .hidden class) and toggling visibility via JS to improve maintainability.

static/style.css (4)

7-11: Avoid using overflow: hidden and fixed height on body
Setting height: 100vh and overflow: hidden on the <body> prevents scrolling and may cause content to be clipped on smaller viewports. Consider using min-height and allowing overflow: auto or handling overflow per container to improve responsiveness.


15-26: Review fixed height on .container
Using height: 95vh on the main container may lead to layout issues on shorter screens or when content grows. Consider switching to max-height with overflow-y: auto to handle dynamic content and improve mobile support.


180-197: Style container for .disconnect-button
You have a <div class="disconnect-button"> wrapper in the HTML, but no corresponding CSS rule targeting .disconnect-button. If you need specific positioning or animations for this container, add styles to ensure consistent layout.


100-115: Accessibility: Add focus styles to interactive buttons
The .protocol-btn selector does not define any :focus styles, which may hinder keyboard navigation. Consider adding visible outlines or custom focus indicators for better accessibility (WCAG).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 617e87e and 9571eec.

📒 Files selected for processing (2)
  • static/style.css (4 hunks)
  • templates/index.html (2 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

♻️ Duplicate comments (1)
app.py (1)

131-132: ⚠️ Potential issue

Debug mode enabled in production.

Running Flask with debug=True in production is a security risk as it exposes sensitive information and enables the interactive debugger.

Modify to use environment variables for configuration:

if __name__ == "__main__":
-    app.run(debug=True)
+    import os
+    debug_mode = os.environ.get('FLASK_DEBUG', 'False').lower() == 'true'
+    app.run(debug=debug_mode)
🧰 Tools
🪛 ast-grep (0.31.1)

[warning] 131-131: 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)

🧹 Nitpick comments (3)
app.py (1)

20-29: Manual asyncio wrapper function can be replaced with Flask-specific solution.

The custom run_async decorator is implementing functionality that can be better handled by existing solutions like quart or flask_async extensions, which provide better integration with Flask's lifecycle and error handling.

Consider using a Flask-specific async extension like Quart or flask_async instead of implementing a custom decorator. This would also help with proper cleanup of asyncio resources.

templates/index.html (2)

531-539: Duplicate DOM element queries in updateConnectionStatus.

The function re-queries DOM elements that are already available in the global scope, which is inefficient.

Remove redundant DOM queries by using the global variables:

// Function to update the connection status on the frontend
function updateConnectionStatus(message, icon, colorClass) {
-    const statusText = document.getElementById('status-text');
-    const statusIcon = document.getElementById('status-icon');
-    const statusDiv = document.getElementById('connection-status');
    
    statusText.textContent = message;
    statusIcon.innerHTML = `<i class="fas ${icon} ${colorClass}"></i>`;
    statusDiv.classList.remove('hidden');
}

129-134: Typo in one of the app titles.

There's a typo in the app title "EOG Keystroke Emualtor" - should be "Emulator".

Fix the typo in the app title:

    const apps = [
        { title: 'ECG with Heart Rate', icon: 'fa-heartbeat', color: 'cyan' },
        { title: 'EMG with Envelope', icon: 'fa-wave-square', color: 'red' },
        { title: 'EOG with Blinks', icon: 'fa-chart-line', color: 'purple' },
        { title: 'EEG with FFT', icon: 'fa-hand-holding-water', color: 'green' },
        { title: 'EEG Tug of War game', icon: 'fa-brain', color: 'yellow' },
        { title: 'EEG Beetle Game', icon: 'fa-eye', color: 'pink' },
-        { title: 'EOG Keystroke Emualtor', icon: 'fa-lungs', color: 'blue' },
+        { title: 'EOG Keystroke Emulator', icon: 'fa-lungs', color: 'blue' },
        { title: 'GUI Visualization', icon: 'fa-heart-pulse', color: 'orange' },
        { title: 'CSV Plotter', icon: 'fa-thermometer-half', color: 'teal' },
    ];
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9571eec and b75edbd.

📒 Files selected for processing (3)
  • app.py (1 hunks)
  • static/style.css (0 hunks)
  • templates/index.html (1 hunks)
💤 Files with no reviewable changes (1)
  • static/style.css
🧰 Additional context used
🪛 Ruff (0.8.2)
app.py

9-9: Redefinition of unused threading from line 3

Remove definition: threading

(F811)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
templates/index.html (5)

2-2: Review static dark mode class
Having class="dark" on the <html> tag forces the dark theme before your script runs and may cause a flash of dark styling even if the user prefers light. Consider removing the static dark class and relying solely on your JS initialization (localStorage or prefers-color-scheme) to set the theme.


7-7: Pin Font Awesome to a maintained version
You’re using Font Awesome v6.0.0 from the CDN. That version is over a year old—consider updating to the latest stable release or a security-patched version to avoid known vulnerabilities.


17-25: Add ARIA labels for icon buttons
Buttons like the theme-toggle, GitHub, and info buttons only contain <i> icons, which are not announced by screen readers. Add aria-label (or aria-labelledby) attributes to each button to improve accessibility.
Example:

-<button id="theme-toggle" class="text-white hover:text-cyan-400 transition-colors">
+<button id="theme-toggle" aria-label="Toggle theme" class="text-white hover:text-cyan-400 transition-colors">

597-599: Display console messages in the UI
Currently, eventSource.onmessage simply logs updates to the browser console. To give users live feedback, append incoming messages to a scrollable container (e.g., <div id="console-container">) in the page instead of—or in addition to—console.log().


727-731: Replace polling with SSE/WebSocket
You’re polling /check_stream every second, which can add unnecessary HTTP overhead. Consider pushing status updates over your existing SSE /console_updates stream or upgrading to a WebSocket solution to drive UI state changes in real time.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e65e05 and 6fb69b1.

📒 Files selected for processing (1)
  • templates/index.html (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
templates/index.html (2)

599-601: Render console updates in the UI
Currently onmessage only logs to console. To surface real-time feedback, append messages to a scrollable container in the DOM (as recommended earlier).


145-154: 🛠️ Refactor suggestion

Dynamic Tailwind classes may be purged
Using template literals like bg-${app.color}-100 and text-${app.color}-600 can be stripped out by PurgeCSS. Either safelist these patterns in tailwind.config.js or map app.color to explicit class strings so PurgeCSS recognizes them.

🧹 Nitpick comments (5)
templates/index.html (5)

2-2: Remove default dark theme class
Setting class="dark" on the <html> tag forces dark mode before your script runs, causing a flash of dark UI for users who prefer light theme. Consider removing this default and relying solely on your JS initialization to apply dark based on localStorage or prefers-color-scheme.


6-7: Add SRI and crossorigin to CDN links
Including integrity (integrity="") and crossorigin="anonymous" attributes on your Font Awesome (and later Tailwind) CDN imports will harden against CDN tampering.


81-94: Enhance modal accessibility
The BLE device modal is great, but consider adding role="dialog", aria-modal="true", and proper focus trapping when it’s shown to improve screen-reader and keyboard navigation support.

Also applies to: 96-99


181-189: Prevent flash of wrong theme on load
You initialize theme after DOM load, but since dark may be on <html>, users can see a brief incorrect theme. Consider inlining a small script in <head> to set the dark class before rendering, or remove the static dark class and rely exclusively on the inline script.


603-608: Implement SSE reconnection strategy
Add retry logic with exponential backoff in eventSource.onerror to handle transient network disruptions and improve resilience.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fb69b1 and e0d255a.

📒 Files selected for processing (1)
  • templates/index.html (1 hunks)
🔇 Additional comments (24)
templates/index.html (24)

9-18: Approve body and header markup
The <body> layout classes and sticky header implementation using Tailwind utilities look solid and responsive.


30-38: Approve connectivity and recording UI markup
The protocol selection buttons, Connect/Disconnect controls, filename input group, recording toggle, and status display are well-structured with clear IDs/classes matching your JS.

Also applies to: 47-51, 53-55, 59-66, 68-72, 74-77


103-116: Approve applications filter and grid layout
The Select dropdown for category filtering and the responsive grid container (#app-grid) correctly set up the applications section.

Also applies to: 119-120


131-142: Approve apps metadata initialization
Defining the apps array and fetching the #app-grid element is clear and concise.

Also applies to: 143-143


157-164: Approve application launch click handler
Checking isConnected before launching and invoking launchApplication(app.script) provides a clear flow and user feedback.


168-176: Approve theme toggle logic
The toggleTheme function and event binding correctly switch icons and persist the user’s preference in localStorage.

Also applies to: 179-179


192-205: Approve connection state variables
Defining selectedProtocol, selectedBleDevice, isConnected, isRecording, and eventSource at the top clearly captures the global state required for connection management.

Also applies to: 207-213


215-223: Approve filename utilities
Your getTimestamp(), initializeFilename(), sanitizeFilename(), and input listener correctly generate and sanitize filenames, preserving cursor position.

Also applies to: 252-257, 259-266, 268-276


279-292: Approve protocol selection handler
The click listener on .connection-btn elements cleanly highlights the selected protocol, manages modal display for BLE, and enables the Connect button appropriately.


295-303: Approve BLE scanning flow
Your scanBleDevices() function guards against duplicate scans, shows loading state, fetches /scan_ble, and handles both success and error cases with user feedback.

Also applies to: 305-313, 319-324, 331-339


343-353: Approve BLE device list rendering and selection
The renderBleDevices() logic creates clickable device entries, highlights the selected one, and ensures the Connect button is enabled only after a selection.

Also applies to: 355-365, 368-375


379-385: Approve BLE modal controls
The modal close and rescan button handlers properly reset state and re-trigger scanning.

Also applies to: 387-389


391-401: Approve Connect button click handler
The validation (protocol & BLE device), UI transitions to “Connecting…”, payload construction, fetch('/connect'), and polling via pollConnectionStatus() are well-structured and error-handled.

Also applies to: 404-442


445-488: Approve polling connection status logic
Your recursive checkStatus() with timeout and max attempts provides a robust approach to detect connection success or failure.


490-515: Approve connection success handling
On successful connect, you update UI controls, disable other protocols, and trigger console updates via SSE.


517-524: Approve application launch API integration
launchApplication() cleanly posts to /launch_app and handles both success and failure JSON responses.

Also applies to: 528-533


536-586: Approve Disconnect button handler and cleanup
The disconnect flow—including UI state transitions, /disconnect call, resetting buttons, stopping recording, and closing SSE—is comprehensive and properly error-handled.


592-610: Approve SSE setup for console updates
Using new EventSource('/console_updates') and cleaning up existing connections avoids duplicate streams.


613-620: Approve recording toggle implementation
The toggleRecording() function correctly handles start/stop flows, updates button labels/styles, and interacts with /start_recording and /stop_recording endpoints.

Also applies to: 623-628, 630-641, 645-661, 663-667


671-674: Approve initial UI setup
Disabling Connect until protocol selection and initializing filename on load ensures a consistent starting state.


674-678: Approve status and alert helpers
showStatus() and showAlert() provide centralized feedback mechanisms for various interactions.

Also applies to: 680-682


684-726: Approve stream status polling
Periodic /check_stream polling with UI toggles for connect/disconnect and cleanup of recording/SSE maintains sync between client and server.


729-733: Approve polling interval and bootstrap call
Calling checkStreamStatus() immediately and then every second ensures the UI stays up-to-date without user action.


734-740: Approve external link buttons
The GitHub and Info buttons open the correct URL and display version info via alert().

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (8)
connection.py (4)

60-82: 🛠️ Refactor suggestion

Use a context manager for CSV file handling.

Opening files without a context manager could lead to resource leaks if an exception occurs. The file might not be properly closed in all scenarios.

    def start_csv_recording(self, filename=None):
        if self.recording_active:
            return False
        
        try:
            if not filename:
                timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
                filename = f"ChordsPy_{timestamp}.csv"
            elif not filename.endswith('.csv'):
                filename += '.csv'
                
-            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)
+            self.csv_file = open(filename, 'w', newline='')
+            try:
+                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)
+                self.recording_active = True
+                self.sample_counter = 0
+                print(f"CSV recording started: {filename}")
+                return True
+            except Exception as e:
+                self.csv_file.close()
+                self.csv_file = None
+                raise e
-            self.recording_active = True
-            self.sample_counter = 0
-            print(f"CSV recording started: {filename}")
-            return True
        except Exception as e:
            print(f"Error starting CSV recording: {str(e)}")
            return False
🧰 Tools
🪛 Ruff (0.8.2)

71-71: Use a context manager for opening files

(SIM115)


183-220: ⚠️ Potential issue

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.

    def connect_wifi(self):
        self.wifi_connection = Chords_WIFI()
        self.wifi_connection.connect()
        
        self.num_channels = self.wifi_connection.channels
        sampling_rate = self.wifi_connection.sampling_rate

        if not self.lsl_connection:
            self.setup_lsl(self.num_channels, sampling_rate)
        
        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.recording_active:
-                            self.log_to_csv(channel_data)
+                try:
+                    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.recording_active:
+                                self.log_to_csv(channel_data)
+                except (websocket.WebSocketConnectionClosedException, ConnectionResetError) as e:
+                    print(f"\nConnection closed: {str(e)}")
+                    break
+                except Exception as e:
+                    print(f"\n[ERROR] Connection error: {e}")
+                    break
                    
        except KeyboardInterrupt:
-            self.wifi_connection.disconnect()
+            self.wifi_connection.cleanup()
            print("\nDisconnected")
        finally:
            self.stop_csv_recording()

216-217: ⚠️ Potential issue

disconnect() is undefined for Chords_WIFI.

Chords_WIFI only exposes cleanup(). Calling a non-existent method will raise AttributeError.

-            self.wifi_connection.disconnect()
+            self.wifi_connection.cleanup()

248-254: ⚠️ Potential issue

disconnect() is undefined for Chords_WIFI.

Similar to the previous issue, disconnect() is called in the cleanup method, but Chords_WIFI only exposes cleanup().

        if self.wifi_connection:
            try:
-                self.wifi_connection.disconnect()
+                self.wifi_connection.cleanup()
                print("WiFi connection closed")
            except Exception as e:
                print(f"Error closing WiFi connection: {str(e)}")
            finally:
                self.wifi_connection = None
templates/index.html (4)

145-154: Tailwind dynamic classes may be purged.

The app-card template uses dynamic class names (bg-${app.color}-100, dark:bg-${app.color}-900, text-${app.color}-600, etc.). Tailwind's PurgeCSS won't detect these at build time and may strip them out.

You have two options:

  1. Safelist all variants of these classes in your tailwind.config.js:
// tailwind.config.js
module.exports = {
  // ...
  safelist: [
    'bg-cyan-100', 'bg-cyan-900', 'text-cyan-600', 'text-cyan-300',
    'bg-red-100', 'bg-red-900', 'text-red-600', 'text-red-300',
    // Add other color combinations used
  ]
}
  1. Use a lookup object instead of dynamic interpolation:
const colorClasses = {
  cyan: {
    bg: 'bg-cyan-100',
    bgDark: 'dark:bg-cyan-900',
    text: 'text-cyan-600',
    textDark: 'dark:text-cyan-300'
  },
  red: {
    // ...other colors
  }
}

// Then in your template:
<div class="${colorClasses[app.color].bg} ${colorClasses[app.color].bgDark} ...">

612-631: Enhance SSE reconnection logic.

The startConsoleUpdates function tears down the EventSource on error but never attempts to reconnect, which may result in loss of console updates if there's a temporary network issue.

function startConsoleUpdates() {
    if (eventSource) {
        eventSource.close();
    }
    
    eventSource = new EventSource('/console_updates');
    
    eventSource.onmessage = function(e) {
        console.log('Console update:', e.data);
    };
    
+    let reconnectAttempts = 0;
+    const maxReconnectAttempts = 5;
+    
    eventSource.onerror = function() {
        console.error('EventSource failed');
+        
+        // Handle reconnection
+        if (reconnectAttempts < maxReconnectAttempts) {
+            reconnectAttempts++;
+            const timeout = Math.min(1000 * reconnectAttempts, 5000); // Exponential backoff with cap
+            console.log(`Attempting to reconnect in ${timeout/1000} seconds...`);
+            setTimeout(startConsoleUpdates, timeout);
+        } else {
+            console.error('Maximum reconnection attempts reached');
+            showStatus('Connection lost', 'fa-exclamation-circle', 'text-orange-500');
+        }
+        
        if (eventSource) {
            eventSource.close();
            eventSource = null;
        }
    };
}

619-622: Add UI console message display.

The EventSource for console updates correctly receives data, but the messages aren't displayed to the user in the UI.

eventSource.onmessage = function(e) {
    console.log('Console update:', e.data);
+    // Display message in UI if it's not a heartbeat
+    if (e.data !== "heartbeat") {
+        const consoleDiv = document.createElement('div');
+        consoleDiv.className = 'text-sm text-gray-600 dark:text-gray-400 my-1';
+        consoleDiv.textContent = e.data;
+        
+        // Find a container to append the message, or create one if it doesn't exist
+        let consoleContainer = document.getElementById('console-container');
+        if (!consoleContainer) {
+            consoleContainer = document.createElement('div');
+            consoleContainer.id = 'console-container';
+            consoleContainer.className = 'mt-2 py-2 px-4 border rounded-lg bg-gray-50 dark:bg-gray-800 max-h-40 overflow-y-auto';
+            statusDiv.parentNode.insertBefore(consoleContainer, statusDiv.nextSibling);
+        }
+        
+        consoleContainer.appendChild(consoleDiv);
+        consoleContainer.scrollTop = consoleContainer.scrollHeight;
+    }
};

767-779: 🛠️ Refactor suggestion

Move Tailwind config before CDN import.

Your tailwind.config script appears after loading https://cdn.tailwindcss.com, so customizations won't apply on first render.

-    <script src="https://cdn.tailwindcss.com"></script>
-    <script>
-        tailwind.config = {
-            darkMode: 'class',
-            theme: {
-                extend: {
-                    cursor: {
-                        'not-allowed': 'not-allowed',
-                    }
-                }
-            }
-        }
-    </script>
+    <script>
+        tailwind.config = {
+            darkMode: 'class',
+            theme: {
+                extend: {
+                    cursor: {
+                        'not-allowed': 'not-allowed',
+                    }
+                }
+            }
+        }
+    </script>
+    <script src="https://cdn.tailwindcss.com"></script>
🧹 Nitpick comments (5)
chords_serial.py (3)

5-5: Remove unused import.

The sys module is imported but never used in the codebase.

-import sys
🧰 Tools
🪛 Ruff (0.8.2)

5-5: sys imported but unused

Remove unused import: sys

(F401)


61-61: Unused variable assignment.

The sampling_rate variable is assigned but never used within this function.

-                    sampling_rate = self.supported_boards[self.board]["sampling_rate"]

If this was for future reference or debugging, consider adding a comment explaining its purpose.

🧰 Tools
🪛 Ruff (0.8.2)

61-61: Local variable sampling_rate is assigned to but never used

Remove assignment to unused variable sampling_rate

(F841)


153-156: Missing error handling in main block.

The main block runs the hardware detection and streaming without any error handling, which could lead to unhandled exceptions if hardware detection fails.

if __name__ == "__main__":
    client = Chords_USB()
-    client.detect_hardware()
-    client.start_streaming()
+    try:
+        if client.detect_hardware():
+            client.start_streaming()
+        else:
+            print("No compatible hardware detected. Exiting.")
+    except KeyboardInterrupt:
+        print("\nProgram terminated by user.")
+    except Exception as e:
+        print(f"Error: {str(e)}")
+    finally:
+        client.cleanup()
connection.py (1)

7-7: Remove unused import.

The sys module is imported but never used in the codebase.

-import sys
🧰 Tools
🪛 Ruff (0.8.2)

7-7: sys imported but unused

Remove unused import: sys

(F401)

templates/index.html (1)

705-751: Make connection status checking more explicit.

The check for connection status uses if (data.connected) which could be more explicit to avoid type coercion issues.

function checkStreamStatus() {
    fetch('/check_stream')
        .then(response => response.json())
        .then(data => {
-            if (data.connected) {
+            if (data.connected === true) {
                // If connected, update the frontend
                if (!isConnected) {
                    // ...
                }
            } else {
                // ...
            }
        })
        // ...
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 04296e6 and 66e0ff0.

📒 Files selected for processing (3)
  • chords_serial.py (1 hunks)
  • connection.py (1 hunks)
  • templates/index.html (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
chords_serial.py (3)
chords_wifi.py (1)
  • cleanup (115-124)
connection.py (1)
  • cleanup (221-258)
chords_ble.py (1)
  • cleanup (151-159)
connection.py (3)
chords_serial.py (5)
  • Chords_USB (9-151)
  • detect_hardware (73-84)
  • read_data (96-126)
  • start_streaming (128-136)
  • cleanup (142-148)
chords_wifi.py (3)
  • Chords_WIFI (7-127)
  • connect (25-34)
  • cleanup (115-124)
chords_ble.py (5)
  • Chords_BLE (8-178)
  • scan_devices (36-45)
  • notification_handler (82-93)
  • connect (161-172)
  • cleanup (151-159)
🪛 Ruff (0.8.2)
chords_serial.py

5-5: sys imported but unused

Remove unused import: sys

(F401)


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)


71-71: Use a context manager for opening files

(SIM115)

🔇 Additional comments (18)
chords_serial.py (7)

31-42: Good initialization practice with signal handler thread safety.

The initialization is well-designed with appropriate member variables and thread-safe signal handler registration limited to the main thread to avoid duplicate signal handlers in threaded environments.


44-72: Well-implemented hardware connection with robust retry mechanism.

The connection implementation includes proper error handling, retries, and validation of device responses. It correctly configures the packet length and data arrays based on the detected board.

🧰 Tools
🪛 Ruff (0.8.2)

61-61: Local variable sampling_rate is assigned to but never used

Remove assignment to unused variable sampling_rate

(F841)


73-85: Efficient hardware detection implementation.

The method appropriately tries different baudrates and ports combinations to detect compatible hardware, with clean handling of success and failure cases.


86-95: Clean serial command implementation.

The send_command method properly flushes I/O buffers, adds error tolerance with utf-8 decoding, and handles the possibility of a closed serial connection.


96-127: Robust packet parsing with proper error handling.

The data reading implementation includes:

  1. Proper detection of serial disconnection
  2. Efficient packet boundary detection
  3. Validation of packet structure with sync and end bytes
  4. Clean extraction of multi-channel sensor data
  5. Appropriate handling of serial exceptions with cleanup

This is a well-implemented data acquisition routine for binary packet protocols.


128-137: Streaming implementation with proper exception handling.

The streaming implementation starts the data flow and handles keyboard interrupts appropriately by triggering cleanup.


142-149: Clean resource management in cleanup method.

The cleanup method properly stops streaming and closes serial connections with exception handling, which is essential for proper resource management.

connection.py (6)

13-32: Well-structured Connection class initialization.

The Connection class has a clean initialization with appropriate attributes for different connection types, LSL stream configuration, and state tracking.


33-52: Good implementation of BLE device discovery and selection.

The async method for BLE device discovery presents devices to the user and handles selection appropriately, with proper error handling for user input and cancellation.


111-155: Robust BLE connection handling with notification interception.

The connection method correctly wraps the original notification handler to intercept data for LSL streaming and CSV recording. It properly handles device address selection and user interaction.


157-181: Clean USB connection implementation with thread safety.

The USB connection method properly detects hardware and sets up a background thread for streaming with daemon=True for automatic cleanup. The method wraps the original read_data method to intercept data for LSL and CSV recording without modifying the original class.


221-258: Well-implemented cleanup with proper resource management.

The cleanup method properly handles all connection types, stops CSV recording, and resets stream flags. It includes proper exception handling and resource cleanup in finally blocks.


263-282: Well-designed CLI entry point.

The main function provides a well-structured command-line interface for connecting to devices with proper error handling and protocol selection.

templates/index.html (5)

1-9: Clean HTML structure with dark mode support.

The HTML is well-structured with proper meta tags, title, font awesome integration, and dark mode enabled by default.


10-28: Responsive header with theme toggling and navigation.

The header is well-designed with a gradient title, theme toggle, GitHub link, and info button using Tailwind's responsive utility classes.


30-78: Well-organized connectivity section with clear protocol options.

The connectivity section provides a clear interface for protocol selection (WiFi, Bluetooth, Serial), connection controls, filename input with CSV extension, and recording functionality. The layout is responsive with appropriate spacing and visual hierarchy.


80-101: Well-designed BLE device selection modal.

The BLE device selection modal provides a clean interface for scanning and selecting devices with appropriate loading states and rescan functionality.


103-122: Responsive application grid with filtering.

The application section provides a filterable grid of applications with appropriate spacing and responsive layouts using Tailwind's grid system.

@PayalLakra PayalLakra changed the title Changes in UI and 3 different protocols-USB, WIFI, BLE added Updated UI with different protocols-USB, WIFI, BLE added May 8, 2025
@PayalLakra
Copy link
Member Author

@lorforlinux
I have made following changes:

  • Updated UI with 3 different connection buttons for Serial, Wifi, Bluetooth
  • Disconnect functionality added
  • CSV Recording from interface itself

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (11)
app.py (7)

60-66: Inconsistent stream status management

This function updates a global stream_active variable, which duplicates the state already tracked in connection_manager.stream_active. This can lead to inconsistent state management.

def post_console_message(message):
-    global stream_active
+    global connection_manager
    if "LSL stream started" in message:
-        stream_active = True
+        if connection_manager:
+            connection_manager.stream_active = True
    elif "disconnected" in message:
-        stream_active = False
+        if connection_manager:
+            connection_manager.stream_active = False
    console_queue.put(message)

68-75: Potential resource leak in event stream

The console_updates function creates an infinite loop that will run until the client disconnects. There's no mechanism to detect client disconnection or to stop the stream gracefully, which could lead to resource leaks.

@app.route('/console_updates')
def console_updates():
    def event_stream():
        while True:
-            message = console_queue.get()
+            try:
+                message = console_queue.get(timeout=30)  # Add timeout to prevent indefinite blocking
+            except queue.Empty:
+                # Send a heartbeat to detect disconnected clients
+                yield "data: heartbeat\n\n"
+                continue
            yield f"data: {message}\n\n"
    
    return Response(event_stream(), mimetype="text/event-stream")

117-124: Redundant stream status setting

The code references a global variable stream_active that duplicates state in connection_manager.stream_active. This should be simplified.

def connect_device():
-    global connection_manager, connection_thread, stream_active
+    global connection_manager, connection_thread
    
    data = request.get_json()
    protocol = data.get('protocol')
    device_address = data.get('device_address')
    
    # Reset stream status
-    stream_active = False
+    if connection_manager:
+        connection_manager.stream_active = False

127-131: Race condition risk in connection cleanup

There's a potential race condition in the cleanup process. If connection_thread is running, the code joins it but doesn't set a timeout, which could block indefinitely if the thread is stuck.

Apply this diff to add a timeout to the join operation:

    # Clean up any existing connection
    if connection_manager:
        connection_manager.cleanup()
        if connection_thread and connection_thread.is_alive():
-            connection_thread.join()
+            connection_thread.join(timeout=5)  # Add timeout to prevent indefinite blocking
+            if connection_thread.is_alive():
+                logging.warning("Connection thread did not terminate within timeout")

141-150: BLE connection creates but doesn't close asyncio event loop

For BLE connections, an event loop is created but never explicitly closed, which may lead to resource leaks and warnings. The finally block should close the loop.

Apply this diff to fix the issue:

            elif protocol == 'ble':
                if not device_address:
                    logging.error("No BLE device address provided")
                    return
                
                # For BLE, we need to run in an event loop
                loop = asyncio.new_event_loop()
                asyncio.set_event_loop(loop)
-                success = connection_manager.connect_ble(device_address)
+                try:
+                    success = connection_manager.connect_ble(device_address)
+                finally:
+                    loop.close()

165-173: Inconsistent stream status management in disconnect handler

The disconnect handler updates a global stream_active variable, but it should be using connection_manager.stream_active to maintain consistency with the rest of the code.

@app.route('/disconnect', methods=['POST'])
def disconnect_device():
-    global connection_manager, stream_active
+    global connection_manager
    if connection_manager:
        connection_manager.cleanup()
-        stream_active = False
        post_console_message("disconnected")
        return jsonify({'status': 'disconnected'})
    return jsonify({'status': 'no active connection'})

211-212: Debug mode enabled in production code

Running Flask with debug=True is not recommended for production environments as it exposes sensitive information and can be a security risk.

if __name__ == "__main__":
-    app.run(debug=True)
+    app.run(debug=False)

For development, consider using environment variables to control debug mode:

import os
if __name__ == "__main__":
    debug_mode = os.environ.get('FLASK_DEBUG', 'False').lower() == 'true'
    app.run(debug=debug_mode)
🧰 Tools
🪛 ast-grep (0.31.1)

[warning] 211-211: 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)

templates/index.html (4)

130-142: Move Tailwind config before CDN import

Your tailwind.config script appears after loading https://cdn.tailwindcss.com, so customizations won't apply on first render. Per the CDN docs, inline the config script before the CDN <script> tag.

    <footer class="bg-gray-900 dark:bg-gray-800 text-white sticky bottom-0 z-50">
        <div class="max-w-full px-8 py-3 text-center">
            Made with ❤️ at Upside Down Labs
        </div>
    </footer>
+    <script>
+        tailwind.config = {
+            darkMode: 'class',
+            theme: {
+                extend: {
+                    cursor: {
+                        'not-allowed': 'not-allowed',
+                    }
+                }
+            }
+        }
+    </script>
    <script src="https://cdn.tailwindcss.com"></script>
-    <script>
-        tailwind.config = {
-            darkMode: 'class',
-            theme: {
-                extend: {
-                    cursor: {
-                        'not-allowed': 'not-allowed',
-                    }
-                }
-            }
-        }
-    </script>

158-167: Tailwind dynamic classes may be purged

The app-card template uses dynamic class names (bg-${app.color}-100, dark:bg-${app.color}-900, text-${app.color}-600, etc.). Tailwind's PurgeCSS won't detect these at build time and may strip them out.

You have two options:

  1. Safelist all variants of these classes in your tailwind.config.js safelist array.
  2. Build the class list explicitly (e.g., use a lookup object) so that PurgeCSS sees the full set of class names.

For option 2, you could use a lookup object:

const colorClasses = {
    cyan: {
        bg: 'bg-cyan-100',
        bgDark: 'dark:bg-cyan-900',
        text: 'text-cyan-600',
        textDark: 'dark:text-cyan-300'
    },
    red: {
        bg: 'bg-red-100',
        bgDark: 'dark:bg-red-900',
        text: 'text-red-600',
        textDark: 'dark:text-red-300'
    },
    // Add entries for all your colors
};

// Then in your template:
`<div class="w-full h-32 ${colorClasses[app.color].bg} ${colorClasses[app.color].bgDark} rounded-lg flex items-center justify-center mb-4">
    <i class="fas ${app.icon} text-4xl ${colorClasses[app.color].text} ${colorClasses[app.color].textDark}"></i>
</div>`

625-644: Enhance SSE reconnection logic

The startConsoleUpdates function tears down the EventSource on error but never attempts to reconnect. For more robust console streaming, implement an exponential-backoff retry strategy.

function startConsoleUpdates() {
    if (eventSource) {
        eventSource.close();
    }
    
+    let reconnectAttempts = 0;
+    const maxReconnectAttempts = 5;
    
    eventSource = new EventSource('/console_updates');
    
    eventSource.onmessage = function(e) {
        console.log('Console update:', e.data);
    };
    
    eventSource.onerror = function() {
        console.error('EventSource failed');
+        
+        // Handle reconnection
+        if (reconnectAttempts < maxReconnectAttempts) {
+            reconnectAttempts++;
+            const timeout = Math.min(1000 * reconnectAttempts, 5000); // Exponential backoff with cap
+            console.log(`Attempting to reconnect in ${timeout/1000} seconds...`);
+            setTimeout(startConsoleUpdates, timeout);
+        } else {
+            console.error('Maximum reconnection attempts reached');
+            showStatus('Connection lost', 'fa-exclamation-circle', 'text-orange-500');
+        }
+        
        if (eventSource) {
            eventSource.close();
            eventSource = null;
        }
    };
}

633-635: 🛠️ Refactor suggestion

Console messages not displayed in UI

The EventSource for console updates correctly receives data, but the messages aren't displayed to the user in the UI. The onmessage handler only logs to the console, but doesn't update the UI with the messages.

eventSource.onmessage = function(e) {
    console.log('Console update:', e.data);
+    // Display message in UI if not a heartbeat
+    if (e.data !== "heartbeat") {
+        const consoleDiv = document.createElement('div');
+        consoleDiv.className = 'text-sm text-gray-600 dark:text-gray-400 my-1';
+        consoleDiv.textContent = e.data;
+        
+        // Find a container to append the message, or create one if it doesn't exist
+        let consoleContainer = document.getElementById('console-container');
+        if (!consoleContainer) {
+            consoleContainer = document.createElement('div');
+            consoleContainer.id = 'console-container';
+            consoleContainer.className = 'mt-2 py-2 px-4 border rounded-lg bg-gray-50 dark:bg-gray-800 max-h-40 overflow-y-auto';
+            statusDiv.parentNode.insertBefore(consoleContainer, statusDiv.nextSibling);
+        }
+        
+        consoleContainer.appendChild(consoleDiv);
+        consoleContainer.scrollTop = consoleContainer.scrollHeight;
+    }
};
🧹 Nitpick comments (2)
app.py (2)

3-9: Fix imports to follow best practices

The imports should be organized more clearly: group standard library imports, third-party imports, and local imports together. The Flask imports are also split across multiple lines.

from flask import Flask, render_template, request, jsonify, Response
from connection import Connection
import threading
import asyncio
import logging
from bleak import BleakScanner
-from flask import Response
import queue

92-96: Imports inside function should be moved to top of file

The subprocess and sys imports should be at the top of the file with other imports, not inside a function. This improves readability and prevents repeated imports.

from flask import Flask, render_template, request, jsonify, Response
from connection import Connection
import threading
import asyncio
import logging
from bleak import BleakScanner
import queue
+import subprocess
+import sys

# Rest of the code...

@app.route('/launch_app', methods=['POST'])
def launch_application():
    # ...
    try:
-        import subprocess
-        import sys
        
        python_exec = sys.executable
        # ...
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 66e0ff0 and 8b7a214.

📒 Files selected for processing (3)
  • app.py (1 hunks)
  • requirements.txt (1 hunks)
  • templates/index.html (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • requirements.txt
🧰 Additional context used
🧬 Code Graph Analysis (1)
app.py (1)
connection.py (7)
  • Connection (13-261)
  • cleanup (221-258)
  • connect_usb (157-181)
  • connect_wifi (183-219)
  • connect_ble (111-155)
  • start_csv_recording (60-81)
  • stop_csv_recording (83-97)
🔇 Additional comments (2)
app.py (1)

48-52:

❓ Verification inconclusive

Inconsistent stream status checking endpoint

The /check_stream endpoint returns {'connected': True/False}, but the frontend API clients might expect a status field for consistency with other endpoints like /check_connection which returns a status field.


🏁 Script executed:

#!/bin/bash
# Check if the frontend is expecting a 'status' field in the response
rg -A 5 $'/check_stream'.*\.then.*json templates

Length of output: 48


I couldn’t find any references to how /check_connection is implemented or consumed in the frontend. Let’s first verify the implementations of both endpoints in app.py so we can confirm what each returns:

#!/bin/bash
# Show the /check_connection endpoint implementation
rg "app\.route.*check_connection" -n app.py -A 5
# Show the /check_stream endpoint implementation
rg "app\.route.*check_stream" -n app.py -A 5

Let’s retrieve both endpoint implementations to compare their response keys:


🏁 Script executed:

#!/bin/bash
# Show /check_connection implementation
rg -n "app\.route\('/check_connection'\)" -A 5 app.py
# Show /check_stream implementation
rg -n "app\.route\('/check_stream'\)" -A 5 app.py

Length of output: 567


Standardize /check_stream response to use status

Currently in app.py:

  • /check_connection returns { status: 'connected' | 'connecting' }
  • /check_stream returns { connected: true | false }

For consistency (and to avoid frontend confusion), consider having /check_stream return a status field as well. For example:

@app.route('/check_stream')
-def check_stream():
-    if connection_manager and connection_manager.stream_active:
-        return jsonify({'connected': True})
-    return jsonify({'connected': False})
+def check_stream():
+    status = (
+        'connected'
+        if connection_manager and connection_manager.stream_active
+        else 'connecting'
+    )
+    return jsonify({'status': status})

• Please verify that your frontend clients expect and handle the status field for both endpoints.

templates/index.html (1)

718-722:

✅ Verification successful

Consistent stream status checking in checkStreamStatus

Ensure the checkStreamStatus function's check is consistent with the expected response format from the /check_stream endpoint for more robust error handling.


🏁 Script executed:

#!/bin/bash
# Compare implementation of stream status checking with backend response format
echo "Backend response format from /check_stream:"
rg -A2 "check_stream" app.py | grep "return jsonify"

echo "\nFrontend stream status checking implementation:"
rg -A5 "data.connected" templates/index.html

Length of output: 673


No changes needed for checkStreamStatus
The frontend already checks the same connected boolean that the /check_stream endpoint returns (jsonify({'connected': True})), so the implementation is consistent.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (13)
app.py (10)

68-73: Inconsistent stream status response format

The /check_stream endpoint returns {'connected': true/false}, but previous review comments noted that the frontend expects a status field to be included.

@app.route('/check_stream')
def check_stream():
    if connection_manager and connection_manager.stream_active:
-        return jsonify({'connected': True})
-    return jsonify({'connected': False})
+        return jsonify({'connected': True, 'status': 'connected'})
+    return jsonify({'connected': False, 'status': 'disconnected'})

19-20: 🛠️ Refactor suggestion

Remove redundant global stream_active variable

The global stream_active variable duplicates functionality already provided by connection_manager.stream_active, which leads to inconsistent state management across the codebase.

This was previously identified in a review comment and should be removed. All references to this global variable should use connection_manager.stream_active instead.


80-87: 🛠️ Refactor suggestion

Inconsistent stream status management

This function updates a global stream_active variable instead of using connection_manager.stream_active, leading to state inconsistency.

def post_console_message(message):
-    global stream_active
+    global connection_manager
    if "LSL stream started" in message:
-        stream_active = True
+        if connection_manager:
+            connection_manager.stream_active = True
    elif "disconnected" in message:
-        stream_active = False
+        if connection_manager:
+            connection_manager.stream_active = False
    console_queue.put(message)

88-96: 🛠️ Refactor suggestion

Add timeout and heartbeat to EventSource stream

The console updates function creates an infinite loop with no mechanism to detect client disconnection or to stop the stream gracefully, which could lead to resource leaks.

@app.route('/console_updates')
def console_updates():
    def event_stream():
        while True:
-            message = console_queue.get()
+            try:
+                message = console_queue.get(timeout=30)  # Add timeout to prevent indefinite blocking
+            except queue.Empty:
+                # Send a heartbeat to detect disconnected clients
+                yield "data: heartbeat\n\n"
+                continue
            yield f"data: {message}\n\n"
    
    return Response(event_stream(), mimetype="text/event-stream")

135-138: 🛠️ Refactor suggestion

Reference to undefined global variable

Line 137 references the global variable stream_active, but this should be using connection_manager.stream_active instead.

@app.route('/connect', methods=['POST'])
def connect_device():
-    global connection_manager, connection_thread, stream_active
+    global connection_manager, connection_thread

143-144: 🛠️ Refactor suggestion

Reset stream status using connection manager

The code resets a global stream_active variable instead of using connection_manager.stream_active.

    # Reset stream status
-    stream_active = False
+    if connection_manager:
+        connection_manager.stream_active = False

147-151: 🛠️ Refactor suggestion

Race condition risk in connection cleanup

The code joins the connection thread without setting a timeout, which could block indefinitely if the thread is stuck.

    # Clean up any existing connection
    if connection_manager:
        connection_manager.cleanup()
        if connection_thread and connection_thread.is_alive():
-            connection_thread.join()
+            connection_thread.join(timeout=5)  # Add timeout to prevent indefinite blocking
+            if connection_thread.is_alive():
+                logging.warning("Connection thread did not terminate within timeout")

161-170: 🛠️ Refactor suggestion

BLE connection creates but doesn't close asyncio event loop

For BLE connections, an event loop is created but not explicitly closed in case of exceptions, which may lead to resource leaks.

            elif protocol == 'ble':
                if not device_address:
                    logging.error("No BLE device address provided")
                    return
                
                # For BLE, we need to run in an event loop
                loop = asyncio.new_event_loop()
                asyncio.set_event_loop(loop)
-                success = connection_manager.connect_ble(device_address)
+                try:
+                    success = connection_manager.connect_ble(device_address)
+                finally:
+                    loop.close()

185-193: 🛠️ Refactor suggestion

Inconsistent stream status management in disconnect handler

The disconnect handler directly updates the global stream_active variable instead of using connection_manager.stream_active.

@app.route('/disconnect', methods=['POST'])
def disconnect_device():
-    global connection_manager, stream_active
+    global connection_manager
    if connection_manager:
        connection_manager.cleanup()
-        stream_active = False
        post_console_message("disconnected")
        return jsonify({'status': 'disconnected'})
    return jsonify({'status': 'no active connection'})

231-232: 🛠️ Refactor suggestion

Debug mode enabled in production code

Running Flask with debug=True is not recommended for production environments as it exposes sensitive information and can be a security risk.

if __name__ == "__main__":
-    app.run(debug=True)
+    app.run(debug=False)

For development, consider using environment variables to control debug mode:

import os
if __name__ == "__main__":
    debug_mode = os.environ.get('FLASK_DEBUG', 'False').lower() == 'true'
    app.run(debug=debug_mode)
🧰 Tools
🪛 ast-grep (0.31.1)

[warning] 231-231: 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)

templates/index.html (3)

200-272: Tailwind dynamic classes may be purged

The app card template uses dynamic class names like bg-${app.color}-100, which Tailwind's PurgeCSS won't detect at build time.

Consider either:

  1. Safelisting these color variants in your tailwind.config.js
  2. Building the class list explicitly using a lookup object
// Example of a color class lookup approach
const colorClasses = {
  red: {
    bg: 'bg-red-100',
    darkBg: 'dark:bg-red-900',
    text: 'text-red-600',
    darkText: 'dark:text-red-300'
  },
  blue: {
    bg: 'bg-blue-100',
    darkBg: 'dark:bg-blue-900',
    text: 'text-blue-600',
    darkText: 'dark:text-blue-300'
  },
  // Add other colors here
};

// Then use it like:
// <div class="${colorClasses[app.color].bg} ${colorClasses[app.color].darkBg}">

130-142: Move Tailwind config before CDN import

Your Tailwind config script appears after loading the CDN, so customizations might not apply on first render.

-    <script src="https://cdn.tailwindcss.com"></script>
-    <script>
-        tailwind.config = {
-            darkMode: 'class',
-            theme: {
-                extend: {
-                    cursor: {
-                        'not-allowed': 'not-allowed',
-                    }
-                }
-            }
-        }
-    </script>
+    <script>
+        tailwind.config = {
+            darkMode: 'class',
+            theme: {
+                extend: {
+                    cursor: {
+                        'not-allowed': 'not-allowed',
+                    }
+                }
+            }
+        }
+    </script>
+    <script src="https://cdn.tailwindcss.com"></script>

757-776: 🛠️ Refactor suggestion

Enhance SSE reconnection logic

The startConsoleUpdates function lacks proper reconnection logic when the EventSource encounters an error.

// Start console updates via SSE
function startConsoleUpdates() {
    if (eventSource) {
        eventSource.close();
    }
    
    eventSource = new EventSource('/console_updates');
    
    eventSource.onmessage = function(e) {
        console.log('Console update:', e.data);
+       // Display console message in UI
+       if (e.data !== "heartbeat") {
+           const consoleDiv = document.getElementById('console-container');
+           if (consoleDiv) {
+               const message = document.createElement('div');
+               message.className = 'text-sm text-gray-600 dark:text-gray-400 my-1';
+               message.textContent = e.data;
+               consoleDiv.appendChild(message);
+               consoleDiv.scrollTop = consoleDiv.scrollHeight;
+           }
+       }
    };
    
+   let reconnectAttempts = 0;
+   const maxReconnectAttempts = 5;
+   
    eventSource.onerror = function() {
        console.error('EventSource failed');
        if (eventSource) {
            eventSource.close();
            eventSource = null;
+           
+           // Handle reconnection
+           if (reconnectAttempts < maxReconnectAttempts) {
+               reconnectAttempts++;
+               const timeout = Math.min(1000 * reconnectAttempts, 5000); // Exponential backoff
+               console.log(`Reconnecting in ${timeout/1000} seconds...`);
+               setTimeout(startConsoleUpdates, timeout);
+           } else {
+               console.error('Maximum reconnection attempts reached');
+               showStatus('Connection lost', 'fa-exclamation-circle', 'text-orange-500');
+           }
        }
    };
}
🧹 Nitpick comments (2)
app.py (1)

1-1: Remove unused import

The send_from_directory function is imported but never used in this file.

-from flask import Flask, render_template, request, jsonify, send_from_directory
+from flask import Flask, render_template, request, jsonify
🧰 Tools
🪛 Ruff (0.8.2)

1-1: flask.send_from_directory imported but unused

Remove unused import: flask.send_from_directory

(F401)

templates/index.html (1)

912-914: Add version number source from server

The version number is hardcoded in the frontend, which will require frontend changes whenever the version changes.

Consider fetching the version number from the server or a configuration file:

document.getElementById('info-btn').addEventListener('click', () => {
-   alert('Chords Python - Biopotential Data Acquisition System\nVersion 2.1.0');
+   fetch('/api/version')
+     .then(response => response.json())
+     .then(data => {
+       alert(`Chords Python - Biopotential Data Acquisition System\nVersion ${data.version}`);
+     })
+     .catch(err => {
+       alert('Chords Python - Biopotential Data Acquisition System\nVersion information unavailable');
+     });
});

You would need to add a corresponding endpoint in the backend:

@app.route('/api/version')
def get_version():
    return jsonify({'version': '2.1.0'})
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b7a214 and 6125a44.

📒 Files selected for processing (4)
  • app.py (1 hunks)
  • config/apps.yaml (1 hunks)
  • requirements.txt (1 hunks)
  • templates/index.html (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • config/apps.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • requirements.txt
🧰 Additional context used
🧬 Code Graph Analysis (1)
app.py (1)
connection.py (7)
  • Connection (13-261)
  • cleanup (221-258)
  • connect_usb (157-181)
  • connect_wifi (183-219)
  • connect_ble (111-155)
  • start_csv_recording (60-81)
  • stop_csv_recording (83-97)
🪛 Ruff (0.8.2)
app.py

1-1: flask.send_from_directory imported but unused

Remove unused import: flask.send_from_directory

(F401)

🔇 Additional comments (8)
app.py (3)

55-67: Good implementation of BLE device scanning

The BLE scanning endpoint is well-implemented with proper async handling using the run_async decorator, error handling, and meaningful responses.


195-215: Well-implemented recording start endpoint

The /start_recording endpoint is properly implemented with good error handling and sends appropriate status responses.


217-229: Well-implemented recording stop endpoint

The /stop_recording endpoint is properly implemented with good error handling and consistent response formats.

templates/index.html (5)

33-45: Well-structured protocol selection UI

The protocol buttons (WiFi, Bluetooth, Serial) are well-designed and structured with clear visual elements, proper icons, and intuitive labels.


59-66: Well-designed recording filename input

The recording input field with the fixed CSV extension indicator provides a good user experience and maintains file format consistency.


80-101: Well-implemented BLE device selection modal

The BLE device selection modal is well-structured with proper loading state, device listing, and selection functionality.


143-196: Well-implemented application loading and error handling

The application loading functionality includes proper loading states, error handling, and user feedback.


850-896: Consistent stream status checking in checkStreamStatus

This function checks data.connected directly, which is correct based on the current backend response format, but be aware that there was a suggestion to modify the backend to include a status field.

However, make sure the backend and frontend expectations align. If you implement the suggested change to the /check_stream endpoint to include a status field, you might need to update this code as well.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (8)
app.py (8)

20-20: Redundant global state variable.

This global variable duplicates state already tracked in connection_manager.stream_active, which can lead to inconsistent state management.


64-68: Inconsistent stream status checking endpoint.

The /check_stream endpoint returns {'connected': True/False}, but the frontend (in index.html) expects a status field. This mismatch will cause the connection status to not update properly.

-    if connection_manager and connection_manager.stream_active:
-        return jsonify({'connected': True})
-    return jsonify({'connected': False})
+    if connection_manager and connection_manager.stream_active:
+        return jsonify({'connected': True, 'status': 'connected'})
+    return jsonify({'connected': False, 'status': 'disconnected'})

76-82: Inconsistent stream status management

This function updates a global stream_active variable, which duplicates the state already tracked in connection_manager.stream_active. This can lead to inconsistent state management.

def post_console_message(message):
-    global stream_active
+    global connection_manager
    if "LSL stream started" in message:
-        stream_active = True
+        if connection_manager:
+            connection_manager.stream_active = True
    elif "disconnected" in message:
-        stream_active = False
+        if connection_manager:
+            connection_manager.stream_active = False
    console_queue.put(message)

84-91: Potential resource leak in event stream.

The console_updates function creates an infinite loop that will run until the client disconnects. There's no mechanism to detect client disconnection or to stop the stream gracefully, which could lead to resource leaks.

@app.route('/console_updates')
def console_updates():
    def event_stream():
        while True:
-            message = console_queue.get()
+            try:
+                message = console_queue.get(timeout=30)  # Add timeout to prevent indefinite blocking
+            except queue.Empty:
+                # Send a heartbeat to detect disconnected clients
+                yield "data: heartbeat\n\n"
+                continue
            yield f"data: {message}\n\n"
    
    return Response(event_stream(), mimetype="text/event-stream")

143-147: Race condition risk in connection cleanup.

There's a potential race condition in the cleanup process. If connection_thread is running, the code joins it but doesn't set a timeout, which could block indefinitely if the thread is stuck.

    # Clean up any existing connection
    if connection_manager:
        connection_manager.cleanup()
        if connection_thread and connection_thread.is_alive():
-            connection_thread.join()
+            connection_thread.join(timeout=5)  # Add timeout to prevent indefinite blocking
+            if connection_thread.is_alive():
+                logging.warning("Connection thread did not terminate within timeout")

157-166: BLE connection creates but doesn't close asyncio event loop.

For BLE connections, an event loop is created but never explicitly closed, which may lead to resource leaks and warnings. The finally block should close the loop.

            elif protocol == 'ble':
                if not device_address:
                    logging.error("No BLE device address provided")
                    return
                
                # For BLE, we need to run in an event loop
                loop = asyncio.new_event_loop()
                asyncio.set_event_loop(loop)
-                success = connection_manager.connect_ble(device_address)
+                try:
+                    success = connection_manager.connect_ble(device_address)
+                finally:
+                    loop.close()

181-189: Inconsistent stream status management in disconnect handler.

The disconnect handler updates a global stream_active variable, but it should be using connection_manager.stream_active to maintain consistency with the rest of the code.

@app.route('/disconnect', methods=['POST'])
def disconnect_device():
-    global connection_manager, stream_active
+    global connection_manager
    if connection_manager:
        connection_manager.cleanup()
-        stream_active = False
        post_console_message("disconnected")
        return jsonify({'status': 'disconnected'})
    return jsonify({'status': 'no active connection'})

227-228: Debug mode enabled in production code.

Running Flask with debug=True is not recommended for production environments as it exposes sensitive information and can be a security risk.

if __name__ == "__main__":
-    app.run(debug=True)
+    app.run(debug=False)

For development, consider using environment variables to control debug mode:

import os
if __name__ == "__main__":
    debug_mode = os.environ.get('FLASK_DEBUG', 'False').lower() == 'true'
    app.run(debug=debug_mode)
🧰 Tools
🪛 ast-grep (0.31.1)

[warning] 227-227: 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)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6125a44 and 4a843e5.

📒 Files selected for processing (1)
  • app.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app.py (1)
connection.py (7)
  • Connection (13-261)
  • cleanup (221-258)
  • connect_usb (157-181)
  • connect_wifi (183-219)
  • connect_ble (111-155)
  • start_csv_recording (60-81)
  • stop_csv_recording (83-97)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (9)
templates/index.html (9)

6-7: Include SRI and crossorigin on external CDNs.
Linking to Font Awesome over the public CDN is fine, but you should add integrity and crossorigin="anonymous" attributes to guard against tampering and improve security.


17-25: Icon-only buttons need accessible labels.
Buttons like the theme toggle, GitHub, and info icons lack aria-label (or visually hidden text), which will be confusing for screen readers.
Example:

- <button id="theme-toggle" class="...">
+ <button id="theme-toggle" class="..." aria-label="Toggle dark/light theme">

31-37: Add ARIA landmarks or labels for the connectivity section.
Your sticky section groups protocol buttons, connect controls, and recording controls, but it has no role or aria-label.
Wrap it in <section role="region" aria-label="Device connectivity and recording"> to improve screen-reader navigation.


34-45: Protocol buttons lack keyboard focus styles.
When navigating via keyboard, it’s hard to see which protocol is selected.
Consider adding focus:outline-none focus:ring-2 focus:ring-cyan-400 to the .connection-btn classes for better accessibility.


47-55: Explicitly set type="button" on non-form buttons.
By default, <button> in some contexts behaves as a submit button. Although you aren’t inside a <form>, it’s a good practice to declare type="button" on #connect-btn, #disconnect-btn, etc.


63-69: Missing <label> for the filename input.
Your text input for recording name has no <label> element. Without it, users relying on assistive tech won’t know what the field is for.
Consider:

<label for="filename" class="sr-only">Recording filename</label>
<input id="filename" >

83-89: BLE modal needs proper dialog semantics.
You should add role="dialog", aria-modal="true", and aria-labelledby="ble-modal-title" to the modal container for accessibility. Also ensure the close button has aria-label="Close Bluetooth device selector".


112-119: Filter <select> should have an id and <label>.
You rely on document.querySelector('select') in JS, which may break if more selects are added later.

  • Give the <select> an id="category-filter".
  • Add a <label for="category-filter">Filter by category</label> for accessibility.

146-922: Extract inline script into modular JS files.
The <script> block now spans hundreds of lines, mixing concerns (app loading, theming, connectivity, recording, SSE, UI updates).

  • Split into separate modules: theme.js, connectivity.js, apps.js, console.js.
  • Serve them as external scripts for better caching, maintainability, and testability.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a843e5 and 899efd1.

📒 Files selected for processing (1)
  • templates/index.html (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
templates/index.html (2)

2-2: Remove hard-coded dark class to avoid FOUC

Leaving class="dark" on the root element forces dark mode on first paint and then toggles to the user’s preferred theme, causing a visual flicker. Instead, add the class only after you’ve read localStorage/prefers-color-scheme in JS (the logic is already in script.js).


133-146: Place Tailwind config before the CDN import

Per Tailwind CDN docs, the <script>tailwind.config…</script> block must precede
<script src="https://cdn.tailwindcss.com"> or the custom settings (dark mode, cursor) won’t be applied on first render.

static/script.js (3)

78-83: Dynamic colour classes may be purged / unavailable

Classes like bg-${app.color}-100 and text-${app.color}-600 are generated at runtime.
Tailwind’s CDN compiler may not catch them, resulting in missing styles.
Prefer a lookup table:

const colourMap = {
  red:  ['bg-red-100','dark:bg-red-900','text-red-600','dark:text-red-300'],
  cyan: ['bg-cyan-100', ...],
  // …
};
const [bg, bgDark, fg, fgDark] = colourMap[app.color] || colourMap.cyan;
card.innerHTML = `
  <div class="${bg} ${bgDark} …">
     <i class="… ${fg} ${fgDark}"></i>
  </div>
`;

617-633: Console SSE only logs to dev console — no UI feedback

Users cannot see live device logs unless they open DevTools.
Append messages to a scrollable <div id="console-container"> (as previously suggested) and implement reconnection/back-off logic so the stream resumes after network hiccups.


426-452: 🛠️ Refactor suggestion

No CSRF protection on POST requests

All fetch POSTs (/connect, /start_recording, etc.) omit CSRF tokens.
If your Flask app has (or will have) CSRF protection, these requests will fail and remain vulnerable to cross-site attacks.

🧹 Nitpick comments (1)
templates/index.html (1)

60-68: Validate & constrain filename on the client

<input id="filename"> currently allows any characters, but the sanitiser in JS only replaces spaces and dots.
Add an explicit pattern (or use type="text" + pattern) to block path separators and non-ASCII characters to prevent invalid filenames reaching the backend.

-<input type="text" id="filename"
-    class="flex-1 h-10 px-4 rounded-l-lg focus:outline-none focus:ring-1  border border-gray-600  dark:text-white dark:bg-gray-800" 
-    placeholder="Enter recording name">
+<input type="text" id="filename"
+    pattern="^[A-Za-z0-9_-]+$"
+    title="Use letters, numbers, dashes or underscores only"
+    class="flex-1 h-10 px-4 rounded-l-lg focus:outline-none focus:ring-1 border border-gray-600 dark:text-white dark:bg-gray-800"
+    placeholder="Enter recording name">
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 899efd1 and 96f4a2e.

📒 Files selected for processing (2)
  • static/script.js (1 hunks)
  • templates/index.html (1 hunks)
🔇 Additional comments (1)
static/script.js (1)

320-358: isScanning flag never reset when modal closes

If the user closes the BLE modal mid-scan, isScanning stays true, blocking future rescans.
Reset the flag in the modal’s close handler.

closeBleModal.addEventListener('click', () => {
   bleModal.classList.add('hidden');
+  isScanning = false;
   …
});

app_processes[app_name] = process # Track running process in global dictionary
current_message = f"{app_name} started successfully"
if connection_manager.start_csv_recording(filename):
post_console_message(f"Recording started: {filename or 'default filename'}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PayalLakra We should add a way to push the toast under our web UI.

time.sleep(0.5)

return Response(event_stream(), mimetype="text/event-stream")
logging.error(f"Recording error: {str(e)}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be log.txt created with timestamps for error logging.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
static/script.js (3)

78-80: Dynamic Tailwind classes will be purged in production builds

bg-${app.color}-100, dark:bg-${app.color}-900, text-${app.color}-600 etc. are generated at runtime.
Unless you safelist every possible permutation in tailwind.config.js, PurgeCSS/JIT will strip them, leaving blank styles in production.

Safelist with a regex pattern or switch to a small predefined map:

const COLOR_CLASSES = {
  red   : ['bg-red-100','dark:bg-red-900','text-red-600','dark:text-red-300'],
  cyan  : ['bg-cyan-100','dark:bg-cyan-900','text-cyan-600','dark:text-cyan-300'],};
const [bgLight,bgDark,txtLight,txtDark] = COLOR_CLASSES[app.color] ?? COLOR_CLASSES.cyan;

761-763: Continuous 1 s polling may overload the backend & client

setInterval(checkStreamStatus, 1000) fires forever, even when the page is idle and an SSE channel already exists. Consider:

  1. Switching to a single SSE/WebSocket event that emits connection changes, or
  2. Increasing the interval / using exponential back-off once a stable state is detected, stopping entirely while the SSE stream is alive.

This will cut network chatter and CPU usage.


708-710: Replace blocking alert() with non-blocking toast for better UX

Native alert() is synchronous, blocks the main thread, and cannot be styled.
Adopt a lightweight toast/snackbar component (e.g. Tailwind + Alpine, or a small JS lib) to keep the UI responsive and visually consistent.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96f4a2e and 46a5185.

📒 Files selected for processing (1)
  • static/script.js (1 hunks)

Comment on lines +646 to +656
// Get the filename (already sanitized in the display)
let filename = filenameInput.value.trim();

// If empty, use default (pass null to let server generate default)
if (filename === '') {
filename = null;
}

if (isRecording) {
// Stop recording
fetch('/stop_recording', { method: 'POST' })
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Re-sanitise filename just before the network request

The input event filter is good for UX, but a malicious user can still alter the DOM value via dev-tools or crafted script injection.
Run sanitizeFilename again right before you send it to the server to guarantee server-side safety.

-// Get the filename (already sanitized in the display)
-let filename = filenameInput.value.trim();
+// Re-sanitise to defend against DOM tampering
+let filename = sanitizeFilename(filenameInput.value);
📝 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.

Suggested change
// Get the filename (already sanitized in the display)
let filename = filenameInput.value.trim();
// If empty, use default (pass null to let server generate default)
if (filename === '') {
filename = null;
}
if (isRecording) {
// Stop recording
fetch('/stop_recording', { method: 'POST' })
// Re-sanitise to defend against DOM tampering
let filename = sanitizeFilename(filenameInput.value);
// If empty, use default (pass null to let server generate default)
if (filename === '') {
filename = null;
}
if (isRecording) {
// Stop recording
fetch('/stop_recording', { method: 'POST' })

Comment on lines +74 to +87
card.className = `group bg-gradient-to-b from-white to-gray-50 dark:from-gray-700 dark:to-gray-800 rounded-xl shadow border hover:shadow-lg transition-all duration-300 dark:border-gray-700 cursor-pointer overflow-hidden`;

card.innerHTML = `
<div class="relative h-full flex flex-col">
<div class="w-full h-32 bg-${app.color}-100 dark:bg-${app.color}-900 rounded-t-lg flex items-center justify-center transition-all duration-300 group-hover:opacity-90">
<i class="fas ${app.icon} text-4xl text-${app.color}-600 dark:text-${app.color}-300"></i>
</div>
<div class="p-4 flex-1 flex flex-col">
<h3 class="text-lg font-semibold text-gray-800 dark:text-gray-200 mb-1">${app.title}</h3>
<p class="text-sm text-gray-600 dark:text-gray-400 mb-3 flex-1">${app.description}</p>
</div>
</div>
`;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid innerHTML with unsanitised data – potential XSS vector

app.title, app.description, app.icon, app.color, and app.script originate from a YAML file that is editable outside the UI. Injecting them directly with innerHTML lets any embedded markup (or worse, <script> tags) execute in the browser.

Refactor to build the DOM with textContent / classList or sanitise the strings with a vetted library such as DOMPurify before insertion.

-card.innerHTML = `
-  <div class="relative h-full flex flex-col">
-    <div class="w-full h-32 bg-${app.color}-100 dark:bg-${app.color}-900 …">
-      <i class="fas ${app.icon} text-4xl …"></i>
-    </div>
-    <div class="p-4 flex-1 flex flex-col">
-      <h3 class="text-lg … mb-1">${app.title}</h3>
-      <p class="text-sm … mb-3 flex-1">${app.description}</p>
-    </div>
-  </div>
-`;
+const wrapper   = document.createElement('div');
+wrapper.className = 'relative h-full flex flex-col';
+
+const banner = document.createElement('div');
+banner.className = `w-full h-32 bg-${app.color}-100 dark:bg-${app.color}-900 …`;
+
+const icon = document.createElement('i');
+icon.className = `fas ${app.icon} text-4xl …`;
+banner.appendChild(icon);
+
+const body = document.createElement('div');
+body.className = 'p-4 flex-1 flex flex-col';
+
+const title = document.createElement('h3');
+title.className = 'text-lg … mb-1';
+title.textContent = app.title;
+
+const desc = document.createElement('p');
+desc.className = 'text-sm … mb-3 flex-1';
+desc.textContent = app.description;
+
+body.append(title, desc);
+wrapper.append(banner, body);
+card.appendChild(wrapper);

Committable suggestion skipped: line range outside the PR's diff.

@lorforlinux
Copy link
Member

@PayalLakra Thank you so much for your contribution, I am merging this now.

@lorforlinux lorforlinux merged commit c71ffe0 into upsidedownlabs:main May 9, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants