Implement AF-Server Client package for LabVIEW TCP communication#1
Implement AF-Server Client package for LabVIEW TCP communication#1
Conversation
… and benchmarking Co-authored-by: Faragoz <62692906+Faragoz@users.noreply.github.com>
Co-authored-by: Faragoz <62692906+Faragoz@users.noreply.github.com>
|
@copilot Acabé de poner el repositorio de AF-Serializer como publico, integralo y prueba si funciona con el codigo proporcionado. |
Co-authored-by: Faragoz <62692906+Faragoz@users.noreply.github.com>
Integrado y probado. AF-Serializer funciona correctamente con el código:
Commit: ee65be5 |
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive AF-Server Client package for low-latency TCP communication with LabVIEW servers using the AF-Serializer protocol. The implementation follows a well-structured, modular architecture with multiple phases completed including core functionality, console UI, benchmarking, PsychoPy integration, and comprehensive testing.
Key Changes:
- Asyncio-based TCP client with low-latency optimizations (TCP_NODELAY, QoS, uvloop)
- Request-response architecture with UUID correlation and metrics tracking
- Interactive Textual TUI console with real-time status monitoring and command execution
- RTT benchmarking system with JSON/CSV/Excel export capabilities
- PsychoPy integration for laboratory experiments with singleton client pattern
- Mock AF-Serializer implementation for testing when the real package is unavailable
- Comprehensive test suite with 96 passing tests
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
af_server_client/core/tcp_client.py |
Implements asyncio TCP client with low-latency optimizations; contains minor bugs with exception naming and uninitialized attribute |
af_server_client/core/response_handler.py |
Manages request-response correlation with UUID tracking and metrics calculation |
af_server_client/core/protocol.py |
Defines AF-Serializer protocol classes for commands and responses |
af_server_client/core/config.py |
TOML-based configuration management with hot-reload support |
af_server_client/console/app.py |
Main Textual TUI application with status panels and command execution |
af_server_client/console/commands.py |
Command executor with registry pattern for console commands |
af_server_client/console/command_parser.py |
Parses and validates console commands with history support |
af_server_client/console/widgets.py |
Custom Textual widgets for status and progress display |
af_server_client/benchmark/rtt_test.py |
RTT benchmarking implementation with progress callbacks |
af_server_client/benchmark/metrics.py |
Data classes for benchmark samples and statistics |
af_server_client/benchmark/export.py |
Export functionality for JSON/CSV/Excel formats |
af_server_client/psychopy_bridge/shared_client.py |
Singleton pattern for shared TCP client; has lock initialization issue |
af_server_client/psychopy_bridge/launcher.py |
Subprocess launcher for PsychoPy experiments |
af_server_client/psychopy_bridge/subject_gui.py |
PsychoPy-based subject interface with buttons and sliders |
af_server_client/cli.py |
CLI entry point with argparse configuration; minor style issue |
af_server_client/_mock/af_serializer.py |
Mock implementation for testing without real AF-Serializer package |
tests/*.py |
Comprehensive test suite covering all major components |
pyproject.toml |
Package configuration with dependencies and build settings |
af_server_config.toml |
Example configuration file with all available settings |
.github/workflows/ci.yml |
CI pipeline with linting, type checking, and testing |
README.md |
Comprehensive documentation with usage examples and API reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| except asyncio.TimeoutError as e: | ||
| sock.close() | ||
| raise ConnectionError(f"Connection timed out to {target_host}:{target_port}") from e |
There was a problem hiding this comment.
Missing format string parameter. The test expects the error message to include "timed out", but the ConnectionError is raised with just a format string placeholder without the actual host/port values being formatted in.
| """Base exception for TCP client errors.""" | ||
|
|
||
|
|
||
| class ConnectionError(TCPClientError): |
There was a problem hiding this comment.
Class name ConnectionError conflicts with Python's built-in ConnectionError exception. This shadowing can cause confusion and potential bugs when trying to catch the built-in exception. Consider renaming to TCPConnectionError or ClientConnectionError.
| class ConnectionError(TCPClientError): | |
| class TCPConnectionError(TCPClientError): |
| self.reader, self.writer = await asyncio.open_connection(sock=sock) | ||
| self.connected = True | ||
| self._connection_time = time.time() | ||
| self._reconnect_attempts = 0 |
There was a problem hiding this comment.
The _reconnect_attempts attribute is set but never defined in __init__. This will cause an AttributeError if the attribute is accessed elsewhere in the code. Either add this to __init__ or remove the assignment if it's unused.
| _instance: Optional["SharedClient"] = None | ||
| _client: Optional["TCPClient"] = None | ||
| _config: Optional["Config"] = None | ||
| _lock = asyncio.Lock() |
There was a problem hiding this comment.
The class-level asyncio.Lock() is created at module import time, which may cause issues if the event loop hasn't been initialized yet. Consider creating the lock lazily on first access or within an async context to avoid potential RuntimeError: no running event loop errors.
|
|
||
| if TYPE_CHECKING: | ||
| from af_server_client.core.config import Config | ||
|
|
There was a problem hiding this comment.
Missing blank line before function definition according to PEP 8 style guidelines.
| license = {text = "MIT"} | ||
|
|
||
| dependencies = [ | ||
| "af-serializer @ git+https://github.com/Faragoz/AF-Serializer.git", |
There was a problem hiding this comment.
The dependency af-serializer @ git+https://github.com/Faragoz/AF-Serializer.git is a VCS URL without a pinned commit/tag, creating a supply-chain risk: upstream changes (or repo compromise) can silently alter installed code and execute during install/import. Pin to an immutable reference or a published package to ensure integrity, e.g.
# Pin to a specific commit SHA
"af-serializer @ git+https://github.com/Faragoz/AF-Serializer.git@<commit_sha>"
# Prefer published versions on PyPI with version constraints
"af-serializer==<version>"| "af-serializer @ git+https://github.com/Faragoz/AF-Serializer.git", | |
| "af-serializer @ git+https://github.com/Faragoz/AF-Serializer.git@<commit_sha>", |
Original prompt
AF-Server Client Implementation Task
🎯 Project Overview
Develop a modular, low-latency Python TCP client to communicate with a LabVIEW server using AF-Serializer protocol. The client includes a Textual-based interactive console for managing communications, executing commands, and launching PsychoPy interfaces for laboratory experiments with CRIO devices.
📋 Core Requirements
1. Low-Latency TCP Communication
Network Optimizations:
socket.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)socket.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, 0xB8)(DSCP EF)Protocol Classes (AF-Serializer):