Python Package, Code Refactoring and Documentation #46
Conversation
WalkthroughThis update restructures the project by modularizing code into the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI (Flask)
participant ConnectionManager
participant Device (USB/WiFi/BLE)
participant LSL/CSV
User->>WebUI (Flask): Connect/Scan/Record/Launch App
WebUI (Flask)->>ConnectionManager: connect_device(protocol, params)
ConnectionManager->>Device: Establish connection (USB/WiFi/BLE)
Device-->>ConnectionManager: Data packets
ConnectionManager->>LSL/CSV: Stream data, optionally record
User->>WebUI (Flask): Stop/Disconnect/Stop Recording
WebUI (Flask)->>ConnectionManager: cleanup()
ConnectionManager->>Device: Close connection
ConnectionManager->>LSL/CSV: Stop streams/recording
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (20)
chordspy/gui.py (1)
125-131: PEP-8 spacing & tightermain()interfaceFlake8 rightly flags the missing blank lines.
Also,plot_lsl_data()returns theappobject but the return value is ignored; relying on a global adds coupling.@@ + + def main(): app = plot_lsl_data() # capture the returned QApplication - if inlet: - sys.exit(app.exec_()) + if inlet: + sys.exit(app.exec_()) if __name__ == "__main__": main()Two blank lines before
main()and before the guard satisfy E302/E305 and the explicitappvariable reduces hidden globals.chordspy/csvplotter.py (1)
102-108: Minor PEP-8 spacing deviationInsert two blank lines before
main()and before theif __name__ …guard to silence E302/E305.@@ + + def main(): root = tk.Tk() # Create the main Tkinter root window CSVPlotterApp(root) # Create an instance of the CSVPlotterApp class root.mainloop() # Start the Tkinter main loop + if __name__ == "__main__": main()chordspy/eog.py (1)
186-194: Consistent spacing around newmain()Same PEP-8 blank-line issue as elsewhere; quick fix:
@@ + + def main(): app = QApplication(sys.argv) window = EOGMonitor() print("Note: There will be a 2s calibration delay before peak detection starts.") window.show() sys.exit(app.exec_()) + if __name__ == "__main__": main()chordspy/ffteeg.py (1)
421-428: Good refactoring for modularity, but fix PEP8 formatting issues.The extraction of main execution logic into a dedicated
main()function improves code modularity and follows the consistent pattern applied across the project. However, the static analysis correctly identifies PEP8 formatting violations.Apply this diff to fix the formatting issues:
- + def main(): app = QApplication(sys.argv) window = EEGMonitor() window.show() sys.exit(app.exec_()) + if __name__ == "__main__": main()chordspy/emgenvelope.py (1)
139-146: Consistent refactoring pattern, but address formatting issues.The main function extraction follows the same beneficial pattern as other GUI modules in the project, improving modularity and consistency.
Apply this diff to fix PEP8 formatting:
self.close() - + def main(): app = QApplication(sys.argv) window = EMGMonitor() window.show() sys.exit(app.exec_()) + if __name__ == "__main__": main()chordspy/heartbeat_ecg.py (1)
163-170: Maintain consistency with other modules, fix formatting.The refactoring maintains consistency with the other GUI modules and improves code organization.
Apply this diff to address PEP8 formatting:
self.r_peak_curve.setData(r_peak_times, r_peak_values) # Plot R-peaks as red dots + def main(): app = QApplication(sys.argv) window = ECGMonitor() window.show() sys.exit(app.exec_()) + if __name__ == "__main__": main()chordspy/__init__.py (1)
1-2: Add__all__to clarify intended public APIThe static analysis warnings about unused imports are false positives - these imports are intended to expose the key components (
mainandConnection) at the package level for external use. Consider adding an__all__declaration to make the intended public API explicit:+__all__ = ['main', 'Connection'] + from chordspy.app import main from chordspy.connection import ConnectionThis will clarify that these imports are intentional for the package's public interface and may resolve the static analysis warnings.
chordspy/static/script.js (1)
951-968: Consider consolidating initialization callsThe DOMContentLoaded handler has several initialization calls that could be organized better:
document.addEventListener('DOMContentLoaded', () => { - initializeApplication(); - checkStreamStatus(); - setInterval(checkStreamStatus, 1000); - startTimestampUpdater(); + // Initialize core application functionality + initializeApplication(); + startTimestampUpdater(); + + // Set up periodic status checking + checkStreamStatus(); + setInterval(checkStreamStatus, 1000); // Error handling window.onerror = function(message, source, lineno, colno, error) {This groups related functionality and removes the duplicate
checkStreamStatus()call (sinceinitializeApplication()likely calls it viastartAppStatusChecker()).README.md (1)
3-3: Fix grammatical issues for better clarityAddress the following grammatical improvements flagged by LanguageTool:
-Chords- Python is an open-source bag of tools designed to interface with Micro-controller development boards running [Chords Arduino Firmware](https://github.com/upsidedownlabs/Chords-Arduino-Firmware). Use Upside Down Labs bio-potential amplifiers to read data, visualize it, record data in CSV Files, and stream it via Lab Streaming Layer. +Chords-Python is an open-source bag of tools designed to interface with microcontroller development boards running [Chords Arduino Firmware](https://github.com/upsidedownlabs/Chords-Arduino-Firmware). Use Upside-Down Labs' bio-potential amplifiers to read data, visualize it, record data in CSV files, and stream it via the Lab Streaming Layer.Changes made:
- Fixed spacing in "Chords-Python"
- Added possessive apostrophe for "Upside-Down Labs'"
- Added hyphen to "Upside-Down"
- Added "the" before "Lab Streaming Layer"
- Capitalization consistency for "CSV files"
.github/workflows/release.yaml (1)
133-133: Add newline at end of file.YAML files should end with a newline character for proper formatting.
Add a newline character after the last line.
chordspy/chords_serial.py (3)
108-108: Remove unused variable assignment.The
sampling_ratevariable is assigned but never used.- sampling_rate = self.supported_boards[self.board]["sampling_rate"]If this value is needed for future use, consider storing it as an instance attribute or using it in logging.
16-17: Add blank line before class definition.PEP 8 requires two blank lines before class definitions.
import threading + class Chords_USB:
276-277: Add blank line after class definition.PEP 8 requires two blank lines after class definitions.
last_ten_minute_time = time.time() + if __name__ == "__main__":chordspy/app.py (2)
59-60: Remove unused exception variable.The exception variable
eis not used in the except block.- except Exception as e: + except Exception: return jsonify({'status': 'error', 'message': 'Logging failed'}), 500
200-200: Add missing whitespace after comma.- return jsonify({'status': 'error', 'message': f'{module_name} is already running','code': 'ALREADY_RUNNING'}), 400 + return jsonify({'status': 'error', 'message': f'{module_name} is already running', 'code': 'ALREADY_RUNNING'}), 400chordspy/chords_ble.py (2)
107-111: Remove or use the extracted channel data.The
channelsvariable is extracted but never used. If this is for debugging, consider adding a comment or removing it.- # Extract channel data (2 bytes per channel, big-endian, signed) - channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True) - for i in range(1, len(sample_data), 2)] + # Extract channel data (2 bytes per channel, big-endian, signed) + # channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True) + # for i in range(1, len(sample_data), 2)] + # TODO: Process channel data when needed
28-28: Add spaces before inline comment.- CONTROL_CHAR_UUID = "0000ff01-0000-1000-8000-00805f9b34fb" # UUID for control characteristic + CONTROL_CHAR_UUID = "0000ff01-0000-1000-8000-00805f9b34fb" # UUID for control characteristicchordspy/connection.py (3)
2-2: Fix typo in documentation header.Remove the trailing dash in the documentation header.
-CHORDS Data Connection- +CHORDS Data Connection
628-628: Add blank lines for PEP 8 compliance.Add two blank lines before the function definition.
self.cleanup() + def main():
680-681: Add blank line for module-level code separation.Add a blank line after the function definition.
if __name__ == '__main__': main() +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
chordspy/media/Beetle1.pngis excluded by!**/*.pngchordspy/media/Beetle10.pngis excluded by!**/*.pngchordspy/media/Beetle2.pngis excluded by!**/*.pngchordspy/media/Beetle3.pngis excluded by!**/*.pngchordspy/media/Beetle4.pngis excluded by!**/*.pngchordspy/media/Beetle5.pngis excluded by!**/*.pngchordspy/media/Beetle6.pngis excluded by!**/*.pngchordspy/media/Beetle7.pngis excluded by!**/*.pngchordspy/media/Beetle8.pngis excluded by!**/*.pngchordspy/media/Beetle9.pngis excluded by!**/*.pngchordspy/media/Interface.pngis excluded by!**/*.pngchordspy/media/Webinterface.pngis excluded by!**/*.pngchordspy/media/brass-fanfare-with-timpani-and-winchimes-reverberated-146260.wavis excluded by!**/*.wavchordspy/media/icons8-eye-30.pngis excluded by!**/*.png
📒 Files selected for processing (27)
.github/workflows/release.yaml(1 hunks)MANIFEST.in(1 hunks)README.md(1 hunks)app.py(0 hunks)app_requirements.txt(0 hunks)chords_requirements.txt(0 hunks)chords_serial.py(0 hunks)chordspy/__init__.py(1 hunks)chordspy/app.py(1 hunks)chordspy/beetle.py(1 hunks)chordspy/chords_ble.py(10 hunks)chordspy/chords_serial.py(1 hunks)chordspy/chords_wifi.py(6 hunks)chordspy/connection.py(1 hunks)chordspy/csvplotter.py(1 hunks)chordspy/emgenvelope.py(1 hunks)chordspy/eog.py(1 hunks)chordspy/ffteeg.py(1 hunks)chordspy/game.py(1 hunks)chordspy/gui.py(1 hunks)chordspy/heartbeat_ecg.py(1 hunks)chordspy/keystroke.py(1 hunks)chordspy/static/script.js(3 hunks)chordspy/templates/index.html(5 hunks)connection.py(0 hunks)pyproject.toml(1 hunks)requirements.txt(0 hunks)
💤 Files with no reviewable changes (6)
- requirements.txt
- chords_requirements.txt
- app_requirements.txt
- chords_serial.py
- app.py
- connection.py
🧰 Additional context used
🧬 Code Graph Analysis (6)
chordspy/__init__.py (2)
chordspy/app.py (1)
main(357-366)chordspy/connection.py (2)
main(628-678)Connection(36-626)
chordspy/emgenvelope.py (3)
chordspy/eog.py (1)
main(186-191)chordspy/heartbeat_ecg.py (1)
main(163-167)chordspy/ffteeg.py (1)
main(421-425)
chordspy/chords_serial.py (5)
chordspy/static/script.js (10)
data(177-177)data(194-194)data(573-573)data(692-692)response(34-34)response(129-133)response(171-171)response(191-191)response(563-567)response(691-691)chordspy/test/new_parser.py (7)
connect_hardware(58-122)detect_hardware(125-133)send_command(50-56)cleanup(275-310)start_timer(192-198)log_one_second_data(201-206)log_ten_minute_data(209-219)chordspy/chords_ble.py (1)
cleanup(204-213)chordspy/chords_wifi.py (1)
cleanup(181-193)chordspy/connection.py (1)
cleanup(558-620)
chordspy/connection.py (3)
chordspy/chords_serial.py (5)
Chords_USB(17-275)read_data(161-200)detect_hardware(121-141)send_command(143-159)cleanup(222-231)chordspy/chords_wifi.py (3)
Chords_WIFI(13-199)connect(61-75)cleanup(181-193)chordspy/chords_ble.py (6)
Chords_BLE(13-238)scan_devices(55-69)notification_handler(113-129)connect(215-231)cleanup(204-213)stop(233-238)
chordspy/app.py (5)
chordspy/connection.py (8)
Connection(36-626)cleanup(558-620)connect_usb(524-556)connect_wifi(460-522)connect_ble(399-458)start_csv_recording(140-171)stop_csv_recording(173-195)main(628-678)chordspy/static/script.js (6)
config(40-40)data(177-177)data(194-194)data(573-573)data(692-692)filename(776-776)chordspy/chords_ble.py (1)
cleanup(204-213)chordspy/chords_wifi.py (1)
cleanup(181-193)chordspy/chords_serial.py (1)
cleanup(222-231)
chordspy/chords_wifi.py (3)
chordspy/chords_ble.py (1)
cleanup(204-213)chordspy/chords_serial.py (1)
cleanup(222-231)chordspy/connection.py (1)
cleanup(558-620)
🪛 Flake8 (7.2.0)
chordspy/__init__.py
[error] 1-1: 'chordspy.app.main' imported but unused
(F401)
[error] 2-2: 'chordspy.connection.Connection' imported but unused
(F401)
chordspy/eog.py
[error] 186-186: expected 2 blank lines, found 1
(E302)
[error] 193-193: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/heartbeat_ecg.py
[error] 163-163: expected 2 blank lines, found 1
(E302)
[error] 169-169: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/csvplotter.py
[error] 102-102: expected 2 blank lines, found 1
(E302)
[error] 107-107: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/gui.py
[error] 125-125: expected 2 blank lines, found 1
(E302)
[error] 130-130: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/emgenvelope.py
[error] 139-139: expected 2 blank lines, found 1
(E302)
[error] 145-145: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/ffteeg.py
[error] 421-421: expected 2 blank lines, found 1
(E302)
[error] 427-427: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/chords_serial.py
[error] 17-17: expected 2 blank lines, found 1
(E302)
[error] 70-70: at least two spaces before inline comment
(E261)
[error] 108-108: local variable 'sampling_rate' is assigned to but never used
(F841)
[error] 256-256: global missing_samples is unused: name is never assigned in scope
(F824)
[error] 265-265: global supported_boards is unused: name is never assigned in scope
(F824)
[error] 265-265: global board is unused: name is never assigned in scope
(F824)
[error] 277-277: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/chords_ble.py
[error] 28-28: at least two spaces before inline comment
(E261)
[error] 108-108: local variable 'channels' is assigned to but never used
(F841)
[error] 109-109: continuation line under-indented for visual indent
(E128)
[error] 240-240: expected 2 blank lines, found 1
(E302)
chordspy/connection.py
[error] 36-36: expected 2 blank lines, found 1
(E302)
[error] 264-264: local variable 'estimated_rate' is assigned to but never used
(F841)
[error] 264-264: at least two spaces before inline comment
(E261)
[error] 428-428: continuation line under-indented for visual indent
(E128)
[error] 628-628: expected 2 blank lines, found 1
(E302)
[error] 680-680: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/app.py
[error] 25-25: redefinition of unused 'logging' from line 17
(F811)
[error] 41-41: expected 2 blank lines, found 1
(E302)
[error] 59-59: local variable 'e' is assigned to but never used
(F841)
[error] 63-63: expected 2 blank lines, found 1
(E302)
[error] 81-81: expected 2 blank lines, found 1
(E302)
[error] 87-87: expected 2 blank lines, found 1
(E302)
[error] 95-95: at least two spaces before inline comment
(E261)
[error] 110-110: expected 2 blank lines, found 1
(E302)
[error] 122-122: continuation line under-indented for visual indent
(E128)
[error] 129-129: expected 2 blank lines, found 1
(E302)
[error] 140-140: expected 2 blank lines, found 1
(E302)
[error] 152-152: expected 2 blank lines, found 1
(E302)
[error] 166-166: expected 2 blank lines, found 1
(E302)
[error] 182-182: expected 2 blank lines, found 1
(E302)
[error] 200-200: missing whitespace after ','
(E231)
[error] 216-216: expected 2 blank lines, found 1
(E302)
[error] 234-234: expected 2 blank lines, found 1
(E302)
[error] 293-293: expected 2 blank lines, found 1
(E302)
[error] 300-300: global connection_manager is unused: name is never assigned in scope
(F824)
[error] 309-309: expected 2 blank lines, found 1
(E302)
[error] 316-316: global connection_manager is unused: name is never assigned in scope
(F824)
[error] 337-337: expected 2 blank lines, found 1
(E302)
[error] 344-344: global connection_manager is unused: name is never assigned in scope
(F824)
[error] 357-357: expected 2 blank lines, found 1
(E302)
[error] 368-368: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/chords_wifi.py
[error] 11-11: 'scipy.signal.butter' imported but unused
(F401)
[error] 11-11: 'scipy.signal.filtfilt' imported but unused
(F401)
[error] 13-13: expected 2 blank lines, found 1
(E302)
[error] 126-126: local variable 'sps' is assigned to but never used
(F841)
[error] 127-127: local variable 'fps' is assigned to but never used
(F841)
[error] 128-128: local variable 'bps' is assigned to but never used
(F841)
[error] 201-201: expected 2 blank lines after class or function definition, found 1
(E305)
🪛 Ruff (0.11.9)
chordspy/__init__.py
1-1: chordspy.app.main imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
2-2: chordspy.connection.Connection imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
chordspy/chords_serial.py
108-108: Local variable sampling_rate is assigned to but never used
Remove assignment to unused variable sampling_rate
(F841)
chordspy/chords_ble.py
108-108: Local variable channels is assigned to but never used
Remove assignment to unused variable channels
(F841)
chordspy/connection.py
161-161: Use a context manager for opening files
(SIM115)
264-264: Local variable estimated_rate is assigned to but never used
Remove assignment to unused variable estimated_rate
(F841)
668-669: Use a single if statement instead of nested if statements
(SIM102)
chordspy/app.py
25-25: Redefinition of unused logging from line 17
Remove definition: logging
(F811)
59-59: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
chordspy/chords_wifi.py
11-11: scipy.signal.butter imported but unused
Remove unused import
(F401)
11-11: scipy.signal.filtfilt imported but unused
Remove unused import
(F401)
126-126: Local variable sps is assigned to but never used
Remove assignment to unused variable sps
(F841)
127-127: Local variable fps is assigned to but never used
Remove assignment to unused variable fps
(F841)
128-128: Local variable bps is assigned to but never used
Remove assignment to unused variable bps
(F841)
🪛 LanguageTool
README.md
[uncategorized] ~3-~3: The adjective “Upside-Down” is spelled with a hyphen.
Context: ...edownlabs/Chords-Arduino-Firmware). Use Upside Down Labs bio-potential amplifiers to read d...
(UPSIDE_DOWN)
[uncategorized] ~3-~3: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ords-Arduino-Firmware). Use Upside Down Labs bio-potential amplifiers to read data, ...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ...rd data in CSV Files, and stream it via Lab Streaming Layer. > [!NOTE] > **Fir...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 actionlint (1.7.7)
.github/workflows/release.yaml
128-128: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
130-130: property "details" is not defined in object type {pypi_publish: {outputs: {}; result: string}; setup_and_build: {outputs: {}; result: string}}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/release.yaml
[error] 133-133: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Pylint (3.3.7)
chordspy/chords_serial.py
[refactor] 17-17: Too many instance attributes (8/7)
(R0902)
[error] 258-258: Undefined variable 'missing_samples'
(E0602)
[error] 269-269: Undefined variable 'supported_boards'
(E0602)
[error] 269-269: Undefined variable 'board'
(E0602)
chordspy/connection.py
[refactor] 36-36: Too many instance attributes (27/7)
(R0902)
[refactor] 161-161: Consider using 'with' for resource-allocating operations
(R1732)
[error] 290-290: Instance of 'Chords_BLE' has no 'data_available' member
(E1101)
[error] 295-295: Instance of 'Chords_BLE' has no 'get_latest_sample' member
(E1101)
[refactor] 287-319: Too many nested blocks (6/5)
(R1702)
[error] 331-331: Instance of 'Chords_WIFI' has no 'data_available' member
(E1101)
[error] 336-336: Instance of 'Chords_WIFI' has no 'get_latest_sample' member
(E1101)
[refactor] 328-357: Too many nested blocks (6/5)
(R1702)
[refactor] 366-397: Too many nested blocks (6/5)
(R1702)
[refactor] 558-558: Too many branches (15/12)
(R0912)
chordspy/app.py
[refactor] 207-207: Consider using 'with' for resource-allocating operations
(R1732)
[refactor] 226-230: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[error] 267-267: Assigning result of a function call, where the function has no return
(E1111)
[error] 278-278: Possibly using variable 'success' before assignment
(E0606)
chordspy/chords_wifi.py
[refactor] 13-13: Too many instance attributes (14/7)
(R0902)
[refactor] 34-34: Too many arguments (6/5)
(R0913)
[refactor] 34-34: Too many positional arguments (6/5)
(R0917)
[refactor] 88-88: Too many branches (15/12)
(R0912)
[refactor] 88-88: Too many statements (58/50)
(R0915)
🪛 ast-grep (0.38.1)
chordspy/app.py
[warning] 365-365: 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, use_reloader=False, host='0.0.0.0', port=5000)
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)
🔇 Additional comments (12)
pyproject.toml (2)
32-33: Console script entry point is well-defined.The console script configuration properly maps the
chordspycommand to the main application entry point.
12-31: Verify dependency security using pip-audit with correct invocationThe Poetry migration is solid—next, ensure all fixed versions are free of known vulnerabilities. Since
pip-auditmay install to~/.local/bin(outside your PATH), invoke it like this:# Create a temporary requirements file cat > temp_requirements.txt << EOF numpy==2.1.3 pylsl==1.16.2 pyserial==3.5 bleak==0.22.3 pyqtgraph==0.13.7 PyQt5==5.15.11 keyboard==0.13.5 scipy==1.14.1 pygame==2.6.1 neurokit2==0.2.10 plotly==5.24.1 pandas==2.2.3 tk==0.1.0 PyAutoGUI==0.9.54 Flask==3.1.1 psutil==6.1.1 websocket-client==1.8.0 PyYAML==6.0.2 EOF # Install pip-audit if needed pip install --user pip-audit # Run vulnerability scan via module to avoid PATH issues python3 -m pip_audit --requirement temp_requirements.txt # Cleanup rm temp_requirements.txt• Ensure the scan completes without errors.
• Review any reported CVEs and update versions as necessary.
• Once all vulnerabilities are addressed, merge the Poetry changes.MANIFEST.in (1)
1-5: Well-structured manifest for Flask web applicationThe MANIFEST.in file correctly includes all necessary resources for the Flask web interface:
- Essential documentation files (
requirements.txt,README.md)- Web application assets (
templates,static)- Configuration files (
config)This ensures the package will work correctly when installed via pip.
chordspy/static/script.js (2)
115-118: Clean implementation of launch button functionalityThe inline
onclickapproach works well for the dynamic button generation. ThelaunchAppfunction properly handles different response scenarios including already running apps.
127-149: Robust error handling in launchApp functionGood implementation with:
- Proper async/await usage
- Specific handling of ALREADY_RUNNING status
- Comprehensive error catching and logging
- Clear user feedback through alerts
README.md (2)
16-32: Excellent simplification of installation processThe new installation instructions are much clearer and more user-friendly compared to the previous multi-step process. The virtual environment setup with platform-specific activation commands is particularly helpful.
35-38: Updated usage reflects new package structureThe change from
python app.pytochordspycommand correctly reflects the new console script entry point defined in the package configuration.chordspy/templates/index.html (2)
3-5: Excellent documentation enhancementThe added HTML comments throughout the template significantly improve code maintainability by clearly explaining:
- The purpose of each major section
- UI component functionality
- Dark mode configuration
- Interactive element behavior
This documentation will be valuable for future developers working on the frontend.
177-181: Clear Tailwind configuration documentationThe comment explaining the class-based dark mode configuration is helpful for understanding the styling approach used throughout the application.
chordspy/chords_ble.py (1)
1-52: Excellent documentation improvements!The added docstrings and comments significantly improve code readability and maintainability.
chordspy/chords_wifi.py (1)
1-43: Well-documented class and methods!The added docstrings provide excellent clarity on the WiFi client's functionality and usage.
chordspy/connection.py (1)
161-161: Current file handling approach is appropriate.The file needs to remain open across multiple method calls for continuous recording, so a context manager is not suitable here. The existing error handling and cleanup in
stop_csv_recording()ensures proper resource management.
| def ble_data_handler(self): | ||
| """ | ||
| BLE-specific data handler with precise timing control. | ||
| The handler ensures: | ||
| 1. Precise sample timing using local_clock() | ||
| 2. Constant sampling rate regardless of BLE packet timing | ||
| 3. Graceful handling of buffer overflows | ||
| 4. Thread-safe operation with the main controller | ||
| """ | ||
| # Target specifications for the BLE stream | ||
| SAMPLE_INTERVAL = 1.0 / self.sampling_rate # Time between samples in seconds | ||
| next_sample_time = local_clock() # Initialize timing baseline | ||
|
|
||
| # Main processing loop - runs while system is active and BLE connected | ||
| while self.running and self.ble_connection: | ||
| try: | ||
| # Check if new BLE data is available | ||
| if hasattr(self.ble_connection, 'data_available') and self.ble_connection.data_available: | ||
| current_time = local_clock() # Get precise current timestamp | ||
|
|
||
| # Only process if we've reached the next scheduled sample time | ||
| if current_time >= next_sample_time: | ||
| sample = self.ble_connection.get_latest_sample() | ||
| if sample: | ||
| channel_data = sample[:self.num_channels] # Extract channel data | ||
|
|
||
| # Calculate precise timestamp | ||
| sample_time = next_sample_time | ||
| next_sample_time += SAMPLE_INTERVAL # Schedule next sample time | ||
|
|
||
| # If we're falling behind, skip samples to catch up | ||
| if current_time > next_sample_time + SAMPLE_INTERVAL: | ||
| next_sample_time = current_time + SAMPLE_INTERVAL | ||
|
|
||
| # Stream to LSL if enabled | ||
| if self.lsl_connection: | ||
| self.lsl_connection.push_sample(channel_data, timestamp=sample_time) | ||
|
|
||
| # Update rate display | ||
| self.update_sample_rate() | ||
|
|
||
| # Log to CSV if recording | ||
| if self.recording_active: | ||
| self.log_to_csv(channel_data) | ||
| except Exception as e: | ||
| print(f"BLE data handler error: {str(e)}") | ||
| break | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix references to non-existent BLE methods.
The method references self.ble_connection.data_available and self.ble_connection.get_latest_sample() which don't exist in the Chords_BLE class according to the provided code snippets. This will cause runtime errors.
Based on the code snippets, the Chords_BLE class processes data in its notification_handler and stores samples internally, but doesn't expose a data_available flag or get_latest_sample() method. The BLE data should be handled differently, possibly by accessing the received samples directly from the BLE handler or implementing the missing methods in Chords_BLE.
Run the following script to verify the available methods in Chords_BLE:
🏁 Script executed:
#!/bin/bash
# Description: Check for data_available and get_latest_sample methods in Chords_BLE class
# Search for the methods in the BLE class
echo "Searching for data_available in Chords_BLE:"
ast-grep --pattern 'class Chords_BLE {
$$$
data_available
$$$
}'
echo -e "\nSearching for get_latest_sample method:"
ast-grep --pattern 'def get_latest_sample($$$):
$$$'
echo -e "\nChecking the actual Chords_BLE class structure:"
rg -A 20 "class Chords_BLE" chordspy/chords_ble.pyLength of output: 1395
Resolve missing BLE data methods in Chords_BLE
The ble_data_handler in chordspy/connection.py (lines 273–320) calls
self.ble_connection.data_availableself.ble_connection.get_latest_sample()
but neither method exists in the Chords_BLE class (chordspy/chords_ble.py), leading to runtime AttributeError.
• File/Location to update:
- chordspy/connection.py (ble_data_handler)
- chordspy/chords_ble.py
• Recommended fixes:
- Implement in
Chords_BLE:- A
@property data_available(e.g. checks internal queue) - A
get_latest_sample()method (e.g. dequeues the next sample)
- A
- Or adjust
ble_data_handlerto use the existing notification/callback API inChords_BLE(e.g. consume its internal buffer or invoke itsnotification_handler).
Ensure the BLE class and handler agree on how samples are buffered and retrieved.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 290-290: Instance of 'Chords_BLE' has no 'data_available' member
(E1101)
[error] 295-295: Instance of 'Chords_BLE' has no 'get_latest_sample' member
(E1101)
[refactor] 287-319: Too many nested blocks (6/5)
(R1702)
🤖 Prompt for AI Agents
In chordspy/chords_ble.py, add a @property method named data_available that
returns True if there are samples in the internal buffer or queue, and implement
a get_latest_sample() method that retrieves and removes the next sample from
this buffer. This will align the Chords_BLE class with the usage in
chordspy/connection.py's ble_data_handler (lines 273-320), which expects these
methods to check for available data and fetch the latest sample. Alternatively,
modify ble_data_handler to use existing Chords_BLE notification or callback
mechanisms to access samples, ensuring both sides use a consistent approach to
sample buffering and retrieval.
| def wifi_data_handler(self): | ||
| """ | ||
| WiFi-specific data handler with network-optimized timing. | ||
| """ | ||
| SAMPLE_INTERVAL = 1.0 / self.sampling_rate # Time between samples in seconds | ||
| next_sample_time = local_clock() # Initialize timing baseline | ||
|
|
||
| while self.running and self.wifi_connection: | ||
| try: | ||
| # Verify WiFi data is available | ||
| if hasattr(self.wifi_connection, 'data_available') and self.wifi_connection.data_available: | ||
| current_time = local_clock() | ||
|
|
||
| # Timing gate ensures precise sample rate | ||
| if current_time >= next_sample_time: | ||
| sample = self.wifi_connection.get_latest_sample() | ||
| if sample: | ||
| channel_data = sample[:self.num_channels] | ||
|
|
||
| # Calculate precise timestamp | ||
| sample_time = next_sample_time | ||
| next_sample_time += SAMPLE_INTERVAL | ||
|
|
||
| # If we're falling behind, skip samples to catch up | ||
| if current_time > next_sample_time + SAMPLE_INTERVAL: | ||
| next_sample_time = current_time + SAMPLE_INTERVAL | ||
|
|
||
| if self.lsl_connection: | ||
| self.lsl_connection.push_sample(channel_data, timestamp=sample_time) | ||
|
|
||
| self.update_sample_rate() | ||
|
|
||
| if self.recording_active: | ||
| self.log_to_csv(channel_data) | ||
| except Exception as e: | ||
| print(f"WiFi data handler error: {str(e)}") | ||
| break | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix references to non-existent WiFi methods.
Similar to the BLE handler, this method references self.wifi_connection.data_available and self.wifi_connection.get_latest_sample() which don't exist in the Chords_WIFI class.
The WiFi handler appears to have the same issue as the BLE handler. These methods need to be implemented in the respective protocol classes or the data handling approach needs to be revised.
Run the following script to verify the WiFi class structure:
🏁 Script executed:
#!/bin/bash
# Description: Check for data_available and get_latest_sample methods in Chords_WIFI class
# Search for the methods in the WiFi class
echo "Searching for data_available in Chords_WIFI:"
ast-grep --pattern 'class Chords_WIFI {
$$$
data_available
$$$
}'
echo -e "\nSearching for get_latest_sample method in WiFi class:"
rg "def get_latest_sample" chordspy/chords_wifi.py
echo -e "\nChecking the actual Chords_WIFI class methods:"
rg -A 5 "def " chordspy/chords_wifi.py | head -30Length of output: 1764
Implement missing WiFi methods referenced in wifi_data_handler
The loop in chordspy/connection.py (around lines 331 and 336) calls:
self.wifi_connection.data_availableself.wifi_connection.get_latest_sample()
However, the Chords_WIFI class in chordspy/chords_wifi.py does not define these. You should either:
- Add the missing API to
Chords_WIFI:@property def data_available(self) -> bool: …def get_latest_sample(self) -> Sequence[float]: …
- Or update
wifi_data_handlerto use the existing methods inChords_WIFI(e.g. integrate with itsprocess_data()loop).
Files to update:
- chordspy/connection.py (wifi_data_handler)
- chordspy/chords_wifi.py (implement or adapt methods)
🧰 Tools
🪛 Pylint (3.3.7)
[error] 331-331: Instance of 'Chords_WIFI' has no 'data_available' member
(E1101)
[error] 336-336: Instance of 'Chords_WIFI' has no 'get_latest_sample' member
(E1101)
[refactor] 328-357: Too many nested blocks (6/5)
(R1702)
🤖 Prompt for AI Agents
In chordspy/chords_wifi.py, implement the missing methods used in
chordspy/connection.py's wifi_data_handler: add a @property data_available that
returns a boolean indicating if new WiFi data is ready, and a method
get_latest_sample() that returns the latest sample as a sequence of floats.
Alternatively, if existing methods in Chords_WIFI provide similar functionality,
update wifi_data_handler in chordspy/connection.py to call those instead,
ensuring the API matches. This will align the wifi_data_handler's calls with the
actual Chords_WIFI class capabilities.
| def connect_ble(self, device_address=None): | ||
| """ | ||
| Establishes and manages a Bluetooth Low Energy (BLE) connection with a device. | ||
| The method handles the complete BLE lifecycle including: | ||
| - Device discovery and selection (if no address provided) | ||
| - Connection establishment | ||
| - Data stream configuration | ||
| - Real-time data processing pipeline | ||
| Args: | ||
| device_address (str, optional): MAC address in "XX:XX:XX:XX:XX:XX" format. If None, initiates interactive device selection. | ||
| Returns: | ||
| bool: True if connection succeeds, False on failure | ||
| Workflow: Initialize BLE handler instance -> Configure custom data notification handler -> Establish connection (direct or interactive) -> Set up data processing pipeline -> Maintain connection until termination. | ||
| """ | ||
| # Initialize BLE protocol handler | ||
| self.ble_connection = Chords_BLE() | ||
| original_notification_handler = self.ble_connection.notification_handler | ||
|
|
||
| def notification_handler(sender, data): | ||
| if len(data) == self.ble_connection.NEW_PACKET_LEN: | ||
| if not self.lsl_connection: | ||
| self.setup_lsl(num_channels=3, sampling_rate=500) | ||
|
|
||
| original_notification_handler(sender, data) | ||
|
|
||
| for i in range(0, self.ble_connection.NEW_PACKET_LEN, self.ble_connection.SINGLE_SAMPLE_LEN): | ||
| sample_data = data[i:i+self.ble_connection.SINGLE_SAMPLE_LEN] | ||
| if len(sample_data) == self.ble_connection.SINGLE_SAMPLE_LEN: | ||
| channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True) | ||
| for i in range(1, len(sample_data), 2)] | ||
| self.last_sample = channels | ||
| self.ble_samples_received += 1 | ||
|
|
||
| if self.lsl_connection: # Push to LSL | ||
| self.lsl_connection.push_sample(channels) | ||
| if self.recording_active: | ||
| self.log_to_csv(channels) | ||
|
|
||
| self.ble_connection.notification_handler = notification_handler | ||
|
|
||
| try: | ||
| if device_address: | ||
| print(f"Connecting to BLE device: {device_address}") | ||
| self.ble_connection.connect(device_address) | ||
| else: | ||
| selected_device = asyncio.run(self.get_ble_device()) | ||
| if not selected_device: | ||
| return False | ||
| print(f"Connecting to BLE device: {selected_device.name}") | ||
| self.ble_connection.connect(selected_device.address) | ||
|
|
||
| self.running = True | ||
| self.ble_thread = threading.Thread(target=self.ble_data_handler) | ||
| self.ble_thread.daemon = True | ||
| self.ble_thread.start() | ||
| threading.Thread(target=self.lsl_rate_checker, daemon=True).start() # Start independent rate monitoring | ||
| return True | ||
| except Exception as e: | ||
| print(f"BLE connection failed: {str(e)}") | ||
| return False | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to avoid duplicate data handling logic.
The method creates a custom notification handler that processes BLE data and pushes to LSL/CSV, but there's also a ble_data_handler thread that attempts to do the same thing. This creates:
- Duplicate data processing logic
- Potential race conditions
- Confusion about which handler is actually processing data
Additionally, line 428 has incorrect indentation.
The BLE data handling should be consolidated. Either:
- Use the notification handler approach and remove the
ble_data_handlerthread logic, or - Store incoming data in a thread-safe queue in the notification handler and let
ble_data_handlerprocess it
Also fix the indentation:
- channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True)
- for i in range(1, len(sample_data), 2)]
+ channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True)
+ for i in range(1, len(sample_data), 2)]📝 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.
| def connect_ble(self, device_address=None): | |
| """ | |
| Establishes and manages a Bluetooth Low Energy (BLE) connection with a device. | |
| The method handles the complete BLE lifecycle including: | |
| - Device discovery and selection (if no address provided) | |
| - Connection establishment | |
| - Data stream configuration | |
| - Real-time data processing pipeline | |
| Args: | |
| device_address (str, optional): MAC address in "XX:XX:XX:XX:XX:XX" format. If None, initiates interactive device selection. | |
| Returns: | |
| bool: True if connection succeeds, False on failure | |
| Workflow: Initialize BLE handler instance -> Configure custom data notification handler -> Establish connection (direct or interactive) -> Set up data processing pipeline -> Maintain connection until termination. | |
| """ | |
| # Initialize BLE protocol handler | |
| self.ble_connection = Chords_BLE() | |
| original_notification_handler = self.ble_connection.notification_handler | |
| def notification_handler(sender, data): | |
| if len(data) == self.ble_connection.NEW_PACKET_LEN: | |
| if not self.lsl_connection: | |
| self.setup_lsl(num_channels=3, sampling_rate=500) | |
| original_notification_handler(sender, data) | |
| for i in range(0, self.ble_connection.NEW_PACKET_LEN, self.ble_connection.SINGLE_SAMPLE_LEN): | |
| sample_data = data[i:i+self.ble_connection.SINGLE_SAMPLE_LEN] | |
| if len(sample_data) == self.ble_connection.SINGLE_SAMPLE_LEN: | |
| channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True) | |
| for i in range(1, len(sample_data), 2)] | |
| self.last_sample = channels | |
| self.ble_samples_received += 1 | |
| if self.lsl_connection: # Push to LSL | |
| self.lsl_connection.push_sample(channels) | |
| if self.recording_active: | |
| self.log_to_csv(channels) | |
| self.ble_connection.notification_handler = notification_handler | |
| try: | |
| if device_address: | |
| print(f"Connecting to BLE device: {device_address}") | |
| self.ble_connection.connect(device_address) | |
| else: | |
| selected_device = asyncio.run(self.get_ble_device()) | |
| if not selected_device: | |
| return False | |
| print(f"Connecting to BLE device: {selected_device.name}") | |
| self.ble_connection.connect(selected_device.address) | |
| self.running = True | |
| self.ble_thread = threading.Thread(target=self.ble_data_handler) | |
| self.ble_thread.daemon = True | |
| self.ble_thread.start() | |
| threading.Thread(target=self.lsl_rate_checker, daemon=True).start() # Start independent rate monitoring | |
| return True | |
| except Exception as e: | |
| print(f"BLE connection failed: {str(e)}") | |
| return False | |
| def connect_ble(self, device_address=None): | |
| """ | |
| Establishes and manages a Bluetooth Low Energy (BLE) connection with a device. | |
| ... | |
| """ | |
| # Initialize BLE protocol handler | |
| self.ble_connection = Chords_BLE() | |
| original_notification_handler = self.ble_connection.notification_handler | |
| def notification_handler(sender, data): | |
| if len(data) == self.ble_connection.NEW_PACKET_LEN: | |
| if not self.lsl_connection: | |
| self.setup_lsl(num_channels=3, sampling_rate=500) | |
| original_notification_handler(sender, data) | |
| for i in range(0, self.ble_connection.NEW_PACKET_LEN, self.ble_connection.SINGLE_SAMPLE_LEN): | |
| sample_data = data[i:i+self.ble_connection.SINGLE_SAMPLE_LEN] | |
| if len(sample_data) == self.ble_connection.SINGLE_SAMPLE_LEN: | |
| channels = [int.from_bytes(sample_data[i:i+2], byteorder='big', signed=True) | |
| for i in range(1, len(sample_data), 2)] | |
| self.last_sample = channels | |
| self.ble_samples_received += 1 | |
| if self.lsl_connection: # Push to LSL | |
| self.lsl_connection.push_sample(channels) | |
| if self.recording_active: | |
| self.log_to_csv(channels) | |
| self.ble_connection.notification_handler = notification_handler | |
| try: | |
| if device_address: | |
| print(f"Connecting to BLE device: {device_address}") | |
| self.ble_connection.connect(device_address) | |
| else: | |
| selected_device = asyncio.run(self.get_ble_device()) | |
| if not selected_device: | |
| return False | |
| print(f"Connecting to BLE device: {selected_device.name}") | |
| self.ble_connection.connect(selected_device.address) | |
| self.running = True | |
| self.ble_thread = threading.Thread(target=self.ble_data_handler) | |
| self.ble_thread.daemon = True | |
| self.ble_thread.start() | |
| threading.Thread(target=self.lsl_rate_checker, daemon=True).start() | |
| return True | |
| except Exception as e: | |
| print(f"BLE connection failed: {e}") | |
| return False |
🧰 Tools
🪛 Flake8 (7.2.0)
[error] 428-428: continuation line under-indented for visual indent
(E128)
🤖 Prompt for AI Agents
In chordspy/connection.py between lines 399 and 459, the BLE data processing
logic is duplicated between the custom notification_handler and the
ble_data_handler thread, causing potential race conditions and confusion. To fix
this, consolidate the data handling by either fully processing data inside the
notification_handler and removing the ble_data_handler thread, or modify the
notification_handler to enqueue incoming data into a thread-safe queue and let
ble_data_handler consume and process it. Also, correct the indentation issue at
line 428 to align the code properly within the notification_handler function.
PayalLakra
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
chordspy/app.py (2)
365-365: Disable debug mode for production deployment.Running Flask with
debug=Trueis a security risk as it exposes sensitive information and enables the interactive debugger.Consider using environment variables to control debug mode:
- app.run(debug=True, use_reloader=False, host='0.0.0.0', port=5000) + debug_mode = os.environ.get('FLASK_DEBUG', 'False').lower() == 'true' + app.run(debug=debug_mode, use_reloader=False, host='0.0.0.0', port=5000)
258-284: Fix connection return value handling in run_connection.The connection methods are expected to return boolean values, but based on the static analysis and past review comments, these methods don't consistently return success/failure status. This causes issues where
successmay beNoneeven when connections succeed.For WiFi connections specifically, the method doesn't return a value, so
successbecomesNone:elif protocol == 'wifi': - success = connection_manager.connect_wifi() + connection_manager.connect_wifi() + success = True # WiFi connection starts background threadsThis ensures the success variable is properly set for all connection types.
chordspy/chords_wifi.py (1)
125-128: Log or use the calculated rates.The sample rate (sps), frame rate (fps), and byte rate (bps) are calculated but never used or logged, making this computation wasteful.
# Calculate samples, packets, and bytes per second 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) + + # Log the rates for monitoring + print(f"Rates - Samples: {sps:.1f}/s, Packets: {fps:.1f}/s, Bytes: {bps:.1f}/s")Alternatively, if these rates aren't needed, consider removing the calculations entirely.
🧹 Nitpick comments (6)
.github/workflows/release.yaml (1)
133-133: Add missing newline at end of file.The file is missing a newline character at the end, which violates YAML formatting standards.
generate_release_notes: true +chordspy/app.py (1)
58-58: Remove unused exception variable.The exception variable
eis assigned but never used in the except block.except Exception as e: - return jsonify({'status': 'error', 'message': 'Logging failed'}), 500 + return jsonify({'status': 'error', 'message': 'Logging failed'}), 500Or utilize the exception for better error reporting:
except Exception as e: - return jsonify({'status': 'error', 'message': 'Logging failed'}), 500 + return jsonify({'status': 'error', 'message': f'Logging failed: {str(e)}'}), 500chordspy/connection.py (4)
44-44: Fix typo in docstring.There's a typo "monitorin" that should be "monitoring".
- - Sample validation and rate monitorin + - Sample validation and rate monitoring
36-36: Add missing blank lines before class definition.Python PEP 8 requires 2 blank lines before class definitions.
+ class Connection:
628-628: Add missing blank lines before function definition.Python PEP 8 requires 2 blank lines before top-level function definitions.
+ def main():
680-681: Add missing blank lines after function definition.Python PEP 8 requires 2 blank lines after class or function definitions at module level.
if __name__ == '__main__': main() + +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yaml(1 hunks)chordspy/app.py(1 hunks)chordspy/chords_wifi.py(6 hunks)chordspy/connection.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
chordspy/connection.py (3)
chordspy/chords_serial.py (5)
Chords_USB(17-275)read_data(161-200)detect_hardware(121-141)send_command(143-159)cleanup(222-231)chordspy/chords_wifi.py (3)
Chords_WIFI(12-198)connect(60-74)cleanup(180-192)chordspy/chords_ble.py (6)
Chords_BLE(13-238)scan_devices(55-69)notification_handler(113-129)connect(215-231)cleanup(204-213)stop(233-238)
🪛 ast-grep (0.38.1)
chordspy/app.py
[warning] 364-364: 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, use_reloader=False, host='0.0.0.0', port=5000)
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)
🪛 Ruff (0.11.9)
chordspy/app.py
58-58: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
chordspy/connection.py
161-161: Use a context manager for opening files
(SIM115)
264-264: Local variable estimated_rate is assigned to but never used
Remove assignment to unused variable estimated_rate
(F841)
668-669: Use a single if statement instead of nested if statements
(SIM102)
chordspy/chords_wifi.py
125-125: Local variable sps is assigned to but never used
Remove assignment to unused variable sps
(F841)
126-126: Local variable fps is assigned to but never used
Remove assignment to unused variable fps
(F841)
127-127: Local variable bps is assigned to but never used
Remove assignment to unused variable bps
(F841)
🪛 Flake8 (7.2.0)
chordspy/app.py
[error] 40-40: expected 2 blank lines, found 1
(E302)
[error] 58-58: local variable 'e' is assigned to but never used
(F841)
[error] 62-62: expected 2 blank lines, found 1
(E302)
[error] 80-80: expected 2 blank lines, found 1
(E302)
[error] 86-86: expected 2 blank lines, found 1
(E302)
[error] 94-94: at least two spaces before inline comment
(E261)
[error] 109-109: expected 2 blank lines, found 1
(E302)
[error] 121-121: continuation line under-indented for visual indent
(E128)
[error] 128-128: expected 2 blank lines, found 1
(E302)
[error] 139-139: expected 2 blank lines, found 1
(E302)
[error] 151-151: expected 2 blank lines, found 1
(E302)
[error] 165-165: expected 2 blank lines, found 1
(E302)
[error] 181-181: expected 2 blank lines, found 1
(E302)
[error] 199-199: missing whitespace after ','
(E231)
[error] 215-215: expected 2 blank lines, found 1
(E302)
[error] 233-233: expected 2 blank lines, found 1
(E302)
[error] 292-292: expected 2 blank lines, found 1
(E302)
[error] 299-299: global connection_manager is unused: name is never assigned in scope
(F824)
[error] 308-308: expected 2 blank lines, found 1
(E302)
[error] 315-315: global connection_manager is unused: name is never assigned in scope
(F824)
[error] 336-336: expected 2 blank lines, found 1
(E302)
[error] 343-343: global connection_manager is unused: name is never assigned in scope
(F824)
[error] 356-356: expected 2 blank lines, found 1
(E302)
[error] 367-367: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/connection.py
[error] 36-36: expected 2 blank lines, found 1
(E302)
[error] 264-264: local variable 'estimated_rate' is assigned to but never used
(F841)
[error] 264-264: at least two spaces before inline comment
(E261)
[error] 428-428: continuation line under-indented for visual indent
(E128)
[error] 628-628: expected 2 blank lines, found 1
(E302)
[error] 680-680: expected 2 blank lines after class or function definition, found 1
(E305)
chordspy/chords_wifi.py
[error] 12-12: expected 2 blank lines, found 1
(E302)
[error] 125-125: local variable 'sps' is assigned to but never used
(F841)
[error] 126-126: local variable 'fps' is assigned to but never used
(F841)
[error] 127-127: local variable 'bps' is assigned to but never used
(F841)
[error] 200-200: expected 2 blank lines after class or function definition, found 1
(E305)
🪛 Pylint (3.3.7)
chordspy/app.py
[refactor] 206-206: Consider using 'with' for resource-allocating operations
(R1732)
[refactor] 225-229: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[error] 266-266: Assigning result of a function call, where the function has no return
(E1111)
[error] 277-277: Possibly using variable 'success' before assignment
(E0606)
chordspy/connection.py
[refactor] 36-36: Too many instance attributes (27/7)
(R0902)
[refactor] 161-161: Consider using 'with' for resource-allocating operations
(R1732)
[error] 290-290: Instance of 'Chords_BLE' has no 'data_available' member
(E1101)
[error] 295-295: Instance of 'Chords_BLE' has no 'get_latest_sample' member
(E1101)
[refactor] 287-319: Too many nested blocks (6/5)
(R1702)
[error] 331-331: Instance of 'Chords_WIFI' has no 'data_available' member
(E1101)
[error] 336-336: Instance of 'Chords_WIFI' has no 'get_latest_sample' member
(E1101)
[refactor] 328-357: Too many nested blocks (6/5)
(R1702)
[refactor] 366-397: Too many nested blocks (6/5)
(R1702)
[refactor] 558-558: Too many branches (15/12)
(R0912)
chordspy/chords_wifi.py
[refactor] 12-12: Too many instance attributes (14/7)
(R0902)
[refactor] 33-33: Too many arguments (6/5)
(R0913)
[refactor] 33-33: Too many positional arguments (6/5)
(R0917)
[refactor] 87-87: Too many branches (15/12)
(R0912)
[refactor] 87-87: Too many statements (58/50)
(R0915)
🪛 YAMLlint (1.37.1)
.github/workflows/release.yaml
[error] 133-133: no new line character at the end of file
(new-line-at-end-of-file)
| class Connection: | ||
| """ | ||
| Main connection manager class for supported devices. | ||
| This class serves as the central hub for all device communication, providing: | ||
| - Unified interface across multiple connection protocols(WiFi/BLE/USB) | ||
| - Data streaming to LSL | ||
| - Data recording to CSV files | ||
| - Connection state management | ||
| - Sample validation and rate monitorin | ||
| The class maintains separate connection handlers for each protocol (USB/WiFi/BLE) | ||
| and manages their lifecycle. It implements thread-safe operations for concurrent | ||
| data handling and provides clean shutdown procedures. | ||
| """ | ||
| def __init__(self): | ||
| """ | ||
| Initialize the connection manager with default values. | ||
| """ | ||
| # Protocol Connection Handlers | ||
| self.ble_connection = None # BLE protocol handler | ||
| self.wifi_connection = None # WiFi protocol handler | ||
| self.usb_connection = None # USB protocol handler | ||
| self.lsl_connection = None # LSL stream outlet (created when streaming starts) | ||
|
|
||
| # LSL Stream Configuration | ||
| self.stream_name = "BioAmpDataStream" # Default LSL stream name | ||
| self.stream_type = "EXG" # LSL stream type | ||
| self.stream_format = "float32" # Data format for LSL samples | ||
| self.stream_id = "UDL" # Unique stream identifier | ||
|
|
||
| # Data Tracking Systems | ||
| self.last_sample = None # Stores the most recent sample received | ||
| self.samples_received = 0 # Total count of samples received | ||
| self.start_time = time.time() # Timestamp when connection was established | ||
|
|
||
| # CSV Recording Systems | ||
| self.csv_file = None # File handle for CSV output | ||
| self.csv_writer = None # CSV writer object | ||
| self.sample_counter = 0 # Count of samples written to CSV | ||
|
|
||
| # Stream Parameters | ||
| self.num_channels = 0 # Number of data channels | ||
| self.sampling_rate = 0 # Current sampling rate in Hz | ||
|
|
||
| # System State Flags | ||
| self.stream_active = False # True when LSL streaming is active | ||
| self.recording_active = False # True when CSV recording is active | ||
|
|
||
| # Thread Management | ||
| self.usb_thread = None # Thread for USB data handling | ||
| self.ble_thread = None # Thread for BLE data handling | ||
| self.wifi_thread = None # Thread for WiFi data handling | ||
| self.running = False # Main system running flag | ||
|
|
||
| # Rate Monitoring Systems | ||
| self.sample_count = 0 # Samples received in current interval | ||
| self.rate_window = deque(maxlen=10) # Window for rate calculation | ||
| self.last_timestamp = time.perf_counter() # Last rate calculation time | ||
| self.rate_update_interval = 0.5 # Seconds between rate updates | ||
| self.ble_samples_received = 0 # Count of BLE-specific samples | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring the Connection class to reduce complexity.
The class has 27 instance attributes and handles multiple responsibilities (protocol management, LSL streaming, CSV recording, rate monitoring, thread management). This violates the Single Responsibility Principle and makes the class harder to maintain and test.
Consider breaking this into separate classes:
ConnectionManager(main orchestrator)LSLStreamer(handles LSL operations)CSVRecorder(handles file recording)RateMonitor(handles sample rate calculations)
This would improve modularity, testability, and maintainability.
🧰 Tools
🪛 Flake8 (7.2.0)
[error] 36-36: expected 2 blank lines, found 1
(E302)
🪛 Pylint (3.3.7)
[refactor] 36-36: Too many instance attributes (27/7)
(R0902)
🤖 Prompt for AI Agents
In chordspy/connection.py around lines 36 to 95, the Connection class is overly
complex with 27 instance attributes managing multiple responsibilities like
protocol connections, LSL streaming, CSV recording, rate monitoring, and
threading. To fix this, refactor by splitting the class into smaller focused
classes: create a ConnectionManager as the main orchestrator, an LSLStreamer
class for LSL stream handling, a CSVRecorder class for CSV file operations, and
a RateMonitor class for sample rate calculations. Move related attributes and
methods into these classes to improve modularity, maintainability, and
testability.
| elif not filename.endswith('.csv'): | ||
| filename += '.csv' | ||
|
|
||
| self.csv_file = open(filename, 'w', newline='') # Open CSV file and initialize writer |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use context manager for file operations.
The file is opened without a context manager, which could lead to resource leaks if an exception occurs before the file is properly closed.
- self.csv_file = open(filename, 'w', newline='')
+ with open(filename, 'w', newline='') as self.csv_file:However, since the file needs to remain open for the duration of recording, consider implementing a proper context manager pattern in the class or ensure robust error handling in the cleanup methods.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.11.9)
161-161: Use a context manager for opening files
(SIM115)
🪛 Pylint (3.3.7)
[refactor] 161-161: Consider using 'with' for resource-allocating operations
(R1732)
🤖 Prompt for AI Agents
In chordspy/connection.py at line 161, the CSV file is opened without using a
context manager, risking resource leaks if exceptions occur. Refactor the class
to implement the context manager protocol (__enter__ and __exit__ methods) to
ensure the file is properly closed after use, or alternatively, add robust error
handling and explicit file closing in the cleanup or destructor methods to
guarantee the file resource is released correctly.
I have made the following changes:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores