diff --git a/README.md b/README.md index 12bf6be..003426b 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,10 @@ Implementation of the FarmBot at UAS Technikum Wien. +[![Pylint](https://github.com/TW-Robotics/TWFarmBot/actions/workflows/pylint.yml/badge.svg)](https://github.com/TW-Robotics/TWFarmBot/actions/workflows/pylint.yml) +[![Python 3.10](https://img.shields.io/badge/python-3.10-blue.svg)](https://www.python.org/downloads/release/python-3100/) +[![Issues](https://img.shields.io/github/issues/TW-Robotics/TWFarmBot)](https://github.com/TW-Robotics/TWFarmBot/issues) + This repository is a **monorepo** for the FarmBot system: the physical robot integration, sensor pipelines, irrigation/vision/planning services, the API and worker apps, and the student projects and experiments that build on top of diff --git a/apps/api_server/src/twfarmbot_api_server/app.py b/apps/api_server/src/twfarmbot_api_server/app.py index ea34a9d..5363758 100644 --- a/apps/api_server/src/twfarmbot_api_server/app.py +++ b/apps/api_server/src/twfarmbot_api_server/app.py @@ -158,6 +158,19 @@ def post_action(payload: ActionPayload, wait: bool = True) -> dict[str, Any]: raise HTTPException(status_code=404, detail=str(err)) from err except UnsafeActionError as err: raise HTTPException(status_code=400, detail=str(err)) from err + except FarmBotConnectionError as err: + raise HTTPException( + status_code=502, + detail=f"FarmBot not connected: {err}", + ) from err + except Exception as err: # noqa: BLE001 — surface real cause to the UI + log.exception( + "action failed kind=%s params=%s", action.kind, action.params + ) + raise HTTPException( + status_code=500, + detail=f"{type(err).__name__}: {err}", + ) from err return { "status": "ok", "action": {"kind": executed.kind, "params": executed.params}, diff --git a/apps/ui/src/twfarmbot_ui/app.py b/apps/ui/src/twfarmbot_ui/app.py index 923b58d..ff46384 100644 --- a/apps/ui/src/twfarmbot_ui/app.py +++ b/apps/ui/src/twfarmbot_ui/app.py @@ -25,7 +25,7 @@ from twfarmbot_core.actions import summarize_action -from twfarmbot_ui.client import ApiClient +from twfarmbot_ui.client import ApiClient, ApiResult # ── config ──────────────────────────────────────────────────────────────────── @@ -336,7 +336,7 @@ def _do_move(client: ApiClient, x: float, y: float, z: float, label: str = "") - st.toast(msg, icon="➡️") _refresh_position(client) else: - st.error(f"HTTP {r.code}: {r.body}") + st.error(r.error_message()) def _do_pin_write( @@ -353,7 +353,7 @@ def _do_pin_write( if r.ok: st.toast(f"pin {pin} = {value}", icon="✏️") else: - st.error(f"HTTP {r.code}: {r.body}") + st.error(r.error_message()) def _do_pin_pulse( @@ -370,7 +370,7 @@ def _do_pin_pulse( if r.ok: st.toast(f"pin {pin} HIGH for {seconds}s", icon="✏️") else: - st.error(f"HTTP {r.code}: {r.body}") + st.error(r.error_message()) # ── page shell ──────────────────────────────────────────────────────────────── @@ -579,7 +579,7 @@ def _do_pin_pulse( if r.ok: st.toast("ESTOP sent", icon="🛑") else: - st.error(str(r.body)) + st.error(r.error_message()) # ── tab content ─────────────────────────────────────────────────────────────── @@ -763,7 +763,7 @@ def _render_garden() -> None: result = client.request("GET", "/garden") if not result.ok or not isinstance(result.body, dict): - st.error(f"Garden model unavailable: {result.body}") + st.error(f"Garden model unavailable: {result.error_message()}") return world = result.body @@ -1065,7 +1065,7 @@ def _render_motion() -> None: if r.ok: st.toast("Homing queued") else: - st.error(str(r.body)) + st.error(r.error_message()) # Presets if "presets" not in st.session_state: @@ -1135,7 +1135,7 @@ def _render_io() -> None: if r.ok: st.success("Queued") else: - st.error(str(r.body)) + st.error(r.error_message()) with b: st.markdown("**Peripheral control**") @@ -1190,7 +1190,7 @@ def _render_camera() -> None: if r.ok: st.toast("Capture queued", icon="📷") else: - st.error(str(r.body)) + st.error(r.error_message()) if refresh.button("↻ Refresh gallery", use_container_width=True): st.session_state["images"] = client.request( "GET", "/images", params={"refresh": "true"}, timeout=10.0 @@ -1694,7 +1694,7 @@ def _close_text_segment() -> None: seg[1] = accumulated seg[0].markdown(accumulated) else: - stream_error = f"Fallback failed: HTTP {r.code}: {r.body}" + stream_error = f"Fallback failed: {r.error_message()}" except Exception as exc: # noqa: BLE001 stream_error = f"Fallback failed: {type(exc).__name__}: {exc}" @@ -1906,7 +1906,11 @@ def _render_plan() -> None: st.json(response) if status and status >= 400: - st.error(f"Planner error (HTTP {status}): {response.get('error', response)}") + err_body = response.get("error", response) + st.error( + f"Planner error (HTTP {status}): " + f"{ApiResult(ok=False, code=status, body=err_body).error_message()}" + ) return actions = response.get("actions", []) or [] @@ -1946,7 +1950,7 @@ def _render_plan() -> None: st.toast(f"Queued {action['kind']}", icon="➡️") else: failed += 1 - st.error(f"Failed to queue {action['kind']}: {r.body}") + st.error(f"Failed to queue {action['kind']}: {r.error_message()}") if failed == 0: st.success(f"Plan queued · {queued} action(s)") else: @@ -1968,7 +1972,7 @@ def _render_diagnostics() -> None: if d.ok and isinstance(d.body, dict): st.session_state["diag"] = d.body.get("state", {}) else: - st.error(f"Read failed: {d.body}") + st.error(f"Read failed: {d.error_message()}") payload = st.session_state.get("diag", {}) or {} info = payload.get("informational_settings", {}) or {} diff --git a/apps/ui/src/twfarmbot_ui/client.py b/apps/ui/src/twfarmbot_ui/client.py index 1dd8353..f44da5a 100644 --- a/apps/ui/src/twfarmbot_ui/client.py +++ b/apps/ui/src/twfarmbot_ui/client.py @@ -19,6 +19,22 @@ class ApiResult: code: int body: Any + def error_message(self) -> str: + """Return a human-readable backend error from the response body. + + FastAPI errors are shaped like ``{"detail": "..."}``; legacy or + connection-failure bodies may use ``{"error": "..."}``. Fall back to + the raw body text when neither key is present. + """ + if isinstance(self.body, dict): + if "detail" in self.body: + return str(self.body["detail"]) + if "error" in self.body: + return str(self.body["error"]) + if isinstance(self.body, str): + return self.body + return str(self.body) + class ApiClient: def __init__(self, base_url: str, timeout: float = 2.0) -> None: diff --git a/services/planning_service/planning_service/providers.py b/services/planning_service/planning_service/providers.py index ddabc2c..3f0dc2e 100644 --- a/services/planning_service/planning_service/providers.py +++ b/services/planning_service/planning_service/providers.py @@ -9,6 +9,7 @@ import os from abc import ABC, abstractmethod +from typing import Any import requests from langchain_core.language_models import BaseChatModel @@ -27,7 +28,7 @@ def build_chat_model( self, model: str, config: PlannerConfig ) -> BaseChatModel: """Return a configured LangChain chat model for ``model``.""" - raise NotImplementedError + ... def list_models(self, config: PlannerConfig) -> list[str]: """Return a list of model ids available from this provider.""" diff --git a/tests/test_actions_endpoint.py b/tests/test_actions_endpoint.py new file mode 100644 index 0000000..78bf30e --- /dev/null +++ b/tests/test_actions_endpoint.py @@ -0,0 +1,81 @@ +"""Tests for the POST /actions endpoint.""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from farmbot_client import FarmBotConnectionError +from safety_service import UnsafeActionError +from twfarmbot_api_server.app import create_app +from twfarmbot_core.actions import ActionRegistry +from twfarmbot_core.domain import Action + + +@pytest.fixture +def client() -> TestClient: + registry = ActionRegistry() + registry.register("noop", lambda a: a) + app = create_app(registry=registry) + return TestClient(app) + + +def test_post_action_returns_200_for_valid_action(client: TestClient) -> None: + r = client.post("/actions", json={"kind": "noop", "params": {"foo": "bar"}}) + assert r.status_code == 200 + body = r.json() + assert body["status"] == "ok" + assert body["action"]["kind"] == "noop" + assert body["action"]["params"] == {"foo": "bar"} + + +def test_post_action_returns_404_for_unknown_kind(client: TestClient) -> None: + r = client.post("/actions", json={"kind": "unknown", "params": {}}) + assert r.status_code == 404 + assert "no handler registered" in r.json()["detail"].lower() + + +def test_post_action_returns_400_for_unsafe_action( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + def fake_validate(action: Action) -> None: + raise UnsafeActionError("too dangerous") + + monkeypatch.setattr("twfarmbot_core.actions.safety_validate", fake_validate) + r = client.post("/actions", json={"kind": "noop", "params": {}}) + assert r.status_code == 400 + assert "too dangerous" in r.json()["detail"] + + +def test_post_action_returns_502_on_farmbot_connection_error( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + def explode(action: Action) -> Action: + raise FarmBotConnectionError("broker down") + + monkeypatch.setattr(client.app.state.registry, "dispatch", explode) + r = client.post("/actions", json={"kind": "noop", "params": {}}) + assert r.status_code == 502 + detail = r.json()["detail"] + assert "FarmBot not connected" in detail + assert "broker down" in detail + + +def test_post_action_returns_500_with_real_error_message( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + def explode(action: Action) -> Action: + raise RuntimeError("handler exploded") + + monkeypatch.setattr(client.app.state.registry, "dispatch", explode) + r = client.post("/actions", json={"kind": "noop", "params": {}}) + assert r.status_code == 500 + detail = r.json()["detail"] + assert "RuntimeError" in detail + assert "handler exploded" in detail + + +def test_post_action_non_wait_returns_404_for_unknown_kind(client: TestClient) -> None: + r = client.post("/actions?wait=false", json={"kind": "unknown", "params": {}}) + assert r.status_code == 404 + assert "unknown action kind" in r.json()["detail"].lower() diff --git a/tests/test_harness.py b/tests/test_harness.py index 6363d34..9a4c508 100644 --- a/tests/test_harness.py +++ b/tests/test_harness.py @@ -11,6 +11,7 @@ import json +import pytest from langchain_core.language_models.fake_chat_models import FakeListChatModel from langchain_core.messages import AIMessage, AIMessageChunk from langchain_core.messages.tool import ToolCallChunk @@ -22,6 +23,7 @@ ContextBuilder, ReasoningController, ToolCategory, + ToolDescriptor, ToolPolicy, ToolRegistry, ) diff --git a/tests/test_ui.py b/tests/test_ui.py index 68cd645..4d83c73 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -73,6 +73,34 @@ def test_api_client_strips_trailing_slash() -> None: assert c.base_url == "http://api" +def test_api_result_error_message_prefers_detail() -> None: + from twfarmbot_ui.client import ApiResult + + r = ApiResult(ok=False, code=500, body={"detail": "FarmBot not connected"}) + assert r.error_message() == "FarmBot not connected" + + +def test_api_result_error_message_falls_back_to_error_key() -> None: + from twfarmbot_ui.client import ApiResult + + r = ApiResult(ok=False, code=0, body={"error": "ConnectError: refused"}) + assert r.error_message() == "ConnectError: refused" + + +def test_api_result_error_message_falls_back_to_string_body() -> None: + from twfarmbot_ui.client import ApiResult + + r = ApiResult(ok=False, code=500, body="raw error text") + assert r.error_message() == "raw error text" + + +def test_api_result_error_message_stringifies_unknown_body() -> None: + from twfarmbot_ui.client import ApiResult + + r = ApiResult(ok=False, code=500, body={"nested": ["info"]}) + assert "nested" in r.error_message() + + def test_huggingface_image_processor_calls_gradio_endpoint( monkeypatch: "pytest.MonkeyPatch", ) -> None: