Repository navigation
Refactor server code into modular structure - #18
Conversation
Break down the monolithic 1154-line server.py into smaller, focused modules: Core modules: - state.py: ServerState class and global state management - channels.py: Channel name mappings and parsing utilities - connection.py: Connection management and auto-reconnect logic - message_handlers.py: Event callbacks and subscription cleanup Tools directory (tools/): - connect.py: Connection/disconnection MCP tools - messages.py: Message sending and listening MCP tools - device.py: Device info, battery, contacts, and advert MCP tools - time.py: Time synchronization MCP tools Benefits: - Main server.py reduced from 1154 to 246 lines (79% smaller) - Each tool category in separate file for easy maintenance - Clear separation of concerns - Easier to test and extend individual components The server functionality remains identical, confirmed by successful --help test.
There was a problem hiding this comment.
Pull request overview
This PR successfully refactors a monolithic 1154-line server.py into a well-organized modular structure, reducing the main file to 246 lines (79% reduction). The refactoring extracts server state, channel utilities, connection management, message handlers, and tool implementations into separate focused modules while maintaining identical functionality.
Key Changes:
- Extracted core functionality into dedicated modules (
state.py,channels.py,connection.py,message_handlers.py) - Organized MCP tools into a
tools/directory with separate files for each category (connect, messages, device, time) - Improved code organization with clear separation of concerns and no circular dependencies
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/meshcore_mcp/server.py |
Main server file reduced from 1154 to 246 lines; now focuses on server initialization, startup logic, and tool registration |
src/meshcore_mcp/state.py |
New module managing global ServerState class with connection and message listening state |
src/meshcore_mcp/channels.py |
New module for channel name mappings and parsing utilities |
src/meshcore_mcp/connection.py |
New module handling connection management and auto-reconnect logic |
src/meshcore_mcp/message_handlers.py |
New module with event callbacks for contact/channel messages and subscription cleanup |
src/meshcore_mcp/tools/__init__.py |
New tools package initialization importing all tool modules |
src/meshcore_mcp/tools/connect.py |
Connection and disconnection MCP tools extracted from server.py |
src/meshcore_mcp/tools/messages.py |
Message sending and listening MCP tools extracted from server.py |
src/meshcore_mcp/tools/device.py |
Device info, battery, contacts, and advertisement MCP tools extracted from server.py |
src/meshcore_mcp/tools/time.py |
Time synchronization MCP tools extracted from server.py |
|
|
||
| try: | ||
| # Get current system time as Unix timestamp | ||
| import time |
There was a problem hiding this comment.
The import time statement should be moved to the module level (top of the file) rather than being inside the function. This is a best practice for maintainability and makes dependencies explicit. Consider adding import time at the top of the file alongside the from datetime import datetime import.
| from starlette.middleware.base import BaseHTTPMiddleware | ||
| from starlette.requests import Request |
There was a problem hiding this comment.
[nitpick] Consider moving these imports (BaseHTTPMiddleware and Request) to the module level at the top of the file, following the same pattern as the asynccontextmanager import that was moved up. This improves code organization and makes dependencies more explicit.
| connection_params: dict = {} | ||
| debug: bool = False | ||
|
|
||
| # Message listening state | ||
| message_buffer: deque = deque(maxlen=1000) # Store up to 1000 messages | ||
| message_subscriptions: List = [] # Active subscriptions | ||
| is_listening: bool = False | ||
|
|
There was a problem hiding this comment.
[nitpick] The class attributes message_buffer, message_subscriptions, and connection_params are mutable objects initialized at the class level. While this works for the singleton pattern used here, it's better practice to initialize mutable default values in an __init__ method to avoid potential issues if multiple instances are created. Consider adding:
def __init__(self):
self.message_buffer = deque(maxlen=1000)
self.message_subscriptions = []
self.connection_params = {}This makes the code more robust and follows Python best practices.
| connection_params: dict = {} | |
| debug: bool = False | |
| # Message listening state | |
| message_buffer: deque = deque(maxlen=1000) # Store up to 1000 messages | |
| message_subscriptions: List = [] # Active subscriptions | |
| is_listening: bool = False | |
| connection_params: dict | |
| debug: bool = False | |
| # Message listening state | |
| message_buffer: deque | |
| message_subscriptions: List | |
| is_listening: bool = False | |
| def __init__(self): | |
| self.message_buffer = deque(maxlen=1000) # Store up to 1000 messages | |
| self.message_subscriptions = [] # Active subscriptions | |
| self.connection_params = {} |
Break down the monolithic 1154-line server.py into smaller, focused modules:
Core modules:
Tools directory (tools/):
Benefits:
The server functionality remains identical, confirmed by successful --help test.