Files
usda-vision/docs/REFACTORING_SUMMARY.md
salirezav f6a37ca1ba Remove deprecated files and scripts to streamline the codebase
- Deleted unused API test files, RTSP diagnostic scripts, and development utility scripts to reduce clutter.
- Removed outdated database schema and modularization proposal documents to maintain focus on current architecture.
- Cleaned up configuration files and logging scripts that are no longer in use, enhancing project maintainability.
2025-11-02 10:07:59 -05:00

14 KiB

Code Quality Refactoring Summary

Date: November 2025
Branch: Modularization branch
Status: Completed and Verified

Overview

This document summarizes the modularization and code quality refactoring work performed on the USDA Vision system. The work was done in two phases:

  1. Frontend Modularization (React Dashboard) - Extracted features into microfrontends using Module Federation
  2. Backend Refactoring (Camera Management API) - Improved code organization within the monolithic architecture

This document focuses primarily on Phase 2 (API refactoring), but provides context about the overall modularization strategy.

Project Context: Two-Phase Modularization

Phase 1: Frontend Modularization (React Dashboard)

Status: Completed

Before working on the API, we first modularized the React dashboard application into a microfrontend architecture:

  • Approach: Used Vite Module Federation to create independently deployable frontend modules
  • First Module Extracted: video-remote - The video library feature was extracted into its own microfrontend
  • Architecture:
    • Main dashboard acts as a "shell" application
    • Remotely loads video-remote module when enabled via feature flags
    • Supports gradual migration (local fallback components remain available)
  • Infrastructure Changes:
    • Created separate media-api container for video processing (thumbnails, transcoding)
    • Added mediamtx container for RTSP/WebRTC streaming
    • video-remote container runs independently and can be updated separately
  • Benefits Achieved:
    • Independent deployment of video library feature
    • Better separation of concerns (media handling separate from main dashboard)
    • Foundation for extracting more features (camera management, experiments, etc.)

Phase 2: Backend Refactoring (Camera Management API)

Status: Completed

After successfully modularizing the frontend, we focused on improving the backend code quality. Important: We chose NOT to split the API into microservices, but rather to improve the organization within the existing monolithic architecture.

  • Approach: Simple, low-risk refactorings within the monolithic structure
  • Philosophy: "Simplest least destructive code refactorings that can significantly make the code more readable and manageable and editable"
  • Decision: Keep monolithic architecture (no microservices) but improve internal organization
  • Why Monolithic:
    • Camera SDK and hardware interactions require tight coupling
    • System is stable and working well
    • Full microservices would be overkill and add complexity
    • Focus on code quality over architectural changes

Motivation

The API refactoring was requested to improve code quality and manageability within the existing monolithic architecture. The goal was to improve organization without resorting to full microservices architecture or breaking changes, following the successful pattern we established with the frontend modularization.

Refactoring Tasks Completed

1. Extract Duplicate Code (suppress_camera_errors)

Problem: The suppress_camera_errors() context manager was duplicated in three files:

  • camera/recorder.py
  • camera/streamer.py
  • camera/monitor.py

Solution:

  • Created camera/utils.py with the centralized suppress_camera_errors() function
  • Updated all three files to import from utils instead of defining locally

Files Changed:

  • Created: camera-management-api/usda_vision_system/camera/utils.py
  • Updated: camera/recorder.py - removed local definition, added import
  • Updated: camera/streamer.py - removed local definition, added import
  • Updated: camera/monitor.py - removed local definition, added import

Benefits:

  • Single source of truth for error suppression logic
  • Easier maintenance (fix bugs in one place)
  • Consistent behavior across all camera modules

2. Extract Magic Numbers into Constants

Problem: Magic numbers scattered throughout camera code made it hard to understand intent and adjust settings:

  • Queue sizes (5, 10, 30)
  • Frame rates (10.0, 15.0, 30.0)
  • Timeouts (200, 1000, 500 milliseconds)
  • JPEG quality (70)
  • Sleep intervals (0.1 seconds)

Solution:

  • Created camera/constants.py with well-named constants
  • Replaced all magic numbers with constant references

Constants Defined:

# Queue sizes
MJPEG_QUEUE_MAXSIZE = 5
RTSP_QUEUE_MAXSIZE = 10
RECORDING_QUEUE_MAXSIZE = 30

# Frame rates
PREVIEW_FPS = 10.0
RTSP_FPS = 15.0
DEFAULT_VIDEO_FPS = 30.0

# JPEG quality
PREVIEW_JPEG_QUALITY = 70

# Timeouts (milliseconds)
CAMERA_GET_BUFFER_TIMEOUT = 200
CAMERA_INIT_TIMEOUT = 1000
CAMERA_TEST_CAPTURE_TIMEOUT = 500

# Sleep intervals (seconds)
STREAMING_LOOP_SLEEP = 0.1
BRIEF_PAUSE_SLEEP = 0.1

Files Changed:

  • Created: camera-management-api/usda_vision_system/camera/constants.py
  • Updated: camera/recorder.py - replaced magic numbers with constants
  • Updated: camera/streamer.py - replaced magic numbers with constants
  • Updated: camera/manager.py - replaced magic numbers with constants
  • Updated: camera/monitor.py - added import for CAMERA_TEST_CAPTURE_TIMEOUT

Benefits:

  • Self-documenting code (constants explain what values represent)
  • Easy to adjust performance settings (change in one place)
  • Reduced risk of inconsistent values across modules
  • Better code readability

3. Split Monolithic API Routes into Domain Modules

Problem: api/server.py was 868 lines with all routes defined in a single _setup_routes() method, making it:

  • Hard to navigate and find specific endpoints
  • Difficult to maintain (one large file)
  • Prone to merge conflicts
  • Not following separation of concerns

Solution:

  • Created api/routes/ directory with domain-specific route modules
  • Each module exports a register_*_routes() function
  • Updated server.py to import and call these registration functions

New File Structure:

api/routes/
├── __init__.py                    # Exports all register functions
├── system_routes.py               # /, /health, /system/status, /system/video-module
├── camera_routes.py               # /cameras, /cameras/{name}/*, RTSP endpoints
├── recording_routes.py            # /cameras/{name}/start-recording, stop-recording
├── mqtt_routes.py                 # /mqtt/status, /mqtt/events
├── storage_routes.py              # /storage/stats, /storage/files, /storage/cleanup
├── auto_recording_routes.py       # /cameras/{name}/auto-recording/*
└── recordings_routes.py           # /recordings

Files Changed:

  • Created: api/routes/__init__.py
  • Created: api/routes/system_routes.py - 7 routes
  • Created: api/routes/camera_routes.py - 14 routes
  • Created: api/routes/recording_routes.py - 2 routes
  • Created: api/routes/mqtt_routes.py - 2 routes
  • Created: api/routes/storage_routes.py - 3 routes
  • Created: api/routes/auto_recording_routes.py - 3 routes
  • Created: api/routes/recordings_routes.py - 1 route
  • Updated: api/server.py - reduced from 868 lines to 315 lines (63% reduction)

Remaining in server.py:

  • WebSocket endpoint (/ws) - kept here as it's core to the server
  • Debug endpoint (/debug/camera-manager) - utility endpoint
  • Video module route integration - dynamic route inclusion

Import Path Corrections Made:

  • Fixed all route modules to use correct relative imports:
    • from ...core.config (three levels up from api/routes/)
    • from ..models (one level up to api/models)
  • Fixed AutoRecordingManager import path (was auto_recording.manager, corrected to recording.auto_manager)
  • Added proper type hints to all registration functions

Benefits:

  • 63% reduction in server.py size (868 → 315 lines)
  • Routes organized by domain (easy to find specific endpoints)
  • Easier maintenance (smaller, focused files)
  • Reduced merge conflicts (different developers work on different route modules)
  • Better code organization following separation of concerns
  • Easier to test (can test route modules independently)

Key Design Decisions

Why Keep WebSocket in server.py?

The WebSocket endpoint (/ws) was kept in server.py because:

  • It's tightly coupled with the WebSocketManager class defined in server.py
  • It's core functionality, not a domain-specific feature
  • Moving it would require refactoring the manager class as well

Why Use register_*_routes() Functions?

Each route module exports a function that takes dependencies (app, managers, logger) and registers routes. This pattern:

  • Keeps route modules testable (can pass mock dependencies)
  • Allows server.py to control dependency injection
  • Makes it clear what dependencies each route module needs

Why Not Move Debug Endpoint?

The /debug/camera-manager endpoint could be moved to camera_routes.py, but it was kept in server.py as a utility endpoint for debugging the server's internal state. This is a reasonable design choice for debug utilities.


Verification

All refactoring changes were verified to work correctly:

API Starts Successfully

  • No import errors
  • No syntax errors
  • All route modules load correctly

Endpoints Function Correctly

  • /health - Returns healthy status
  • /system/status - Returns system status with cameras, machines, recordings
  • /cameras - Returns camera status (both cameras now show correct status)
  • All other endpoints maintain functionality

No Regressions

  • Camera monitoring works correctly (camera1 shows "available" status)
  • Constants are properly imported and used
  • Utility functions work as expected

Migration Notes for Future Developers

Adding New Routes

  1. Identify the domain: Which route module does your endpoint belong to?

    • System/health → system_routes.py
    • Camera operations → camera_routes.py
    • Recording → recording_routes.py
    • Storage → storage_routes.py
    • MQTT → mqtt_routes.py
    • Auto-recording → auto_recording_routes.py
    • Recording sessions → recordings_routes.py
  2. Add route to appropriate module: Use the existing pattern:

    @app.get("/your/endpoint")
    async def your_endpoint():
        # Implementation
    
  3. If creating a new domain:

    • Create new file: api/routes/your_domain_routes.py
    • Export register_your_domain_routes() function
    • Add import to api/routes/__init__.py
    • Register in server.py's _setup_routes() method

Using Constants

When you need camera-related constants:

from ...camera.constants import CAMERA_GET_BUFFER_TIMEOUT, PREVIEW_FPS

Using Utility Functions

When you need camera error suppression:

from ...camera.utils import suppress_camera_errors

with suppress_camera_errors():
    # Camera operations

  • CODE_QUALITY_IMPROVEMENTS.md - Original analysis and suggestions
  • REFACTORING_PLAN.md - Step-by-step implementation guide

Lessons Learned

  1. Start Small: The refactoring started with the simplest tasks (extracting duplicates and constants) before tackling the larger route split.

  2. Verify as You Go: Each task was verified before moving to the next, preventing cascading errors.

  3. Fix Imports Systematically: When splitting routes, import paths needed careful correction. Using relative imports requires counting directory levels carefully.

  4. Maintain Type Safety: Added type hints to all route registration functions for better IDE support and error detection.

  5. Test Endpoints: Always test actual API endpoints after refactoring to ensure no functionality was broken.


Future Improvement Opportunities

While not included in this refactoring, potential future improvements:

  1. Move Debug Endpoint: Consider moving /debug/camera-manager to camera_routes.py for better organization
  2. Extract WebSocket Manager: Could move WebSocketManager to a separate module if it grows
  3. Route Unit Tests: Add unit tests for route modules to prevent regressions
  4. API Documentation: Consider adding OpenAPI/Swagger tags to organize routes in API docs
  5. More Constants: Consider extracting more magic numbers as the codebase evolves

Overall Project Status

Frontend (React Dashboard)

Microfrontend architecture implemented

  • video-remote module extracted and working
  • Module Federation configured and tested
  • Feature flags system in place for gradual rollout
  • Foundation ready for extracting additional modules

Backend (Camera Management API)

Monolithic refactoring completed

  • Code organization significantly improved
  • Routes split into domain modules
  • Constants and utilities extracted
  • 63% reduction in main server file size
  • Decision: Maintained monolithic architecture (not split into microservices)
  • All functionality preserved and verified

Summary

Frontend Modularization

Microfrontend architecture established
Video library extracted as first module
Independent deployment pipeline ready
Scalable pattern for future feature extraction

Backend Refactoring

3 refactoring tasks completed successfully
Code quality significantly improved
No functionality broken
63% reduction in main server file size
Better code organization and maintainability
Maintained monolithic architecture (intentional decision)

Overall

Frontend: Microfrontends (independent modules, Module Federation)
Backend: Improved Monolith (better organization, maintainability, no microservices)

The system now has a modular frontend with improved backend code quality, all while maintaining full backward compatibility and system stability.