-
Notifications
You must be signed in to change notification settings - Fork 18
Copilot/add comprehensive testing #7
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
base: main
Are you sure you want to change the base?
Copilot/add comprehensive testing #7
Conversation
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
…delays Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
…ed cache writes Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
…-functionality Add advanced DLC flashing with WebSocket progress tracking and one-click workflow
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
…ization Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
Optimize performance bottlenecks: concurrent BLE reads, async I/O, cache write batching
Co-authored-by: benbalter <282759+benbalter@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive DLC (DownLoadable Content) management functionality to PyFluff, including a complete web UI, backend API endpoints, extensive test coverage, and CI/CD configuration. The changes enable users to upload custom DLC files to Furby Connect via a modern web interface with real-time progress tracking.
Key changes:
- Complete DLC management web UI with progress tracking via WebSocket
- New FastAPI endpoint for one-click "flash and activate" workflow
- Async file I/O optimizations for better DLC upload performance
- 114 new tests achieving 37% overall coverage (100% for core protocol/models)
- GitHub Actions CI/CD pipeline with testing, linting, and type checking
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/index.html | Adds DLC management UI section with file upload, slot selection, and advanced controls |
| web/style.css | Adds styling for DLC UI including progress bars, buttons, and animations |
| web/app.js | Implements DLC upload logic, WebSocket progress tracking, and event handlers |
| pyfluff/server.py | Adds /dlc/flash-and-activate endpoint and WebSocket for progress updates |
| pyfluff/dlc.py | Adds flash_and_activate() method and async file I/O with progress callbacks |
| pyfluff/furby.py | Adds device info caching and concurrent BLE reads for performance |
| pyfluff/furby_cache.py | Adds async file I/O and batched writes for cache operations |
| pyfluff/protocol.py | Reformats FURBY_NAMES dict and adds missing name entry (Tay-Tah) |
| tests/* | Adds 114 tests across 4 new test files for protocol, models, cache, and DLC |
| .github/workflows/ci.yml | Adds CI/CD pipeline with Python 3.11/3.12 testing matrix |
| TESTING.md | Comprehensive testing documentation with examples and best practices |
| pyproject.toml | Updates ruff config to ignore B008 and B904 for specific use cases |
| offset = 0 | ||
| chunk_count = 0 | ||
|
|
||
| while offset < file_size: | ||
| chunk = dlc_data[offset : offset + FILE_CHUNK_SIZE] | ||
| await self.furby._write_file(chunk) | ||
| offset += len(chunk) | ||
| chunk_count += 1 | ||
| async with aiofiles.open(dlc_path, "rb") as f: | ||
| while True: | ||
| chunk = await f.read(FILE_CHUNK_SIZE) | ||
| if not chunk: | ||
| break | ||
|
|
||
| # Small delay to prevent overwhelming Furby | ||
| await asyncio.sleep(0.005) | ||
| await self.furby._write_file(chunk) | ||
| chunk_count += 1 | ||
|
|
||
| # Progress logging | ||
| if chunk_count % 100 == 0: | ||
| progress = (offset / file_size) * 100 | ||
| logger.info(f"Upload progress: {progress:.1f}%") | ||
| # Small delay to prevent overwhelming Furby | ||
| # Reduced from 0.005 to 0.002 to speed up transfer and avoid Furby timeout. | ||
| # NOTE: This value may require calibration for different Furby devices | ||
| # or BLE implementations. | ||
| await asyncio.sleep(0.002) | ||
|
|
||
| # Progress updates (every 5% of file size for consistent UX) | ||
| progress = (offset / file_size) * 100 |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The offset variable is never incremented inside the loop, causing progress calculation to always be 0%. Add offset += len(chunk) after line 144 to track bytes uploaded correctly.
| assert cmd[1] == 0x00 # Fixed 0x00 byte | ||
|
|
||
| # Check 0x00 byte | ||
| assert cmd[1] == 0x00 |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 103 and 106 are duplicate assertions checking the same value. Remove the duplicate assertion at line 106.
| assert cmd[1] == 0x00 |
| # Check size bytes (big-endian) - note offset by 1 due to 0x00 byte | ||
| # Check size bytes (big-endian, 3 bytes) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two redundant comments on consecutive lines describing the same thing. Merge into a single clear comment: # Check size bytes (3 bytes, big-endian, offset by 1 due to 0x00 byte)
| # Check size bytes (big-endian) - note offset by 1 due to 0x00 byte | |
| # Check size bytes (big-endian, 3 bytes) | |
| # Check size bytes (3 bytes, big-endian, offset by 1 due to 0x00 byte) |
| elif mode == FileTransferMode.FILE_TRANSFER_TIMEOUT: | ||
| self._transfer_error = "File transfer timeout" | ||
| self._transfer_complete.set() | ||
| self._transfer_complete.set() # Must set event to unblock the wait |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The inline comment states 'Must set event to unblock the wait', but this applies to all error paths (lines 54, 57, 59). Consider moving this explanation to a docstring or single comment above the error handling block to avoid repetition and improve clarity.
See below for a potential fix:
# In all error paths, must set the event to unblock any waiters.
elif mode == FileTransferMode.FILE_RECEIVED_ERROR:
self._transfer_error = "File transfer failed"
self._transfer_complete.set()
elif mode == FileTransferMode.FILE_TRANSFER_TIMEOUT:
self._transfer_error = "File transfer timeout"
self._transfer_complete.set()
Testing and CI Enhancements:
TESTING.mdguide covering test coverage, running tests, coverage reporting, mocking strategies, and best practices for contributing tests. This will help new contributors understand and maintain the test suite..github/workflows/ci.ymlthat runs tests on Python 3.11 and 3.12, checks code formatting and linting, and performs type checking. Coverage reports are uploaded to Codecov.DLC Upload Improvements:
DLCManager.upload_dlcto support an optional async progress callback, increased the default upload timeout to 5 minutes, and improved internal progress tracking. This enables better feedback for long-running uploads and more robust error handling. [1] [2] [3] [4] [5]CLI and Code Quality Improvements:
raise typer.Exit(1) from efor better tracebacks and debugging. [1] [2] [3] [4]pyfluff/cli.pyandpyfluff/__init__.pyfor clarity and style consistency. [1] [2] [3] [4] [5] [6]These updates collectively improve the maintainability, usability, and reliability of the PyFluff codebase.