From 8f566e80c15a6d13dd3526f8ab96c46f9708fd3b Mon Sep 17 00:00:00 2001 From: James Harton Date: Tue, 30 Dec 2025 19:47:11 +1300 Subject: [PATCH] improvement: use structured errors in MoveTo command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace tuple-based error returns with structured BB.Error types: - `{:missing_parameter, key}` → `BB.Error.Invalid.Command` - `{:ik_failed, ...}` → `BB.Error.Kinematics.MultiFailed` This provides consistent error handling with severity levels and better error messages across the codebase. --- lib/bb/command/move_to.ex | 25 ++++++++++++++--- lib/bb/error/kinematics/multi_failed.ex | 36 +++++++++++++++++++++++++ test/bb/command/move_to_test.exs | 16 ++++++++--- 3 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 lib/bb/error/kinematics/multi_failed.ex diff --git a/lib/bb/command/move_to.ex b/lib/bb/command/move_to.ex index c5a6f91..29a16b7 100644 --- a/lib/bb/command/move_to.ex +++ b/lib/bb/command/move_to.ex @@ -82,6 +82,8 @@ defmodule BB.Command.MoveTo do """ @behaviour BB.Command + alias BB.Error.Invalid.Command, as: InvalidCommand + alias BB.Error.Kinematics.MultiFailed alias BB.Math.Vec3 alias BB.Message.Geometry.Point3D alias BB.Motion @@ -94,7 +96,14 @@ defmodule BB.Command.MoveTo do def handle_command(goal, context) when is_map_key(goal, :target), do: handle_single_target(goal, context) - def handle_command(_goal, _context), do: {:error, {:missing_parameter, :target_or_targets}} + def handle_command(_goal, _context) do + {:error, + %InvalidCommand{ + command: __MODULE__, + argument: :target_or_targets, + reason: "required: must specify either :target or :targets" + }} + end defp handle_single_target(goal, context) do with {:ok, target} <- fetch_required(goal, :target), @@ -123,15 +132,23 @@ defmodule BB.Command.MoveTo do {:ok, results} {:error, failed_link, error, results} -> - {:error, {:ik_failed, failed_link, error, results}} + {:error, + %MultiFailed{ + failed_link: failed_link, + error: error, + partial_results: results + }} end end end defp fetch_required(goal, key) do case Map.fetch(goal, key) do - {:ok, value} -> {:ok, value} - :error -> {:error, {:missing_parameter, key}} + {:ok, value} -> + {:ok, value} + + :error -> + {:error, %InvalidCommand{command: __MODULE__, argument: key, reason: "required"}} end end diff --git a/lib/bb/error/kinematics/multi_failed.ex b/lib/bb/error/kinematics/multi_failed.ex new file mode 100644 index 0000000..920955b --- /dev/null +++ b/lib/bb/error/kinematics/multi_failed.ex @@ -0,0 +1,36 @@ +# SPDX-FileCopyrightText: 2025 James Harton +# +# SPDX-License-Identifier: Apache-2.0 + +defmodule BB.Error.Kinematics.MultiFailed do + @moduledoc """ + Multi-target inverse kinematics failed. + + Raised when a multi-target IK operation fails for one or more targets. + Contains the link that failed, the underlying error, and partial results + from any successful targets. + """ + use BB.Error, + class: :kinematics, + fields: [:failed_link, :error, :partial_results] + + @type t :: %__MODULE__{ + failed_link: atom(), + error: term(), + partial_results: map() + } + + defimpl BB.Error.Severity do + def severity(_), do: :error + end + + def message(%{failed_link: link, error: error, partial_results: results}) do + successful_count = map_size(results) + + "Multi-target IK failed for #{inspect(link)}: #{format_error(error)}. " <> + "#{successful_count} target(s) solved before failure." + end + + defp format_error(%{__struct__: _} = error), do: Exception.message(error) + defp format_error(error), do: inspect(error) +end diff --git a/test/bb/command/move_to_test.exs b/test/bb/command/move_to_test.exs index d8e78f4..67d4c30 100644 --- a/test/bb/command/move_to_test.exs +++ b/test/bb/command/move_to_test.exs @@ -7,6 +7,7 @@ defmodule BB.Command.MoveToTest do alias BB.Command.Context alias BB.Command.MoveTo + alias BB.Error.Invalid.Command, as: InvalidCommand alias BB.Error.Kinematics.Unreachable alias BB.Math.Vec3 alias BB.Robot.State, as: RobotState @@ -64,7 +65,8 @@ defmodule BB.Command.MoveToTest do goal = %{target: {0.3, 0.0, 0.0}, solver: MockSolver} - assert {:error, {:missing_parameter, :target_link}} = MoveTo.handle_command(goal, context) + assert {:error, %InvalidCommand{argument: :target_link, reason: "required"}} = + MoveTo.handle_command(goal, context) end test "returns error when solver is missing in single-target mode" do @@ -80,7 +82,8 @@ defmodule BB.Command.MoveToTest do goal = %{target: Vec3.new(0.3, 0.0, 0.0), target_link: :tip} - assert {:error, {:missing_parameter, :solver}} = MoveTo.handle_command(goal, context) + assert {:error, %InvalidCommand{argument: :solver, reason: "required"}} = + MoveTo.handle_command(goal, context) end test "returns ok with metadata on success" do @@ -219,7 +222,8 @@ defmodule BB.Command.MoveToTest do goal = %{targets: %{tip: {0.3, 0.0, 0.0}}} - assert {:error, {:missing_parameter, :solver}} = MoveTo.handle_command(goal, context) + assert {:error, %InvalidCommand{argument: :solver, reason: "required"}} = + MoveTo.handle_command(goal, context) end test "returns error when neither target nor targets provided" do @@ -235,7 +239,11 @@ defmodule BB.Command.MoveToTest do goal = %{solver: MockSolver} - assert {:error, {:missing_parameter, :target_or_targets}} = + assert {:error, + %InvalidCommand{ + argument: :target_or_targets, + reason: "required: must specify either :target or :targets" + }} = MoveTo.handle_command(goal, context) end end