Skip to content

Decouple hardware components into standalone services (#1413)#1413

Open
wtgee wants to merge 28 commits into
developfrom
decouple-hardware-services
Open

Decouple hardware components into standalone services (#1413)#1413
wtgee wants to merge 28 commits into
developfrom
decouple-hardware-services

Conversation

@wtgee

@wtgee wtgee commented Mar 21, 2026

Copy link
Copy Markdown
Member

Description

This PR implements an architectural refactor to decouple the tightly-coupled hardware components (Mount, Cameras, Scheduler) from the central Observatory / POCS monolithic process. Each component can now run as a standalone microservice using FastAPI, communicating with the main POCS instance via the Proxy Pattern.

Key Changes

  • Microservices Layer: Added src/panoptes/pocs/services/ containing FastAPI wrappers for:
    • mount_api.py: Wraps any standard POCS mount (simulator, iOptron, Bisque).
    • camera_api.py: Wraps a collection of cameras (ZWO, Canon, etc.) using existing factory methods.
    • scheduler_api.py: Wraps the configured scheduler (Dispatch/Base).
  • Remote Proxy Classes: Implemented RemoteMount, RemoteCamera, and RemoteScheduler inheriting from the base abstract classes. These classes translate standard method calls into HTTP requests to the respective services.
  • Service Management: Updated conf_files/pocs-supervisord.conf to include sections for the new hardware services.
  • Factory Updates: Enhanced create_mount_from_config, create_cameras_from_config, and create_scheduler_from_config to support an endpoint_url parameter, enabling remote connectivity when configured.

Motivation

This decoupling allows for:

  • Distributing hardware control across multiple physical machines (e.g., one RPi for cameras, one for the mount).
  • Independent scaling and failure recovery for hardware services.
  • Simplified testing of the core state machine using remote-backed simulators.

Compatibility

  • Backward Compatible: The default behavior remains monolithic. These changes are only active if an endpoint_url is provided in the configuration for a specific component.
  • Hardware Support: Tested with Simulator modes; ZWO and Canon cameras are supported remotely through the same camera_api wrapper.

Tests

  • Verified with existing test_pocs.py suite.
  • Linting and formatting checked with ruff.

Closes #1413

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors PANOPTES POCS hardware control to support running Mount, Camera, and Scheduler as standalone FastAPI services, with the main process talking to them via new HTTP-based proxy classes.

Changes:

  • Added FastAPI microservice wrappers for mount, cameras, and scheduler under panoptes.pocs.services.
  • Introduced remote proxy implementations for mount/camera/scheduler that translate method calls into HTTP requests.
  • Updated factories and supervisord configuration to enable endpoint-based remote connectivity, and documented the feature in the changelog.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
src/panoptes/pocs/services/scheduler_api.py New FastAPI scheduler service endpoints.
src/panoptes/pocs/services/mount_api.py New FastAPI mount service endpoints (incl. slew/park ops).
src/panoptes/pocs/services/camera_api.py New FastAPI camera service endpoints (status, exposure, cooling).
src/panoptes/pocs/scheduler/remote.py New scheduler proxy that delegates to a remote scheduler service.
src/panoptes/pocs/scheduler/__init__.py Factory updated to pass endpoint_url when configured.
src/panoptes/pocs/mount/remote.py New mount proxy that delegates to a remote mount service.
src/panoptes/pocs/mount/__init__.py Factory updated to pass endpoint_url when configured.
src/panoptes/pocs/camera/remote.py New camera proxy that delegates to a remote camera service.
src/panoptes/pocs/camera/camera.py Formatting-only adjustments in threading kwargs and arithmetic.
conftest.py Whitespace-only change.
conf_files/pocs-supervisord.conf Added supervisord programs for the new hardware services (ports 8001–8003).
CHANGELOG.md Documented the new decoupled hardware services and remote proxies.
Comments suppressed due to low confidence (1)

src/panoptes/pocs/scheduler/init.py:81

  • This adds support for scheduler.endpoint_url but the existing scheduler factory tests don't exercise the remote scheduler path. Consider adding a test that sets scheduler.type to panoptes.pocs.scheduler.remote with an endpoint_url and verifies the scheduler is constructed correctly (and that the URL is stored/used), using httpx mocking for the remote calls.
            # Pass endpoint_url if this is a remote scheduler
            if "endpoint_url" in scheduler_config:
                kwargs["endpoint_url"] = scheduler_config["endpoint_url"]

            # Create the Scheduler instance
            pocs_scheduler = module.Scheduler(
                observer, fields_file=str(fields_path), constraints=constraints, *args, **kwargs
            )

Comment thread src/panoptes/pocs/services/mount_api.py Outdated
Comment on lines +2 to +68
from fastapi import BackgroundTasks, FastAPI, HTTPException
from pydantic import BaseModel

from panoptes.pocs.mount import create_mount_from_config

app = FastAPI()

# Global mount instance
mount = None


@app.on_event("startup")
async def startup_event():
global mount
try:
mount = create_mount_from_config()
except Exception as e:
print(f"Failed to create mount: {e}")
mount = None


@app.get("/status")
def status():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return mount.status()


@app.post("/connect")
def connect():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.connect()}


@app.post("/initialize")
def initialize():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.initialize()}


@app.post("/disconnect")
def disconnect():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.disconnect()}


@app.get("/is_connected")
def is_connected():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.is_connected}


@app.get("/is_initialized")
def is_initialized():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.is_initialized}


@app.get("/is_parked")
def is_parked():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These services use module-level globals (mount) and @app.on_event("startup") plus print(...) for failures. Elsewhere in the repo (e.g. src/panoptes/pocs/utils/service/weather.py) FastAPI apps use a lifespan context manager and store instances on app.state with proper logging. Aligning with that pattern avoids issues with testability and multi-worker deployments and ensures errors go through the standard logger.

Suggested change
from fastapi import BackgroundTasks, FastAPI, HTTPException
from pydantic import BaseModel
from panoptes.pocs.mount import create_mount_from_config
app = FastAPI()
# Global mount instance
mount = None
@app.on_event("startup")
async def startup_event():
global mount
try:
mount = create_mount_from_config()
except Exception as e:
print(f"Failed to create mount: {e}")
mount = None
@app.get("/status")
def status():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return mount.status()
@app.post("/connect")
def connect():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.connect()}
@app.post("/initialize")
def initialize():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.initialize()}
@app.post("/disconnect")
def disconnect():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.disconnect()}
@app.get("/is_connected")
def is_connected():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.is_connected}
@app.get("/is_initialized")
def is_initialized():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return {"result": mount.is_initialized}
@app.get("/is_parked")
def is_parked():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
from contextlib import asynccontextmanager
import logging
from fastapi import BackgroundTasks, FastAPI, HTTPException, Request
from pydantic import BaseModel
from panoptes.pocs.mount import create_mount_from_config
logger = logging.getLogger(__name__)
@asynccontextmanager
async def lifespan(app: FastAPI):
"""FastAPI lifespan context manager that manages the mount instance."""
try:
app.state.mount = create_mount_from_config()
except Exception:
logger.exception("Failed to create mount from config")
app.state.mount = None
try:
yield
finally:
mount = getattr(app.state, "mount", None)
if mount is not None:
try:
# Attempt a clean disconnect on shutdown.
mount.disconnect()
except Exception:
logger.exception("Error while disconnecting mount during shutdown")
app = FastAPI(lifespan=lifespan)
def get_mount(request: Request):
"""Retrieve the mount instance from the FastAPI application state."""
mount = getattr(request.app.state, "mount", None)
if mount is None:
raise HTTPException(status_code=503, detail="Mount not initialized")
return mount
@app.get("/status")
def status(request: Request):
mount = get_mount(request)
return mount.status()
@app.post("/connect")
def connect(request: Request):
mount = get_mount(request)
return {"result": mount.connect()}
@app.post("/initialize")
def initialize(request: Request):
mount = get_mount(request)
return {"result": mount.initialize()}
@app.post("/disconnect")
def disconnect(request: Request):
mount = get_mount(request)
return {"result": mount.disconnect()}
@app.get("/is_connected")
def is_connected(request: Request):
mount = get_mount(request)
return {"result": mount.is_connected}
@app.get("/is_initialized")
def is_initialized(request: Request):
mount = get_mount(request)
return {"result": mount.is_initialized}
@app.get("/is_parked")
def is_parked(request: Request):
mount = get_mount(request)

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +30
@app.get("/status")
def status():
if not scheduler:
raise HTTPException(status_code=503, detail="Scheduler not initialized")
return {"result": scheduler.status}

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/status returns scheduler.status, but BaseScheduler.status contains live Python objects (constraints instances and a current_observation object) that are not JSON-serializable, so this endpoint will 500 under FastAPI's JSON encoding. Consider returning a JSON-safe schema (e.g., constraint class names/params and current observation name) or a dedicated pydantic response model.

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +23
app = FastAPI()

# Global scheduler instance
scheduler = None


@app.on_event("startup")
async def startup_event():
global scheduler
try:
scheduler = create_scheduler_from_config()
except Exception as e:
print(f"Failed to create scheduler: {e}")
scheduler = None

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module uses a module-level global (scheduler) and @app.on_event("startup") with print(...) on failure. The existing FastAPI services in this repo (e.g. src/panoptes/pocs/utils/service/power.py, weather.py) use a lifespan context manager and store resources on app.state with standard logging. Aligning with that pattern will reduce concurrency surprises and make initialization/shutdown behavior consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +48
@app.get("/{name}/status")
def status(name: str):
cam = get_camera(name)
return {
"uid": cam.uid,
"is_connected": cam.is_connected,
"is_exposing": cam.is_exposing,
"is_ready": cam.is_ready,
"temperature": cam.temperature,
"target_temperature": cam.target_temperature,
"cooling_enabled": cam.cooling_enabled,
"cooling_power": cam.cooling_power,
}

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several fields returned by this endpoint (e.g. temperature, target_temperature, cooling_power) are often astropy.units.Quantity objects (e.g. simulator/FLI/ZWO cameras) and are not JSON-serializable. This will break the API for common camera implementations. Convert quantities to plain values (and optionally include units) before returning.

Copilot uses AI. Check for mistakes.
Comment on lines +58 to +79
@app.get("/{name}/properties")
def properties(name: str):
cam = get_camera(name)
return {
"uid": cam.uid,
"is_connected": cam.is_connected,
"readout_time": cam.readout_time.value if hasattr(cam.readout_time, "value") else cam.readout_time,
"file_extension": cam.file_extension,
"egain": cam.egain,
"bit_depth": cam.bit_depth,
"temperature": cam.temperature,
"target_temperature": cam.target_temperature,
"temperature_tolerance": cam.temperature_tolerance,
"cooling_enabled": cam.cooling_enabled,
"cooling_power": cam.cooling_power,
"filter_type": cam.filter_type,
"is_cooled_camera": cam.is_cooled_camera,
"is_temperature_stable": cam.is_temperature_stable,
"is_exposing": cam.is_exposing,
"is_ready": cam.is_ready,
"can_take_internal_darks": cam.can_take_internal_darks,
}

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to /{name}/status, this endpoint returns values that are commonly astropy.units.Quantity (e.g. temperature, target_temperature, temperature_tolerance, cooling_power, egain, bit_depth) without normalization, which will fail JSON encoding. It would be safer to normalize all Quantity-like values consistently (as you already do for readout_time).

Copilot uses AI. Check for mistakes.
Comment thread src/panoptes/pocs/scheduler/remote.py Outdated
Comment on lines +80 to +93
super().clear_available_observations()
self._post("clear_available_observations")

def reset_observed_list(self):
super().reset_observed_list()
self._post("reset_observed_list")

def add_observation(self, observation_config: dict, **kwargs):
super().add_observation(observation_config, **kwargs)
self._post("add_observation", json={"observation_config": observation_config})

def remove_observation(self, field_name):
super().remove_observation(field_name)
self._post("remove_observation", params={"field_name": field_name})

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local scheduler state is mutated via super().add_observation(...) before the remote call. If the remote POST fails (your _post returns None on error), the local and remote scheduler states will diverge, and subsequent remote scheduling decisions won't match local bookkeeping. Consider making the remote call first and only updating local state on success, or raising on remote failure so callers can handle it.

Suggested change
super().clear_available_observations()
self._post("clear_available_observations")
def reset_observed_list(self):
super().reset_observed_list()
self._post("reset_observed_list")
def add_observation(self, observation_config: dict, **kwargs):
super().add_observation(observation_config, **kwargs)
self._post("add_observation", json={"observation_config": observation_config})
def remove_observation(self, field_name):
super().remove_observation(field_name)
self._post("remove_observation", params={"field_name": field_name})
"""Clear available observations on both remote and local schedulers.
The remote scheduler is updated first; the local state is only cleared if the
remote call succeeds, to avoid divergence between local and remote state.
"""
result = self._post("clear_available_observations")
if not result:
self.logger.error("Failed to clear available observations on remote scheduler")
raise RuntimeError("Remote scheduler clear_available_observations failed")
super().clear_available_observations()
def reset_observed_list(self):
"""Reset the observed list on both remote and local schedulers.
The remote scheduler is updated first; the local state is only reset if the
remote call succeeds, to avoid divergence between local and remote state.
"""
result = self._post("reset_observed_list")
if not result:
self.logger.error("Failed to reset observed list on remote scheduler")
raise RuntimeError("Remote scheduler reset_observed_list failed")
super().reset_observed_list()
def add_observation(self, observation_config: dict, **kwargs):
"""Add an observation to both remote and local schedulers.
The observation is first sent to the remote scheduler; the local scheduler
is only updated if the remote call succeeds, to keep their states in sync.
"""
result = self._post("add_observation", json={"observation_config": observation_config})
if not result:
self.logger.error("Failed to add observation on remote scheduler")
raise RuntimeError("Remote scheduler add_observation failed")
super().add_observation(observation_config, **kwargs)
def remove_observation(self, field_name):
"""Remove an observation from both remote and local schedulers.
The observation is first removed from the remote scheduler; the local
scheduler is only updated if the remote call succeeds, to keep their
states in sync.
"""
result = self._post("remove_observation", params={"field_name": field_name})
if not result:
self.logger.error("Failed to remove observation on remote scheduler")
raise RuntimeError("Remote scheduler remove_observation failed")
super().remove_observation(field_name)

Copilot uses AI. Check for mistakes.
Comment thread src/panoptes/pocs/camera/remote.py Outdated
Comment on lines +28 to +32
def _get(self, path):
try:
response = self.client.get(f"{self.endpoint_url}/{self.name}/{path}")
response.raise_for_status()
if "result" in response.json():

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request path is built using self.name (.../{self.name}/{path}), but camera names can contain spaces (the default here is "Remote Camera"). That will lead to invalid/unmatched routes (e.g. /Remote Camera/status) unless callers URL-encode exactly the same way as the server expects. Prefer using a URL-safe identifier (e.g. uid) and/or URL-encode the path segment consistently on both client and server.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +3
import httpx
from astropy.coordinates import SkyCoord

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

httpx is imported and used directly here. It doesn't appear to be declared as a direct dependency in pyproject.toml, so installations that don't include it transitively may break at import time. Add httpx as an explicit project dependency (or move the import behind an optional extra with a clear error).

Copilot uses AI. Check for mistakes.
Comment thread src/panoptes/pocs/services/mount_api.py Outdated
def status():
if not mount:
raise HTTPException(status_code=503, detail="Mount not initialized")
return mount.status()

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractMount.status is a @property that returns a dict. Calling mount.status() will raise TypeError: 'dict' object is not callable for standard mounts. Return mount.status (and optionally wrap it in a {"result": ...} envelope for consistency with other endpoints).

Suggested change
return mount.status()
return {"result": mount.status}

Copilot uses AI. Check for mistakes.
Comment on lines +98 to 104
# If it's a remote mount, pass the endpoint_url
if "endpoint_url" in mount_info:
kwargs["endpoint_url"] = mount_info["endpoint_url"]

# Make the mount include site information
mount = module.Mount(location=earth_location, *args, **kwargs)

Copilot AI Mar 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces a new config-driven code path (endpoint_url) that alters mount construction, but there are existing tests for create_mount_from_config that don't cover the remote case. Please add a focused test that sets mount.endpoint_url in config and asserts the factory passes it through to the driver (e.g. using the new panoptes.pocs.mount.remote driver with mocked HTTP).

Copilot uses AI. Check for mistakes.
@codecov

codecov Bot commented Mar 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 63.88889% with 143 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.28%. Comparing base (0ada85a) to head (4fbca26).

Files with missing lines Patch % Lines
src/panoptes/pocs/mount/remote.py 50.44% 54 Missing and 2 partials ⚠️
src/panoptes/pocs/camera/remote.py 53.68% 43 Missing and 1 partial ⚠️
src/panoptes/pocs/scheduler/remote.py 53.94% 30 Missing and 5 partials ⚠️
src/panoptes/pocs/mount/__init__.py 61.90% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1413      +/-   ##
==========================================
+ Coverage   66.17%   66.28%   +0.10%     
==========================================
  Files         106      110       +4     
  Lines        9639    10019     +380     
  Branches      847      872      +25     
==========================================
+ Hits         6379     6641     +262     
- Misses       3114     3225     +111     
- Partials      146      153       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wtgee wtgee changed the title Decouple hardware components into standalone services (#1420) Decouple hardware components into standalone services (#1413) Mar 21, 2026
@wtgee wtgee force-pushed the decouple-hardware-services branch from 5803287 to d88a3f2 Compare March 21, 2026 02:56
@wtgee wtgee force-pushed the decouple-hardware-services branch from f0a9f94 to 273b1d7 Compare March 21, 2026 17:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 20 changed files in this pull request and generated 14 comments.

Comment on lines +11 to +28
def __init__(self, location, endpoint_url="http://localhost:8001", *args, **kwargs):
# We don't necessarily need the commands dict here since the real mount
# on the remote end has it, but AbstractMount requires commands in its
# _setup_commands. Let's let the parent initialize.
# We might need to mock commands or just pass an empty dict.
kwargs.setdefault("commands", {})
super().__init__(location, *args, **kwargs)
self.endpoint_url = endpoint_url
self.client = httpx.Client(timeout=300.0) # Long timeout for blocking slews
self.logger.info(f"Initialized RemoteMount pointing to {self.endpoint_url}")

def _get(self, path):
try:
response = self.client.get(f"{self.endpoint_url}/{path}")
response.raise_for_status()
data = from_json(response.text)
if isinstance(data, dict) and "result" in data:
return data["result"]

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

httpx.Client is created and stored on the instance but never closed. Since these proxy objects are long-lived, this can leak connections/sockets over time and can produce ResourceWarnings in tests. Add a close() method (and/or __enter__/__exit__) that calls self.client.close(), and ensure callers/service shutdown paths invoke it.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +31
super().__init__(name=name, model=model, port=port, primary=primary, *args, **kwargs)
self.endpoint_url = endpoint_url
self.client = httpx.Client(timeout=300.0)
# URL-safe camera name for routing
self.safe_name = quote(self.name)
self.logger.info(f"Initialized RemoteCamera {name} pointing to {self.endpoint_url}")

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

httpx.Client is created but never closed. Add a close()/context-manager and ensure it’s called when the camera is released/shutdown to prevent connection leakage.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +6
from contextlib import asynccontextmanager

from astropy.coordinates import SkyCoord
from fastapi import BackgroundTasks, FastAPI, HTTPException, Request
from pydantic import BaseModel

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

panoptes.pocs.services.* won’t be packaged/installed because src/panoptes/pocs/services/ has no __init__.py and pyproject.toml uses setuptools.packages.find with namespaces = false. This will make uvicorn panoptes.pocs.services.mount_api:app fail at runtime. Add src/panoptes/pocs/services/__init__.py (or switch package discovery to include namespace packages).

Copilot uses AI. Check for mistakes.
@app.post("/disconnect")
def disconnect(request: Request):
mount = get_mount(request)
return {"result": mount.disconnect()}

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mount.disconnect() returns None for some mount implementations (e.g., the base AbstractMount.disconnect), so this endpoint can respond with {"result": null}. For consistency with the other endpoints (and to make proxy behavior predictable), return an explicit boolean (e.g., True on successful call, or False/HTTP error on failure).

Suggested change
return {"result": mount.disconnect()}
disconnect_result = mount.disconnect()
if disconnect_result is None:
# Some mount implementations return None; infer success from connection state.
result = not mount.is_connected
else:
result = bool(disconnect_result)
return {"result": result}

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +18
logger = get_logger()


import asyncio

@asynccontextmanager
async def lifespan(app: FastAPI):

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import asyncio appears after logger = ..., which will trigger Ruff E402/I001. Move asyncio up with the other standard-library imports at the top of the file.

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +16
logger = get_logger()


import asyncio

@asynccontextmanager

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import asyncio is placed after non-import statements (logger = ...), which violates Ruff’s import rules (E402/I001) and will fail lint. Move all standard-library imports (including asyncio) to the top of the module in the correct section order.

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +18
logger = get_logger()


import asyncio

@asynccontextmanager
async def lifespan(app: FastAPI):

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import asyncio is not at the top of the module (comes after logger = ...), which will fail Ruff E402/I001. Move it into the standard-library import section at the top.

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +9
from astroplan import Observer
import astropy.units as u

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import order doesn’t follow the project’s Ruff/isort configuration (standard library → third-party → first-party). In particular import astropy.units as u is out of order relative to the other third-party imports, which will fail lint (I001). Run ruff format / ruff check --fix or reorder the imports manually.

Suggested change
from astroplan import Observer
import astropy.units as u
import astropy.units as u
from astroplan import Observer

Copilot uses AI. Check for mistakes.
device_config.update(cfg)

cam_name = device_config.setdefault("name", f"Cam{cam_num:02d}")

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is trailing whitespace on the blank line after cam_name = ... (Ruff W291). This will fail lint with the current Ruff configuration; remove the extra spaces.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +76 to +105
[program:pocs-mount-service]
priority=10
user=panoptes
directory=/home/panoptes
command=uvicorn --host 0.0.0.0 --port 8001 panoptes.pocs.services.mount_api:app
redirect_stderr=true
stdout_logfile=/home/panoptes/logs/mount-service.log
autostart=true
startsecs=5
stopasgroup=true
killasgroup=true

[program:pocs-camera-service]
priority=11
user=panoptes
directory=/home/panoptes
command=uvicorn --host 0.0.0.0 --port 8002 panoptes.pocs.services.camera_api:app
redirect_stderr=true
stdout_logfile=/home/panoptes/logs/camera-service.log
autostart=true
startsecs=5
stopasgroup=true
killasgroup=true

[program:pocs-scheduler-service]
priority=12
user=panoptes
directory=/home/panoptes
command=uvicorn --host 0.0.0.0 --port 8003 panoptes.pocs.services.scheduler_api:app
redirect_stderr=true

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new hardware service processes are configured to bind uvicorn to 0.0.0.0 on ports 8001–8003, but the APIs expose privileged hardware controls (slew/park/take_exposure) without any authentication. Unless these hosts are always firewalled, this is a security risk. Consider binding to 127.0.0.1 by default and/or adding an auth mechanism (e.g., shared token) and documenting required network restrictions.

Copilot uses AI. Check for mistakes.
@wtgee wtgee changed the base branch from main to develop March 30, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants