From d4dcb0da8a6cb7b4bfb98a1fa0b1a5aaccab9257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 11:23:50 +0100 Subject: [PATCH 1/7] chore: add ms suffix to time variables and move test wait time to test utils --- lib/elevator.ex | 19 ++++++------------- lib/elevator/communicator.ex | 4 ++-- test/multi/cab_orders_test.exs | 11 ++++++----- test/multi/hall_orders_test.exs | 5 +++-- test/utils/test_compiled.ex | 2 ++ 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index d94d335..37a60a9 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -1,24 +1,17 @@ defmodule Elevator do @num_floors Application.compile_env(:elevator, :num_floors, 4) - # ms - @resend_period 50 - @msg_ts_cutoff 10000 - @test_convergence_wait_time 3 * @resend_period - + @resend_period_ms 50 + @msg_cutoff_ms 10000 def num_floors do @num_floors end - def resend_period do - @resend_period - end - - def msg_ts_cutoff do - @msg_ts_cutoff + def resend_period_ms do + @resend_period_ms end - def test_convergence_wait_time do - @test_convergence_wait_time + def msg_cutoff_ms do + @msg_cutoff_ms end def time_to_serve_executable do diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index bec8baa..93a44c6 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -76,7 +76,7 @@ defmodule Elevator.Communicator do # Schedules another round of state broadcasting. defp schedule_state_broadcast do - time_ms = Elevator.resend_period() + time_ms = Elevator.resend_period_ms() Process.send_after(self(), :broadcast_state, time_ms) end @@ -128,7 +128,7 @@ defmodule Elevator.Communicator do end def handle_call(:who_can_serve, _from, state) do - cutoff_ms = Elevator.msg_ts_cutoff() + cutoff_ms = Elevator.msg_cutoff_ms() communicating_nodes = state.connected_nodes diff --git a/test/multi/cab_orders_test.exs b/test/multi/cab_orders_test.exs index ed4ec73..2bee631 100644 --- a/test/multi/cab_orders_test.exs +++ b/test/multi/cab_orders_test.exs @@ -1,6 +1,7 @@ defmodule Test.Multi.CabOrdersTest do alias Elevator.CabOrders alias Test.Utils.MultiCluster + alias Test.Utils.TestCompiled, as: TestUtils use ExUnit.Case, async: false @@ -38,7 +39,7 @@ defmodule Test.Multi.CabOrdersTest do %{node1 => %{version: 420, orders: MapSet.new([3])}} ]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) node1_orders = :rpc.call(node1, CabOrders, :get_my_orders, []) assert node1_orders == MapSet.new([3]) @@ -56,7 +57,7 @@ defmodule Test.Multi.CabOrdersTest do %{node1 => %{version: 69, orders: MapSet.new([1])}} ]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) %{version: node1_version, orders: node1_orders} = :rpc.call(node1, CabOrders, :get_state, [])[node1] @@ -69,7 +70,7 @@ defmodule Test.Multi.CabOrdersTest do %{node1 => %{version: 67, orders: MapSet.new([1, 2])}} ]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) %{version: node1_version, orders: node1_orders} = :rpc.call(node1, CabOrders, :get_state, [])[node1] @@ -82,7 +83,7 @@ defmodule Test.Multi.CabOrdersTest do %{node1 => %{version: 69, orders: MapSet.new([1, 2])}} ]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) %{version: node1_version, orders: node1_orders} = :rpc.call(node1, CabOrders, :get_state, [])[node1] @@ -107,7 +108,7 @@ defmodule Test.Multi.CabOrdersTest do node3_orders == MapSet.new() :rpc.cast(node1, CabOrders, :button_press, [1]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) # Ensure that node1's version and order map has propagated across all nodes %{version: node1_version, orders: node1_orders} = diff --git a/test/multi/hall_orders_test.exs b/test/multi/hall_orders_test.exs index 982e528..785eb95 100644 --- a/test/multi/hall_orders_test.exs +++ b/test/multi/hall_orders_test.exs @@ -2,6 +2,7 @@ defmodule Test.Multi.HallOrders do alias Elevator.HallOrders alias Elevator.Communicator alias Test.Utils.MultiCluster + alias Test.Utils.TestCompiled, as: TestUtils use ExUnit.Case, async: false setup context do @@ -114,7 +115,7 @@ defmodule Test.Multi.HallOrders do test "communicator causes convergence", %{nodes: [node1, node2, node3]} do :rpc.call(node1, HallOrders, :button_press, [2, :hall_up]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) node1_orders = :rpc.call(node1, HallOrders, :get_my_orders, []) node2_orders = :rpc.call(node2, HallOrders, :get_my_orders, []) @@ -136,7 +137,7 @@ defmodule Test.Multi.HallOrders do :rpc.call(who_arrives, HallOrders, :arrived_at_floor, [2, :up]) - Process.sleep(Elevator.test_convergence_wait_time()) + Process.sleep(TestUtils.convergence_wait_ms()) node1_state = :rpc.call(node1, HallOrders, :get_state, []) node2_state = :rpc.call(node2, HallOrders, :get_state, []) diff --git a/test/utils/test_compiled.ex b/test/utils/test_compiled.ex index 62c7da4..503f51c 100644 --- a/test/utils/test_compiled.ex +++ b/test/utils/test_compiled.ex @@ -3,6 +3,8 @@ defmodule Test.Utils.TestCompiled do This module exists because to run :rpc-calls, the called code has to be compiled, not .exs. So put code here if it is the endpoint of an RPC for testing that is not suitable for lib/. """ + def convergence_wait_ms, do: 3 * Elevator.resend_period_ms() + def start_order_modules(num_floors, do_resend) do children = [ {Elevator.Communicator, [do_resend: do_resend]}, From 6ba3e0d3934376e7d484c4f7063480d3aae59a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 12:03:44 +0100 Subject: [PATCH 2/7] chore: remove dead code and improve variable naming --- lib/elevator/communicator.ex | 8 ++------ lib/elevator/fsm/action.ex | 19 ++++++++----------- lib/elevator/fsm/state.ex | 11 +++-------- lib/elevator/hardware/driver.ex | 10 +++++----- lib/elevator/hardware/input_poller.ex | 18 ++++++++---------- lib/elevator/hardware/outputs.ex | 5 +++-- 6 files changed, 29 insertions(+), 42 deletions(-) diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 93a44c6..4723d5e 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -116,17 +116,13 @@ defmodule Elevator.Communicator do def handle_info(:log_debug, state) do Process.send_after(self(), :log_debug, 1000) Logger.debug("My id: #{my_id()}") - others = who_can_serve() |> Enum.map(fn x -> "#{x}" end) |> Enum.join(", ") + others = who_can_serve() |> Enum.map(fn node -> "#{node}" end) |> Enum.join(", ") Logger.debug("Others: #{others}") {:noreply, state} end # --- Handle calls --- - def handle_call(:self, _, state) do - {:reply, my_id(), state} - end - def handle_call(:who_can_serve, _from, state) do cutoff_ms = Elevator.msg_cutoff_ms() @@ -165,7 +161,7 @@ defmodule Elevator.Communicator do {:noreply, new_state} end - @spec handle_cast({:update_operation_status, boolean()}, state_t()) :: state_t() + @spec handle_cast({:update_operation_status, boolean()}, state_t()) :: {:noreply, state_t()} def handle_cast({:update_operation_status, status}, state) do {:noreply, %{state | operational: status}} end diff --git a/lib/elevator/fsm/action.ex b/lib/elevator/fsm/action.ex index c9dfe27..6e694d1 100644 --- a/lib/elevator/fsm/action.ex +++ b/lib/elevator/fsm/action.ex @@ -9,9 +9,9 @@ defmodule Elevator.FSM.Action do alias Elevator.HallOrders alias Elevator.Decision - @door_open_time 1000 - @motor_timeout 4000 - @action_interval 100 + @door_open_time_ms 1000 + @motor_timeout_ms 4000 + @action_interval_ms 100 def start_link(_arg) do pid = spawn_link(fn -> poll_action() end) @@ -31,9 +31,9 @@ defmodule Elevator.FSM.Action do defp poll_action() do poll_door_timer() - check_motor_timeout() + check_motor_timeout_ms() decide_and_take_action() - Process.sleep(@action_interval) + Process.sleep(@action_interval_ms) poll_action() end @@ -45,7 +45,7 @@ defmodule Elevator.FSM.Action do Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) end - defp check_motor_timeout() do + defp check_motor_timeout_ms() do state = State.get_state() timed_out = @@ -54,7 +54,7 @@ defmodule Elevator.FSM.Action do false last_floor_time -> - Time.diff(Time.utc_now(), last_floor_time, :millisecond) > @motor_timeout + Time.diff(Time.utc_now(), last_floor_time, :millisecond) > @motor_timeout_ms end State.set_motor_timed_out(timed_out) @@ -71,9 +71,6 @@ defmodule Elevator.FSM.Action do {new_direction, new_behavior} = Decision.next_action(orders, state) - # Logger.debug("Deciding on behavior from state:\n #{inspect(state)}\n Orders: #{inspect(orders)}") - # Logger.debug("Got behavior #{new_direction} and #{new_behavior}") - cond do state.behavior == :door_open -> CabOrders.arrived_at_floor(state.floor) @@ -100,7 +97,7 @@ defmodule Elevator.FSM.Action do if state.behavior == :door_open and Time.after?( Time.utc_now(), - Time.add(state.door_open_time, @door_open_time, :millisecond) + Time.add(state.door_open_time_ms, @door_open_time_ms, :millisecond) ) do if state.obstructed do State.open_door() diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 1caeb71..1ec404d 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -12,7 +12,7 @@ defmodule Elevator.FSM.State do between_floors: true, obstructed: false, motor_timed_out: false, - door_open_time: Time.utc_now(), + door_open_time_ms: Time.utc_now(), last_floor_time: nil @type t :: %__MODULE__{ @@ -22,7 +22,7 @@ defmodule Elevator.FSM.State do between_floors: boolean(), obstructed: boolean(), motor_timed_out: boolean(), - door_open_time: Time.t(), + door_open_time_ms: Time.t(), last_floor_time: Time.t() | nil } @@ -86,18 +86,13 @@ defmodule Elevator.FSM.State do {:noreply, %{state | behavior: behavior}, {:continue, :set_outputs}} end - @impl true - def handle_cast({:set_door_open_time, door_open_time}, state) do - {:noreply, %{state | door_open_time: door_open_time}, {:continue, :set_outputs}} - end - @impl true def handle_cast(:open_door, state) do new_state = if state.between_floors do state else - %{state | behavior: :door_open, door_open_time: Time.utc_now()} + %{state | behavior: :door_open, door_open_time_ms: Time.utc_now()} end {:noreply, new_state, {:continue, :set_outputs}} diff --git a/lib/elevator/hardware/driver.ex b/lib/elevator/hardware/driver.ex index 3d10f19..7b1943a 100644 --- a/lib/elevator/hardware/driver.ex +++ b/lib/elevator/hardware/driver.ex @@ -1,6 +1,6 @@ defmodule Elevator.Hardware.Driver do use GenServer - @call_timeout 1000 + @call_timeout_ms 1000 @button_map %{:hall_up => 0, :hall_down => 1, :cab => 2} @state_map %{:on => 1, :off => 0} @direction_map %{:up => 1, :down => 255, :stop => 0} @@ -176,7 +176,7 @@ defmodule Elevator.Hardware.Driver do :gen_tcp.send(socket, [6, @button_map[order_type], floor, 0]) button_state = - case :gen_tcp.recv(socket, 4, @call_timeout) do + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do {:ok, [6, 0, 0, 0]} -> :inactive {:ok, [6, 1, 0, 0]} -> :active end @@ -189,7 +189,7 @@ defmodule Elevator.Hardware.Driver do :gen_tcp.send(socket, [7, 0, 0, 0]) floor_state = - case :gen_tcp.recv(socket, 4, @call_timeout) do + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do {:ok, [7, 0, _, 0]} -> :between_floors {:ok, [7, 1, floor, 0]} -> floor end @@ -202,7 +202,7 @@ defmodule Elevator.Hardware.Driver do :gen_tcp.send(socket, [8, 0, 0, 0]) stop_state = - case :gen_tcp.recv(socket, 4, @call_timeout) do + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do {:ok, [8, 0, 0, 0]} -> :inactive {:ok, [8, 1, 0, 0]} -> :active end @@ -215,7 +215,7 @@ defmodule Elevator.Hardware.Driver do :gen_tcp.send(socket, [9, 0, 0, 0]) obstruction_state = - case :gen_tcp.recv(socket, 4, @call_timeout) do + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do {:ok, [9, 0, 0, 0]} -> :inactive {:ok, [9, 1, 0, 0]} -> :active end diff --git a/lib/elevator/hardware/input_poller.ex b/lib/elevator/hardware/input_poller.ex index 04bece5..d1ee06f 100644 --- a/lib/elevator/hardware/input_poller.ex +++ b/lib/elevator/hardware/input_poller.ex @@ -10,9 +10,9 @@ defmodule Elevator.Hardware.InputPoller do alias Elevator.HallOrders alias Elevator.Hardware.Driver - @floor_poll_interval 50 - @button_poll_interval 20 - @obstruction_poll_interval 500 + @floor_poll_interval_ms 50 + @button_poll_interval_ms 20 + @obstruction_poll_interval_ms 500 # Public API def start_link(_opts) do @@ -53,9 +53,7 @@ defmodule Elevator.Hardware.InputPoller do @impl true def handle_info(:poll_buttons, state) do schedule_button_poll() - # Polls button and notifies State if any are pressed - - prev_buttons = Map.get(state, :prev_buttons, MapSet.new()) + # Polls button and notifies Cab- and HallOrders if any are pressed current_buttons = for floor <- 0..(Elevator.num_floors() - 1), @@ -65,7 +63,7 @@ defmodule Elevator.Hardware.InputPoller do end # Only notify on new presses (in current but not in previous) - new_presses = MapSet.difference(current_buttons, prev_buttons) + new_presses = MapSet.difference(current_buttons, state.prev_buttons) Enum.each(new_presses, fn {floor, btn} -> case btn do @@ -91,14 +89,14 @@ defmodule Elevator.Hardware.InputPoller do # Schedule functions defp schedule_button_poll do - Process.send_after(self(), :poll_buttons, @button_poll_interval) + Process.send_after(self(), :poll_buttons, @button_poll_interval_ms) end defp schedule_floor_poll do - Process.send_after(self(), :poll_floor, @floor_poll_interval) + Process.send_after(self(), :poll_floor, @floor_poll_interval_ms) end defp schedule_obstruction_poll do - Process.send_after(self(), :poll_obstruction, @obstruction_poll_interval) + Process.send_after(self(), :poll_obstruction, @obstruction_poll_interval_ms) end end diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index 5af9b34..d8280e2 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -24,8 +24,9 @@ defmodule Elevator.Hardware.Outputs do set_motors(state) set_floor_light(state) - operational? = !((state.behavior == :door_open and state.obstructed) or state.motor_timed_out) - Communicator.update_operation_status(operational?) + door_blocked = state.behavior == :door_open and state.obstructed + operational = not (door_blocked or state.motor_timed_out) + Communicator.update_operation_status(operational) Task.start(fn -> orders = get_light_orders() From 17d2ef4b5ded7fc440900ab97cb7f435dce7cf85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 13:13:12 +0100 Subject: [PATCH 3/7] chore: impl true + cleanup --- lib/elevator/cab_orders.ex | 7 ++++++- lib/elevator/communicator.ex | 13 +++++++++---- lib/elevator/fsm/action.ex | 4 ++-- lib/elevator/hall_orders.ex | 14 ++++++-------- lib/elevator/hall_orders/cost.ex | 8 +++++--- lib/elevator/hall_orders/order.ex | 1 - 6 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index a37fcd1..d486a4b 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -12,6 +12,7 @@ defmodule Elevator.CabOrders do GenServer.start_link(__MODULE__, arg, name: __MODULE__) end + @impl true @spec init(any()) :: {:ok, state_t()} def init(_arg \\ []) do state = %{Communicator.my_id() => %{version: 0, orders: MapSet.new()}} @@ -50,18 +51,20 @@ defmodule Elevator.CabOrders do end # --- Handle calls --- - + @impl true def handle_call(:get_my_orders, _from, state) do orders = state[Communicator.my_id()].orders {:reply, orders, state} end + @impl true def handle_call(:get_state, _from, state) do {:reply, state, state} end # --- Handle casts --- + @impl true @spec handle_cast({:receive_state, state_t()}, state_t()) :: {:noreply, state_t()} def handle_cast({:receive_state, other_state}, state) do new_state = @@ -78,6 +81,7 @@ defmodule Elevator.CabOrders do {:noreply, new_state} end + @impl true @spec handle_cast({:button_press, floor_t()}, state_t()) :: {:noreply, state_t()} def handle_cast({:button_press, floor}, state) do new_state = @@ -88,6 +92,7 @@ defmodule Elevator.CabOrders do {:noreply, new_state} end + @impl true @spec handle_cast({:arrived_at_floor, floor_t()}, state_t()) :: {:noreply, state_t()} def handle_cast({:arrived_at_floor, floor}, state) do new_state = diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 4723d5e..203a8a7 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -15,12 +15,13 @@ defmodule Elevator.Communicator do @type cab_orders_t :: Elevator.Types.cab_order_map() @type state_t :: Elevator.Types.communicator_state_map() - @type communicator_options :: [do_resend: boolean()] + @type communicator_options :: [do_resend: boolean(), do_logging: boolean()] - def start_link(arg \\ [do_resend: true]) do + def start_link(arg \\ [do_resend: true, do_logging: false]) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) end + @impl true def init(opts \\ [do_resend: true, do_logging: false]) do if Keyword.get(opts, :do_resend, true) do schedule_state_broadcast() @@ -45,7 +46,6 @@ defmodule Elevator.Communicator do Returns the ID of this node. """ @spec my_id() :: node_id_t() - # TODO: decide on this def my_id, do: Node.self() @doc """ @@ -67,7 +67,7 @@ defmodule Elevator.Communicator do GenServer.cast(__MODULE__, {:update_operation_status, status}) end - # Updates the timestamp when a message is recieved from a node + # Updates the timestamp when a message is received from a node @spec update_state_map(state_t(), node_id_t(), boolean()) :: state_t() defp update_state_map(state, from_node, operational) do from_node_map = %{operational: operational, timestamp: Time.utc_now()} @@ -83,6 +83,7 @@ defmodule Elevator.Communicator do @doc """ Sends the cab and hall state to all connected nodes. """ + @impl true def handle_info(:broadcast_state, state) do # For periodic execution schedule_state_broadcast() @@ -104,6 +105,7 @@ defmodule Elevator.Communicator do end # Update the state map when a new node connects + @impl true def handle_info({:nodeup, node}, state) do {:noreply, update_state_map(state, node, true)} end @@ -123,6 +125,7 @@ defmodule Elevator.Communicator do # --- Handle calls --- + @impl true def handle_call(:who_can_serve, _from, state) do cutoff_ms = Elevator.msg_cutoff_ms() @@ -149,6 +152,7 @@ defmodule Elevator.Communicator do @doc """ Sends received hall and cab orders to respective modules, and updates timestamps for when the connected nodes last sent something. """ + @impl true @spec handle_cast( {:state_update, node_id_t(), boolean(), hall_orders_t(), cab_orders_t()}, state_t() @@ -161,6 +165,7 @@ defmodule Elevator.Communicator do {:noreply, new_state} end + @impl true @spec handle_cast({:update_operation_status, boolean()}, state_t()) :: {:noreply, state_t()} def handle_cast({:update_operation_status, status}, state) do {:noreply, %{state | operational: status}} diff --git a/lib/elevator/fsm/action.ex b/lib/elevator/fsm/action.ex index 6e694d1..1f223e7 100644 --- a/lib/elevator/fsm/action.ex +++ b/lib/elevator/fsm/action.ex @@ -31,7 +31,7 @@ defmodule Elevator.FSM.Action do defp poll_action() do poll_door_timer() - check_motor_timeout_ms() + check_motor_timeout() decide_and_take_action() Process.sleep(@action_interval_ms) poll_action() @@ -45,7 +45,7 @@ defmodule Elevator.FSM.Action do Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) end - defp check_motor_timeout_ms() do + defp check_motor_timeout() do state = State.get_state() timed_out = diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 6aaa4bd..30b57f1 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -14,7 +14,7 @@ defmodule Elevator.HallOrders do @tracked_key {0, :hall_up} - @hall_order_refresh_period 1000 + @hall_order_refresh_period_ms 1000 def start_link(arg) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) @@ -37,7 +37,7 @@ defmodule Elevator.HallOrders do |> Enum.map(&{&1, {0, :idle}}) |> Enum.into(%{}) - Process.send_after(self(), :refresh_hall_orders, @hall_order_refresh_period) + Process.send_after(self(), :refresh_hall_orders, @hall_order_refresh_period_ms) {:ok, state} end @@ -129,7 +129,6 @@ defmodule Elevator.HallOrders do new_order_map = Map.keys(order_map) |> Enum.map(fn key -> - # TODO new_value = Order.merge_hall_orders(key, order_map[key], other_order_map[key], my_orders) {key, new_value} end) @@ -158,7 +157,7 @@ defmodule Elevator.HallOrders do {old_order_version, old_order_state} = old_order_value - order_map = + new_order_map = case old_order_state do :idle -> Map.put(order_map, key, {old_order_version + 1, {:pending, MapSet.new([Node.self()])}}) @@ -167,13 +166,13 @@ defmodule Elevator.HallOrders do order_map end - new_order_value = order_map[key] + new_order_value = new_order_map[key] Logger.info(fn -> "Hall button press #{inspect(key)}: #{inspect(old_order_value)} -> #{inspect(new_order_value)}" end) - {:noreply, order_map, {:continue, :hall_update_state}} + {:noreply, new_order_map, {:continue, :hall_update_state}} end @impl true @@ -199,7 +198,7 @@ defmodule Elevator.HallOrders do @impl true def handle_info(:refresh_hall_orders, order_map) do - Process.send_after(self(), :refresh_hall_orders, @hall_order_refresh_period) + Process.send_after(self(), :refresh_hall_orders, @hall_order_refresh_period_ms) {:noreply, order_map, {:continue, :hall_update_state}} end @@ -234,7 +233,6 @@ defmodule Elevator.HallOrders do Enum.filter(order_map, fn {_, {_order_version, order_state}} -> case order_state do {:confirmed, cost_map} -> - # Hmm. Cost.min_alive_cost(cost_map, alive) == Node.self() _ -> diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 5bed8e5..ab66ee9 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -3,7 +3,8 @@ defmodule Elevator.HallOrders.Cost do require Logger @doc """ - Maybe even random numbers? + Compute the cost of serving an order. + Returns the estimated serve time in ms. """ def compute_cost({floor, btn_dir}, my_hall_orders) do state = Elevator.FSM.State.get_state() @@ -24,7 +25,7 @@ defmodule Elevator.HallOrders.Cost do end @doc """ - Merge two cost maps. + Merge two cost maps. Uses pessimistic merge: If two conflicting costs for the same node are found, keep the higher one. """ def merge_cost(cost_map_1, cost_map_2) do @@ -58,7 +59,8 @@ defmodule Elevator.HallOrders.Cost do fn {node1, cost1}, {node2, cost2} -> cost1 < cost2 or (cost1 == cost2 and node1 < node2) end, - fn -> {:nonode@nohost, Inf} end + # Fallback when no alive costs exist + fn -> {:nonode@nohost, :infinity} end ) min_node diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index 31001a4..dacf63c 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -4,7 +4,6 @@ defmodule Elevator.HallOrders.Order do A hall order is tied to a floor and direction (up/down). It is essentially one of the hall buttons. It is in one of the following states: - - unknown: Initial, will transition to any state. Light: off - idle: No known order. Light: off - pending: Someone pressed a button, but everyone does not know it. Light: off - confirmed: All alive nodes know about the order and has indicated their preference to it. Light on. From a5107963ddb0873032ef03686f58384a6f7f9ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 14:47:06 +0100 Subject: [PATCH 4/7] chore: nitpicking cleanup --- lib/elevator/communicator.ex | 5 ++++- lib/elevator/fsm/state.ex | 4 ++++ lib/elevator/hall_orders.ex | 11 +++++++---- lib/elevator/hall_orders/order.ex | 6 +++--- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 203a8a7..9ac3398 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -60,7 +60,8 @@ defmodule Elevator.Communicator do end @doc """ - Updates the operational key in the state map. + Updates the `operational` part of the state. + Signals to peers whether this node can serve orders. """ @spec update_operation_status(boolean()) :: :ok def update_operation_status(status) do @@ -111,10 +112,12 @@ defmodule Elevator.Communicator do end # Delete node from state map on disconnect + @impl true def handle_info({:nodedown, node}, state) do {:noreply, %{state | connected_nodes: Map.delete(state.connected_nodes, node)}} end + @impl true def handle_info(:log_debug, state) do Process.send_after(self(), :log_debug, 1000) Logger.debug("My id: #{my_id()}") diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 1ec404d..066eaf0 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -48,6 +48,10 @@ defmodule Elevator.FSM.State do def set_behavior(behavior), do: GenServer.cast(__MODULE__, {:set_behavior, behavior}) + @doc """ + Opens the door if the elevator is at a floor. + Does nothing if the elevator is between floors. + """ def open_door(), do: GenServer.cast(__MODULE__, :open_door) def set_motor_timed_out(timed_out), diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 30b57f1..2e73e60 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -12,6 +12,7 @@ defmodule Elevator.HallOrders do @type floor :: Elevator.Types.floor() @type hall_btn :: Elevator.Types.hall_btn() + # Key tracked for debug logging only @tracked_key {0, :hall_up} @hall_order_refresh_period_ms 1000 @@ -43,8 +44,8 @@ defmodule Elevator.HallOrders do end @doc """ - Callback for receiving the hall order state from another node. - Merges the states by updating the individual + Receives the hall order state from another node and merges it into local state. + Each order is merged individually using the consensus algorithm in `HallOrders.Order`. """ @spec receive_state(hall_order_map()) :: :ok def receive_state(other_state), do: GenServer.cast(__MODULE__, {:receive_state, other_state}) @@ -58,6 +59,7 @@ defmodule Elevator.HallOrders do @doc """ Callback for clearing a floor. + Only clears `confirmed` orders, not pending ones. """ @spec arrived_at_floor(floor(), :up | :down) :: :ok def arrived_at_floor(floor, direction) do @@ -113,6 +115,7 @@ defmodule Elevator.HallOrders do {:reply, confirmed_orders, order_map} end + @impl true def handle_call(:get_state, _, order_map) do {:reply, order_map, order_map} end @@ -184,7 +187,7 @@ defmodule Elevator.HallOrders do order_value = order_map[key] # TODO: Maybe check that it is our order - order_map = + new_order_map = case order_value do {order_version, {:confirmed, _}} -> Map.put(order_map, key, {order_version + 1, :idle}) @@ -193,7 +196,7 @@ defmodule Elevator.HallOrders do order_map end - {:noreply, order_map} + {:noreply, new_order_map} end @impl true diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index dacf63c..59a8606 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -53,9 +53,9 @@ defmodule Elevator.HallOrders.Order do end @doc """ - Maybe update a hall order based on its own state. - This may happen for example when the order autonomously transitions from pending to - confirmed when only one elevator is alive. + Advances a pending order to confirmed if the barrier set is full. + Computes and records this node's cost at the point of confirmation. + Returns `{true, new_value}` if the state changed, `{false, unchanged}` otherwise. """ @spec update_hall_order(hall_order_key(), hall_order_value(), %{ Types.floor() => MapSet.t(Types.hall_btn()) From b8fac1d1dcff91bc1cc211d19650fd6cbbd48a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 15:29:56 +0100 Subject: [PATCH 5/7] merge in main --- README.md | 4 +- config/config.exs | 1 - lib/elevator.ex | 8 +- lib/elevator/application.ex | 4 +- lib/elevator/decision.ex | 33 +---- lib/elevator/fsm/action.ex | 3 +- lib/elevator/fsm/state.ex | 2 +- lib/elevator/hall_orders.ex | 36 ++--- lib/elevator/hall_orders/cost.ex | 234 ++++++++++++++++++++++++------ lib/elevator/hall_orders/order.ex | 84 +++++------ lib/elevator/hardware/driver.ex | 107 ++++++++------ lib/elevator/types.ex | 2 +- mix.exs | 3 +- mix.lock | 1 - test/single/cost_test.exs | 49 +++++++ test/single/decision_test.exs | 34 ++--- 16 files changed, 392 insertions(+), 213 deletions(-) create mode 100644 test/single/cost_test.exs diff --git a/README.md b/README.md index d2c3fe0..663a375 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # TTK4145 Elevator ### LOC stats (Elixir) -**Lib:** 1115\ -**Test:** 546 +**Lib:** 1214\ +**Test:** 582 ## Running nodes diff --git a/config/config.exs b/config/config.exs index c3d8f91..289e3e2 100644 --- a/config/config.exs +++ b/config/config.exs @@ -1,5 +1,4 @@ import Config -config :elevator, num_floors: 4 if config_env() == :dev do config :pre_commit, commands: ["format --check-formatted"], verbose: true diff --git a/lib/elevator.ex b/lib/elevator.ex index 37a60a9..76f4158 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -1,11 +1,17 @@ defmodule Elevator do - @num_floors Application.compile_env(:elevator, :num_floors, 4) + @num_floors 4 @resend_period_ms 50 @msg_cutoff_ms 10000 + @door_open_duration_ms 1000 + def num_floors do @num_floors end + def door_open_duration_ms do + @door_open_duration_ms + end + def resend_period_ms do @resend_period_ms end diff --git a/lib/elevator/application.ex b/lib/elevator/application.ex index a2e1486..ebc8890 100644 --- a/lib/elevator/application.ex +++ b/lib/elevator/application.ex @@ -10,13 +10,13 @@ defmodule Elevator.Application do Elevator.Communicator, {Elevator.HallOrders, Elevator.num_floors()}, Elevator.CabOrders, - {Elevator.Hardware.Driver, [{127, 0, 0, 1}, driver_port]}, Elevator.FSM.State, Elevator.FSM.Action, + {Elevator.Hardware.Driver, [{127, 0, 0, 1}, driver_port]}, Elevator.Hardware.InputPoller ] - opts = [strategy: :one_for_one, name: Elevator.Supervisor] + opts = [strategy: :rest_for_one, name: Elevator.Supervisor] Supervisor.start_link(children, opts) end end diff --git a/lib/elevator/decision.ex b/lib/elevator/decision.ex index 8c3bf71..9ce5508 100644 --- a/lib/elevator/decision.ex +++ b/lib/elevator/decision.ex @@ -5,13 +5,11 @@ defmodule Elevator.Decision do These functions are intentionally pure to make them easy to unit test. """ - # Private helpers - - defp requests_above?(reqs, floor) do + def requests_above?(reqs, floor) do Enum.any?(reqs, fn {f, _} -> f > floor end) end - defp requests_below?(reqs, floor) do + def requests_below?(reqs, floor) do Enum.any?(reqs, fn {f, _} -> f < floor end) end @@ -31,7 +29,6 @@ defmodule Elevator.Decision do btn_type == :cab -> true direction == :up and btn_type == :hall_up -> true direction == :down and btn_type == :hall_down -> true - direction == :stop -> true true -> false end end @@ -49,9 +46,7 @@ defmodule Elevator.Decision do @doc "Single decision function for elevator behavior. Returns both direction and behavior for the current state and order snapshot." @spec next_action(Elevator.Types.combined_order_map(), Elevator.FSM.State.t()) :: - {:down, :moving | :door_open} - | {:up, :moving | :door_open} - | {:stop, :idle | :door_open} + {Elevator.Types.elev_dir(), :moving | :door_open | :idle} def next_action( orders, %Elevator.FSM.State{ @@ -61,16 +56,14 @@ defmodule Elevator.Decision do } ) do btns_at_floor = Map.get(orders, floor, MapSet.new()) + direction = if direction in [:up, :down], do: direction, else: :down cond do - between_floors and direction == :stop -> - {:down, :moving} - between_floors -> {direction, :moving} map_size(orders) == 0 -> - {:stop, :idle} + {direction, :idle} direction == :up -> cond do @@ -87,7 +80,7 @@ defmodule Elevator.Decision do {:down, :moving} true -> - {:stop, :idle} + {:up, :idle} end direction == :down -> @@ -105,21 +98,11 @@ defmodule Elevator.Decision do {:up, :moving} true -> - {:stop, :idle} - end - - direction == :stop -> - cond do - MapSet.member?(btns_at_floor, :hall_up) -> {:up, :door_open} - MapSet.member?(btns_at_floor, :hall_down) -> {:down, :door_open} - MapSet.member?(btns_at_floor, :cab) -> {:stop, :door_open} - requests_above?(orders, floor) -> {:up, :moving} - requests_below?(orders, floor) -> {:down, :moving} - true -> {:stop, :idle} + {:down, :idle} end true -> - {:stop, :idle} + {:down, :idle} end end end diff --git a/lib/elevator/fsm/action.ex b/lib/elevator/fsm/action.ex index 1f223e7..12c1db0 100644 --- a/lib/elevator/fsm/action.ex +++ b/lib/elevator/fsm/action.ex @@ -9,7 +9,6 @@ defmodule Elevator.FSM.Action do alias Elevator.HallOrders alias Elevator.Decision - @door_open_time_ms 1000 @motor_timeout_ms 4000 @action_interval_ms 100 @@ -97,7 +96,7 @@ defmodule Elevator.FSM.Action do if state.behavior == :door_open and Time.after?( Time.utc_now(), - Time.add(state.door_open_time_ms, @door_open_time_ms, :millisecond) + Time.add(state.door_open_time, Elevator.door_open_duration_ms(), :millisecond) ) do if state.obstructed do State.open_door() diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 066eaf0..e0de1b7 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -6,7 +6,7 @@ defmodule Elevator.FSM.State do alias Elevator.Hardware.Outputs alias Elevator.Types - defstruct direction: :stop, + defstruct direction: :down, behavior: :idle, floor: :unknown, between_floors: true, diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 2e73e60..abf7981 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -1,6 +1,10 @@ defmodule Elevator.HallOrders do @moduledoc """ Module responsible for all changes occuring to the hall_order part of the state. + The events that can change hall orders are: + - Button is pressed. + - Arrived at floor. + - Received hall orders from another node. """ alias Elevator.HallOrders.Order alias Elevator.HallOrders.Cost @@ -12,9 +16,6 @@ defmodule Elevator.HallOrders do @type floor :: Elevator.Types.floor() @type hall_btn :: Elevator.Types.hall_btn() - # Key tracked for debug logging only - @tracked_key {0, :hall_up} - @hall_order_refresh_period_ms 1000 def start_link(arg) do @@ -51,15 +52,14 @@ defmodule Elevator.HallOrders do def receive_state(other_state), do: GenServer.cast(__MODULE__, {:receive_state, other_state}) @doc """ - Callback for a button press. + Places the corresponding order in pending state if it is in idle. """ @spec button_press(floor(), hall_btn()) :: :ok def button_press(floor, button_type), do: GenServer.cast(__MODULE__, {:button_press, floor, button_type}) @doc """ - Callback for clearing a floor. - Only clears `confirmed` orders, not pending ones. + Goes back to idle if the order is confirmed. """ @spec arrived_at_floor(floor(), :up | :down) :: :ok def arrived_at_floor(floor, direction) do @@ -83,7 +83,7 @@ defmodule Elevator.HallOrders do end @doc """ - Get all orders in same format as get_my_orders. + Get all confirmed orders in same format as get_my_orders. These are the orders we turn the light on for. """ @spec get_confirmed_orders() :: %{Elevator.Types.floor() => MapSet.t(Elevator.Types.hall_btn())} @@ -137,16 +137,6 @@ defmodule Elevator.HallOrders do end) |> Enum.into(%{}) - old_tracked = Map.get(order_map, @tracked_key) - new_tracked = Map.get(new_order_map, @tracked_key) - - if old_tracked != new_tracked do - Logger.info(fn -> - "Tracked hall order #{inspect(@tracked_key)} changed: " <> - "#{inspect(old_tracked)} -> #{inspect(new_tracked)}" - end) - end - {:noreply, new_order_map, {:continue, :hall_update_state}} end @@ -171,9 +161,11 @@ defmodule Elevator.HallOrders do new_order_value = new_order_map[key] - Logger.info(fn -> - "Hall button press #{inspect(key)}: #{inspect(old_order_value)} -> #{inspect(new_order_value)}" - end) + if old_order_value != new_order_value do + Logger.debug(fn -> + "hall_button_press floor=#{floor} button=#{direction} from=#{inspect(old_order_value)} to=#{inspect(new_order_value)}" + end) + end {:noreply, new_order_map, {:continue, :hall_update_state}} end @@ -186,7 +178,6 @@ defmodule Elevator.HallOrders do key = {floor, button_type} order_value = order_map[key] - # TODO: Maybe check that it is our order new_order_map = case order_value do {order_version, {:confirmed, _}} -> @@ -236,7 +227,7 @@ defmodule Elevator.HallOrders do Enum.filter(order_map, fn {_, {_order_version, order_state}} -> case order_state do {:confirmed, cost_map} -> - Cost.min_alive_cost(cost_map, alive) == Node.self() + Cost.min_alive_cost(cost_map, alive) == Communicator.my_id() _ -> false @@ -250,6 +241,7 @@ defmodule Elevator.HallOrders do | Enumerable.t({Elevator.Types.hall_order_key(), Elevator.Types.hall_order_value()}) @spec orders_by_floor(enum_orders()) :: %{floor() => MapSet.t(hall_btn())} defp orders_by_floor(orders) do + # Restructure order map to the format floor => MapSet(order) orders |> Enum.map(fn {{floor, btn_type}, _} -> {floor, btn_type} end) |> Enum.group_by(fn {floor, _} -> floor end) diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index ab66ee9..b5f7937 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -1,26 +1,50 @@ defmodule Elevator.HallOrders.Cost do + @moduledoc """ + Hall order cost utilities. + + Cost is estimated by simulating the local elevator with current requests plus the candidate hall request. + """ + alias Elevator.CabOrders + alias Elevator.Decision + alias Elevator.FSM.State require Logger - @doc """ - Compute the cost of serving an order. - Returns the estimated serve time in ms. - """ + @travel_duration_ms 2500 + @max_simulation_steps 256 + @unreachable_cost 30000 + + @type floor_t :: Elevator.Types.floor() + @type hall_btn_t :: Elevator.Types.hall_btn() + @type combined_orders_t :: Elevator.Types.combined_order_map() + @type cost_map_t :: %{node() => non_neg_integer()} + + @spec compute_cost({floor_t(), hall_btn_t()}, %{floor_t() => MapSet.t(hall_btn_t())}) :: + non_neg_integer() def compute_cost({floor, btn_dir}, my_hall_orders) do - state = Elevator.FSM.State.get_state() + try do + state = State.get_state() + cab_orders = CabOrders.get_my_orders() - cab_orders = CabOrders.get_my_orders() + hall_orders_with_target = + Map.update(my_hall_orders, floor, MapSet.new([btn_dir]), &MapSet.put(&1, btn_dir)) - payload_map = - state_and_orders_to_external_format({floor, btn_dir}, state, cab_orders, my_hall_orders) + combined_orders = Decision.combine_hall_and_cab(hall_orders_with_target, cab_orders) - try do - json_input = JSON.encode!(payload_map) - {output, 0} = System.cmd(Elevator.time_to_serve_executable(), ["-i", json_input]) - String.to_integer(String.trim(output)) + result = simulate_cost_until_served(combined_orders, state, {floor, btn_dir}) + + Logger.debug(fn -> + "hall_cost request=#{inspect({floor, btn_dir})} state=#{state.behavior}@#{inspect(state.floor)} dir=#{state.direction} result=#{result}" + end) + + result rescue - _ -> - 30000 + error -> + Logger.warning( + "Failed to compute hall cost for #{inspect({floor, btn_dir})}: #{inspect(error)}" + ) + + @unreachable_cost end end @@ -28,6 +52,7 @@ defmodule Elevator.HallOrders.Cost do Merge two cost maps. Uses pessimistic merge: If two conflicting costs for the same node are found, keep the higher one. """ + @spec merge_cost(cost_map_t(), cost_map_t()) :: cost_map_t() def merge_cost(cost_map_1, cost_map_2) do MapSet.new(Map.keys(cost_map_1) ++ Map.keys(cost_map_2)) |> Enum.map(fn node -> @@ -50,6 +75,7 @@ defmodule Elevator.HallOrders.Cost do @doc """ Returns the node with the lowest cost for a given cost map and alive set. """ + @spec min_alive_cost(cost_map_t(), MapSet.t(node())) :: node() def min_alive_cost(cost_map, alive_set) do alive_costs = Enum.filter(cost_map, fn {node, _} -> MapSet.member?(alive_set, node) end) @@ -66,38 +92,154 @@ defmodule Elevator.HallOrders.Cost do min_node end - # Represent state and orders in the format expected by the time_to_serve program. - defp state_and_orders_to_external_format( - {order_floor, order_btn_dir}, - elev_state, - cab_orders, - hall_orders - ) do - behavior_remap = [idle: "idle", moving: "moving", door_open: "doorOpen"][elev_state.behavior] - order_dir_remap = [hall_up: :up, hall_down: :down][order_btn_dir] - - cab_orders_bool_table = - 0..(Elevator.num_floors() - 1) - |> Enum.map(fn floor -> MapSet.member?(cab_orders, floor) end) - - hall_orders_bool_table = - 0..(Elevator.num_floors() - 1) - |> Enum.map(fn floor -> - [ - MapSet.member?(Map.get(hall_orders, floor, MapSet.new()), :hall_up), - MapSet.member?(Map.get(hall_orders, floor, MapSet.new()), :hall_down) - ] - end) + @spec simulate_cost_until_served(combined_orders_t(), State.t(), {floor_t(), hall_btn_t()}) :: + non_neg_integer() + defp simulate_cost_until_served(_orders, %{floor: :unknown}, _target), do: @unreachable_cost + + defp simulate_cost_until_served(orders, state, target) do + normalized_state = + if state.direction in [:up, :down], do: state, else: %{state | direction: :down} - %{ - state: %{ - state: behavior_remap, - floor: elev_state.floor, - direction: elev_state.direction, - cabRequests: cab_orders_bool_table - }, - hallRequests: hall_orders_bool_table, - newOrder: %{floor: order_floor, direction: order_dir_remap} - } + if target_cleared?(orders, target) do + 0 + else + do_simulate(orders, normalized_state, target, 0, @max_simulation_steps) + end + end + + defp do_simulate(_orders, _state, _target, _time_ms, 0), do: @unreachable_cost + + defp do_simulate(orders, state, target, time_ms, steps_left) do + if target_cleared?(orders, target) do + time_ms + else + {direction, behavior} = Decision.next_action(orders, state) + + case behavior do + :idle -> + @unreachable_cost + + :moving -> + case move_one_floor(state.floor, direction) do + {:ok, next_floor} -> + next_state = %{ + state + | floor: next_floor, + between_floors: false, + direction: direction, + behavior: :moving + } + + do_simulate( + orders, + next_state, + target, + time_ms + @travel_duration_ms, + steps_left - 1 + ) + + :error -> + @unreachable_cost + end + + :door_open -> + next_orders = clear_requests_at_floor_in_direction(orders, state.floor, direction) + + next_state = %{ + state + | direction: direction, + behavior: :idle, + between_floors: false + } + + do_simulate( + next_orders, + next_state, + target, + time_ms + Elevator.door_open_duration_ms(), + steps_left - 1 + ) + end + end + end + + defp target_cleared?(orders, {floor, btn_dir}) do + orders + |> Map.get(floor, MapSet.new()) + |> MapSet.member?(btn_dir) + |> Kernel.not() + end + + defp move_one_floor(floor, :up) when is_integer(floor) do + if floor < Elevator.num_floors() - 1, do: {:ok, floor + 1}, else: :error + end + + defp move_one_floor(floor, :down) when is_integer(floor) do + if floor > 0, do: {:ok, floor - 1}, else: :error + end + + defp move_one_floor(_, _), do: :error + + defp clear_requests_at_floor_in_direction(orders, floor, direction) do + orders + |> clear_button(floor, :cab) + |> clear_hall_for_direction(floor, direction) + |> prune_empty_floor(floor) + end + + defp clear_hall_for_direction(orders, floor, :up) do + cond do + button_present?(orders, floor, :hall_up) -> + clear_button(orders, floor, :hall_up) + + Decision.requests_above?(orders, floor) -> + orders + + true -> + clear_button(orders, floor, :hall_down) + end + end + + defp clear_hall_for_direction(orders, floor, :down) do + cond do + button_present?(orders, floor, :hall_down) -> + clear_button(orders, floor, :hall_down) + + Decision.requests_below?(orders, floor) -> + orders + + true -> + clear_button(orders, floor, :hall_up) + end + end + + defp button_present?(orders, floor, btn) do + orders + |> Map.get(floor, MapSet.new()) + |> MapSet.member?(btn) + end + + defp clear_button(orders, floor, btn) do + case Map.get(orders, floor) do + nil -> + orders + + buttons -> + Map.put(orders, floor, MapSet.delete(buttons, btn)) + end + end + + defp prune_empty_floor(orders, floor) do + case Map.get(orders, floor) do + nil -> + orders + + buttons -> + if MapSet.size(buttons) == 0 do + Map.delete(orders, floor) + else + orders + end + end end end diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index 59a8606..89cb5cd 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -3,10 +3,11 @@ defmodule Elevator.HallOrders.Order do Logic concerning a single Hall Order. A hall order is tied to a floor and direction (up/down). It is essentially - one of the hall buttons. It is in one of the following states: + one of the hall buttons. An order has both a version number and a state. + State is one of the following: - idle: No known order. Light: off - pending: Someone pressed a button, but everyone does not know it. Light: off - - confirmed: All alive nodes know about the order and has indicated their preference to it. Light on. + - confirmed: All alive nodes know about the order and has indicated their cost to serve it. Light on. """ alias Elevator.Types @@ -23,33 +24,36 @@ defmodule Elevator.HallOrders.Order do Types.floor() => MapSet.t(Types.hall_btn()) }) :: hall_order_value() - def merge_hall_orders(button_key, button_state, other_state, my_hall_orders) do - {new_button_version, new_button_state} = merge_orders(button_state, other_state) + def merge_hall_orders(order_key, order_value, other_order_value, my_hall_orders) do + {new_order_version, new_order_state} = merge_orders(order_value, other_order_value) + # Ensure self is in any barrier set. - {new_button_version, new_button_state} = - case new_button_state do + new_order_state = + case new_order_state do {:pending, barrier_set} -> - {new_button_version, {:pending, MapSet.put(barrier_set, Node.self())}} + {:pending, MapSet.put(barrier_set, Node.self())} _ -> - {new_button_version, new_button_state} + new_order_state end # Ensure self is in a score map - case new_button_state do - {:confirmed, cost_map} -> - my_id = Communicator.my_id() - - if not Map.has_key?(cost_map, my_id) do - {new_button_version, - {:confirmed, Map.put(cost_map, my_id, Cost.compute_cost(button_key, my_hall_orders))}} - else - {new_button_version, new_button_state} - end + new_order_state = + case new_order_state do + {:confirmed, cost_map} -> + my_id = Communicator.my_id() - _ -> - {new_button_version, new_button_state} - end + if not Map.has_key?(cost_map, my_id) do + {:confirmed, Map.put(cost_map, my_id, Cost.compute_cost(order_key, my_hall_orders))} + else + new_order_state + end + + _ -> + new_order_state + end + + {new_order_version, new_order_state} end @doc """ @@ -60,25 +64,27 @@ defmodule Elevator.HallOrders.Order do @spec update_hall_order(hall_order_key(), hall_order_value(), %{ Types.floor() => MapSet.t(Types.hall_btn()) }) :: {boolean(), hall_order_value()} - def update_hall_order(key, {button_version, button_state}, confirmed_hall_orders) do + def update_hall_order(order_key, {order_version, order_state}, confirmed_hall_orders) do alive = Communicator.who_can_serve() - case button_state do - {:pending, barrier_set} -> - if MapSet.intersection(barrier_set, alive) == alive do - my_cost = Cost.compute_cost(key, confirmed_hall_orders) - {true, {button_version, {:confirmed, %{Communicator.my_id() => my_cost}}}} - else - {false, {button_version, button_state}} - end + {did_change, new_state} = + case order_state do + {:pending, barrier_set} -> + if MapSet.intersection(barrier_set, alive) == alive do + my_cost = Cost.compute_cost(order_key, confirmed_hall_orders) + {true, {:confirmed, %{Communicator.my_id() => my_cost}}} + else + {false, order_state} + end - _ -> - {false, {button_version, button_state}} - end + _ -> + {false, order_state} + end + + {did_change, {order_version, new_state}} end defp merge_orders({my_version, my_state}, {other_version, other_state}) do - # This is the full state machine of the hall order consensus algorithm. cond do my_version > other_version -> {my_version, my_state} @@ -88,20 +94,14 @@ defmodule Elevator.HallOrders.Order do true -> case {my_state, other_state} do - {:idle, other_state} -> - {other_version, other_state} - - {_, :idle} -> - {my_version, my_state} - {{:pending, my_barrier}, {:pending, other_barrier}} -> {my_version, {:pending, MapSet.union(my_barrier, other_barrier)}} {{:confirmed, my_cost_map}, {:confirmed, other_cost_map}} -> {my_version, {:confirmed, Cost.merge_cost(my_cost_map, other_cost_map)}} - {{:confirmed, _}, _} -> - {my_version, my_state} + {:idle, _} -> + {other_version, other_state} {_, {:confirmed, _}} -> {other_version, other_state} diff --git a/lib/elevator/hardware/driver.ex b/lib/elevator/hardware/driver.ex index 7b1943a..ef44fa8 100644 --- a/lib/elevator/hardware/driver.ex +++ b/lib/elevator/hardware/driver.ex @@ -1,6 +1,9 @@ defmodule Elevator.Hardware.Driver do use GenServer + require Logger + @call_timeout_ms 1000 + @reconnect_interval_ms 1000 @button_map %{:hall_up => 0, :hall_down => 1, :cab => 2} @state_map %{:on => 1, :off => 0} @direction_map %{:up => 1, :down => 255, :stop => 0} @@ -19,8 +22,24 @@ defmodule Elevator.Hardware.Driver do @impl true def init([address, port]) do - {:ok, socket} = :gen_tcp.connect(address, port, [{:active, false}]) - {:ok, socket} + socket = connect(address, port) + {:ok, {socket, address, port}} + end + + defp connect(address, port) do + case :gen_tcp.connect(address, port, [{:active, false}]) do + {:ok, socket} -> + Logger.info("Driver connected to elevator server at #{inspect(address)}:#{port}") + socket + + {:error, reason} -> + Logger.warning( + "Driver failed to connect (#{reason}), retrying in #{@reconnect_interval_ms}ms..." + ) + + Process.sleep(@reconnect_interval_ms) + connect(address, port) + end end # User API ---------------------------------------------- @@ -141,95 +160,87 @@ defmodule Elevator.Hardware.Driver do # Casts ---------------------------------------------- @impl true - def handle_cast({:set_motor_direction, direction}, socket) do + def handle_cast({:set_motor_direction, direction}, {socket, addr, port}) do :gen_tcp.send(socket, [1, @direction_map[direction], 0, 0]) - {:noreply, socket} + {:noreply, {socket, addr, port}} end @impl true - def handle_cast({:set_order_button_light, button_type, floor, state}, socket) do + def handle_cast({:set_order_button_light, button_type, floor, state}, {socket, addr, port}) do :gen_tcp.send(socket, [2, @button_map[button_type], floor, @state_map[state]]) - {:noreply, socket} + {:noreply, {socket, addr, port}} end @impl true - def handle_cast({:set_floor_indicator, floor}, socket) do + def handle_cast({:set_floor_indicator, floor}, {socket, addr, port}) do :gen_tcp.send(socket, [3, floor, 0, 0]) - {:noreply, socket} + {:noreply, {socket, addr, port}} end @impl true - def handle_cast({:set_door_open_light, state}, socket) do + def handle_cast({:set_door_open_light, state}, {socket, addr, port}) do :gen_tcp.send(socket, [4, @state_map[state], 0, 0]) - {:noreply, socket} + {:noreply, {socket, addr, port}} end @impl true - def handle_cast({:set_stop_button_light, state}, socket) do + def handle_cast({:set_stop_button_light, state}, {socket, addr, port}) do :gen_tcp.send(socket, [5, @state_map[state], 0, 0]) - {:noreply, socket} + {:noreply, {socket, addr, port}} end # Calls ---------------------------------------------- @impl true - def handle_call({:get_order_button_state, floor, order_type}, _from, socket) do + def handle_call({:get_order_button_state, floor, order_type}, _from, {socket, addr, port}) do :gen_tcp.send(socket, [6, @button_map[order_type], floor, 0]) - button_state = - case :gen_tcp.recv(socket, 4, @call_timeout_ms) do - {:ok, [6, 0, 0, 0]} -> :inactive - {:ok, [6, 1, 0, 0]} -> :active - end - - {:reply, button_state, socket} + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do + {:ok, [6, 0, 0, 0]} -> {:reply, :inactive, {socket, addr, port}} + {:ok, [6, 1, 0, 0]} -> {:reply, :active, {socket, addr, port}} + {:error, reason} -> {:stop, reason, {:error, reason}, {socket, addr, port}} + end end @impl true - def handle_call(:get_floor_sensor_state, _from, socket) do + def handle_call(:get_floor_sensor_state, _from, {socket, addr, port}) do :gen_tcp.send(socket, [7, 0, 0, 0]) - floor_state = - case :gen_tcp.recv(socket, 4, @call_timeout_ms) do - {:ok, [7, 0, _, 0]} -> :between_floors - {:ok, [7, 1, floor, 0]} -> floor - end - - {:reply, floor_state, socket} + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do + {:ok, [7, 0, _, 0]} -> {:reply, :between_floors, {socket, addr, port}} + {:ok, [7, 1, floor, 0]} -> {:reply, floor, {socket, addr, port}} + {:error, reason} -> {:stop, reason, {:error, reason}, {socket, addr, port}} + end end @impl true - def handle_call(:get_stop_button_state, _from, socket) do + def handle_call(:get_stop_button_state, _from, {socket, addr, port}) do :gen_tcp.send(socket, [8, 0, 0, 0]) - stop_state = - case :gen_tcp.recv(socket, 4, @call_timeout_ms) do - {:ok, [8, 0, 0, 0]} -> :inactive - {:ok, [8, 1, 0, 0]} -> :active - end - - {:reply, stop_state, socket} + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do + {:ok, [8, 0, 0, 0]} -> {:reply, :inactive, {socket, addr, port}} + {:ok, [8, 1, 0, 0]} -> {:reply, :active, {socket, addr, port}} + {:error, reason} -> {:stop, reason, {:error, reason}, {socket, addr, port}} + end end @impl true - def handle_call(:get_obstruction_switch_state, _from, socket) do + def handle_call(:get_obstruction_switch_state, _from, {socket, addr, port}) do :gen_tcp.send(socket, [9, 0, 0, 0]) - obstruction_state = - case :gen_tcp.recv(socket, 4, @call_timeout_ms) do - {:ok, [9, 0, 0, 0]} -> :inactive - {:ok, [9, 1, 0, 0]} -> :active - end - - {:reply, obstruction_state, socket} + case :gen_tcp.recv(socket, 4, @call_timeout_ms) do + {:ok, [9, 0, 0, 0]} -> {:reply, :inactive, {socket, addr, port}} + {:ok, [9, 1, 0, 0]} -> {:reply, :active, {socket, addr, port}} + {:error, reason} -> {:stop, reason, {:error, reason}, {socket, addr, port}} + end end @impl true - def handle_call(:ping, _from, socket) when is_port(socket) do - {:reply, :ok, socket} + def handle_call(:ping, _from, {socket, addr, port}) when is_port(socket) do + {:reply, :ok, {socket, addr, port}} end @impl true - def handle_call(:ping, _from, socket) do - {:reply, {:error, :not_connected}, socket} + def handle_call(:ping, _from, state) do + {:reply, {:error, :not_connected}, state} end end diff --git a/lib/elevator/types.ex b/lib/elevator/types.ex index acb4786..d42bd6b 100644 --- a/lib/elevator/types.ex +++ b/lib/elevator/types.ex @@ -6,7 +6,7 @@ defmodule Elevator.Types do @type btn_dir :: :up | :down - @type elev_dir :: :up | :down | :stop + @type elev_dir :: :up | :down @type elev_behavior :: :moving | :idle | :door_open diff --git a/mix.exs b/mix.exs index fc72285..91e1781 100644 --- a/mix.exs +++ b/mix.exs @@ -28,8 +28,7 @@ defmodule Elevator.MixProject do # {:dep_from_git, git: "https://github.com/elixir-lang/my_dep.git", tag: "0.1.0"} {:libcluster, "~> 3.3"}, {:dotenvy, "~> 1.0.0"}, - {:pre_commit, "~> 0.3.4", only: :dev}, - {:json, "~> 1.4"} + {:pre_commit, "~> 0.3.4", only: :dev} ] end diff --git a/mix.lock b/mix.lock index 7758ccf..be5b680 100644 --- a/mix.lock +++ b/mix.lock @@ -1,7 +1,6 @@ %{ "dotenvy": {:hex, :dotenvy, "1.0.1", "2d5204e22e44a640c6f43f3a90035fbb65af281e1a2967be1dab63eee87aeabc", [:mix], [], "hexpm", "0727f6c08636b6d6f935c5a0ccedfe0c05bc75e638683bd9fa0a26d7a9931d15"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, - "json": {:hex, :json, "1.4.1", "8648f04a9439765ad449bc56a3ff7d8b11dd44ff08ffcdefc4329f7c93843dfa", [:mix], [], "hexpm", "9abf218dbe4ea4fcb875e087d5f904ef263d012ee5ed21d46e9dbca63f053d16"}, "libcluster": {:hex, :libcluster, "3.5.0", "5ee4cfde4bdf32b2fef271e33ce3241e89509f4344f6c6a8d4069937484866ba", [:mix], [{:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ebf6561fcedd765a4cd43b4b8c04b1c87f4177b5fb3cbdfe40a780499d72f743"}, "pre_commit": {:hex, :pre_commit, "0.3.4", "e2850f80be8090d50ad8019ef2426039307ff5dfbe70c736ad0d4d401facf304", [:mix], [], "hexpm", "16f684ba4f1fed1cba6b19e082b0f8d696e6f1c679285fedf442296617ba5f4e"}, "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"}, diff --git a/test/single/cost_test.exs b/test/single/cost_test.exs new file mode 100644 index 0000000..33ae667 --- /dev/null +++ b/test/single/cost_test.exs @@ -0,0 +1,49 @@ +defmodule Test.Single.CostTest do + use ExUnit.Case, async: false + + alias Elevator.CabOrders + alias Elevator.FSM.State + alias Elevator.HallOrders.Cost + + @travel_duration 2500 + + setup do + start_supervised!({CabOrders, []}) + start_supervised!({Elevator.HallOrders, Elevator.num_floors()}) + start_supervised!(State) + + :ok + end + + test "same-floor request in current direction costs one door cycle" do + set_state(floor: 1, direction: :up, behavior: :idle) + + assert Cost.compute_cost({1, :hall_up}, %{}) == Elevator.door_open_duration_ms() + end + + test "same-floor opposite request is served immediately when no further requests" do + set_state(floor: 1, direction: :up, behavior: :idle) + + assert Cost.compute_cost({1, :hall_down}, %{}) == Elevator.door_open_duration_ms() + end + + test "one floor away request costs travel plus door time" do + set_state(floor: 0, direction: :up, behavior: :idle) + + assert Cost.compute_cost({1, :hall_up}, %{}) == + @travel_duration + Elevator.door_open_duration_ms() + end + + test "unknown floor yields unreachable cost" do + set_state(floor: :unknown, direction: :down, behavior: :idle) + + assert Cost.compute_cost({1, :hall_up}, %{}) == 30000 + end + + defp set_state(opts) do + State.set_direction(Keyword.fetch!(opts, :direction)) + State.set_behavior(Keyword.fetch!(opts, :behavior)) + State.set_floor(Keyword.fetch!(opts, :floor)) + Process.sleep(10) + end +end diff --git a/test/single/decision_test.exs b/test/single/decision_test.exs index 3fa9e0a..7151b34 100644 --- a/test/single/decision_test.exs +++ b/test/single/decision_test.exs @@ -3,27 +3,27 @@ defmodule Test.Single.DecisionTest do alias Elevator.Decision - test "unknown floor, :down -> stop,idle" do + test "unknown floor, :down -> down,idle" do orders = %{} state = %Elevator.FSM.State{floor: :unknown, direction: :down, between_floors: false} - assert Decision.next_action(orders, state) == {:stop, :idle} + assert Decision.next_action(orders, state) == {:down, :idle} end - test "no requests -> stop,idle" do + test "no requests -> keep direction,idle" do orders = %{} - state = %Elevator.FSM.State{floor: 1, direction: :stop, between_floors: false} - assert Decision.next_action(orders, state) == {:stop, :idle} + state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} + assert Decision.next_action(orders, state) == {:up, :idle} end test "nearest above -> up,moving" do orders = %{3 => MapSet.new([:cab]), 4 => MapSet.new([:hall_up])} - state = %Elevator.FSM.State{floor: 1, direction: :stop, between_floors: false} + state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} assert Decision.next_action(orders, state) == {:up, :moving} end test "nearest below -> down,moving" do orders = %{0 => MapSet.new([:cab]), 2 => MapSet.new([:cab])} - state = %Elevator.FSM.State{floor: 3, direction: :stop, between_floors: false} + state = %Elevator.FSM.State{floor: 3, direction: :down, between_floors: false} assert Decision.next_action(orders, state) == {:down, :moving} end @@ -33,21 +33,21 @@ defmodule Test.Single.DecisionTest do assert Decision.next_action(orders, state) == {:up, :moving} end - test "same floor cab -> stop,door_open" do + test "same floor cab while moving up -> up,door_open" do orders = %{2 => MapSet.new([:cab])} - state = %Elevator.FSM.State{floor: 2, direction: :stop, between_floors: false} - assert Decision.next_action(orders, state) == {:stop, :door_open} + state = %Elevator.FSM.State{floor: 2, direction: :up, between_floors: false} + assert Decision.next_action(orders, state) == {:up, :door_open} end - test "same floor hall_up -> up,door_open" do + test "same floor hall_up while idle up -> up,door_open" do orders = %{1 => MapSet.new([:hall_up])} - state = %Elevator.FSM.State{floor: 1, direction: :stop, between_floors: false} + state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} assert Decision.next_action(orders, state) == {:up, :door_open} end - test "same floor hall_down -> down,door_open" do + test "same floor hall_down while idle down -> down,door_open" do orders = %{1 => MapSet.new([:hall_down])} - state = %Elevator.FSM.State{floor: 1, direction: :stop, between_floors: false} + state = %Elevator.FSM.State{floor: 1, direction: :down, between_floors: false} assert Decision.next_action(orders, state) == {:down, :door_open} end @@ -81,13 +81,13 @@ defmodule Test.Single.DecisionTest do moving = %Elevator.FSM.State{floor: 2, direction: :up, behavior: :moving} assert Decision.should_clear_immediately?(moving, 2, :cab) - door_open_stop = %Elevator.FSM.State{ + door_open_down = %Elevator.FSM.State{ floor: 2, - direction: :stop, + direction: :down, behavior: :door_open, between_floors: false } - assert Decision.should_clear_immediately?(door_open_stop, 2, :hall_down) + assert Decision.should_clear_immediately?(door_open_down, 2, :hall_down) end end From ed8e995f08a1e0959e553665401a860e7ab3cdd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 16:32:39 +0100 Subject: [PATCH 6/7] chore: remove more dead code and tidy up tests --- lib/elevator.ex | 5 ----- lib/elevator/hall_orders/cost.ex | 1 + test/multi/cab_orders_test.exs | 4 ++-- test/multi/hall_orders_test.exs | 2 +- test/single/cost_test.exs | 11 +++++++---- test/single/hall_orders_test.exs | 11 ++++++----- test/utils/multi_cluster.ex | 14 +++++++++----- 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index 76f4158..cc37a40 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -19,9 +19,4 @@ defmodule Elevator do def msg_cutoff_ms do @msg_cutoff_ms end - - def time_to_serve_executable do - {:ok, path} = Application.fetch_env(:elevator, :time_to_serve_executable) - path - end end diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index b5f7937..dc9cd6b 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -196,6 +196,7 @@ defmodule Elevator.HallOrders.Cost do orders true -> + # No reason to continue up: clear hall down when turning around clear_button(orders, floor, :hall_down) end end diff --git a/test/multi/cab_orders_test.exs b/test/multi/cab_orders_test.exs index 2bee631..c935805 100644 --- a/test/multi/cab_orders_test.exs +++ b/test/multi/cab_orders_test.exs @@ -45,7 +45,7 @@ defmodule Test.Multi.CabOrdersTest do assert node1_orders == MapSet.new([3]) end - test "elevator ingores lower version numbers", %{nodes: [node1, node2, node3]} do + test "elevator ignores lower version numbers", %{nodes: [node1, node2, node3]} do %{version: node1_version, orders: node1_orders} = :rpc.call(node1, CabOrders, :get_state, [])[node1] @@ -92,7 +92,7 @@ defmodule Test.Multi.CabOrdersTest do assert node1_orders == MapSet.new([1]) end - test "cab order states progate", %{nodes: [node1, node2, node3]} do + test "cab order states propagate", %{nodes: [node1, node2, node3]} do %{version: node1_version, orders: node1_orders} = :rpc.call(node1, CabOrders, :get_state, [])[node1] diff --git a/test/multi/hall_orders_test.exs b/test/multi/hall_orders_test.exs index c38c8c7..e666dfd 100644 --- a/test/multi/hall_orders_test.exs +++ b/test/multi/hall_orders_test.exs @@ -1,4 +1,4 @@ -defmodule Test.Multi.HallOrders do +defmodule Test.Multi.HallOrdersTest do alias Elevator.HallOrders alias Elevator.Communicator alias Test.Utils.MultiCluster diff --git a/test/single/cost_test.exs b/test/single/cost_test.exs index 33ae667..6e680e6 100644 --- a/test/single/cost_test.exs +++ b/test/single/cost_test.exs @@ -5,7 +5,9 @@ defmodule Test.Single.CostTest do alias Elevator.FSM.State alias Elevator.HallOrders.Cost - @travel_duration 2500 + @travel_duration_ms 2500 + @unreachable_cost 30000 + @state_settle_ms 10 setup do start_supervised!({CabOrders, []}) @@ -31,19 +33,20 @@ defmodule Test.Single.CostTest do set_state(floor: 0, direction: :up, behavior: :idle) assert Cost.compute_cost({1, :hall_up}, %{}) == - @travel_duration + Elevator.door_open_duration_ms() + @travel_duration_ms + Elevator.door_open_duration_ms() end test "unknown floor yields unreachable cost" do set_state(floor: :unknown, direction: :down, behavior: :idle) - assert Cost.compute_cost({1, :hall_up}, %{}) == 30000 + assert Cost.compute_cost({1, :hall_up}, %{}) == @unreachable_cost end defp set_state(opts) do State.set_direction(Keyword.fetch!(opts, :direction)) State.set_behavior(Keyword.fetch!(opts, :behavior)) State.set_floor(Keyword.fetch!(opts, :floor)) - Process.sleep(10) + # Wait for GenServer casts to settle + Process.sleep(@state_settle_ms) end end diff --git a/test/single/hall_orders_test.exs b/test/single/hall_orders_test.exs index c36d3b7..64e11d3 100644 --- a/test/single/hall_orders_test.exs +++ b/test/single/hall_orders_test.exs @@ -1,7 +1,8 @@ -defmodule Test.Single.HallOrders do +defmodule Test.Single.HallOrdersTest do use ExUnit.Case, async: false # TODO: Maybe doctest - # doctest Elevator + + @max_continue_iterations 100 setup_all do start_supervised!(Elevator.FSM.State) @@ -41,9 +42,9 @@ defmodule Test.Single.HallOrders do test "clear floor from pending state leaves elevator state unchanged" do {:ok, state} = Elevator.HallOrders.init(3) id = Node.self() - state = Map.put(state, {1, :hall_up}, {:pending, MapSet.new([id])}) + state = Map.put(state, {1, :hall_up}, {0, {:pending, MapSet.new([id])}}) assert {:noreply, final_state} = hallorder_cast_full({:arrived_at_floor, 1, :up}, state) - assert {:pending, _} = final_state[{1, :hall_up}] + assert {_, {:pending, _}} = final_state[{1, :hall_up}] end @tag :hall_orders_single @@ -97,7 +98,7 @@ defmodule Test.Single.HallOrders do defp hallorder_continue_full(continue_arg, state, continue_counter \\ 0) do # Prevent infinite continue loop - assert continue_counter < 100 + assert continue_counter < @max_continue_iterations ret = Elevator.HallOrders.handle_continue(continue_arg, state) diff --git a/test/utils/multi_cluster.ex b/test/utils/multi_cluster.ex index 732aabe..5bc2b40 100644 --- a/test/utils/multi_cluster.ex +++ b/test/utils/multi_cluster.ex @@ -1,6 +1,10 @@ defmodule Test.Utils.MultiCluster do @moduledoc false + @node_shutdown_timeout_ms 1000 + @cluster_wait_timeout_ms 2000 + @poll_interval_ms 50 + @spec start_three_node_cluster(non_neg_integer(), boolean()) :: %{ nodes: [node()], peers: [pid()] @@ -47,7 +51,7 @@ defmodule Test.Utils.MultiCluster do receive do {:DOWN, _, :process, ^pid, _} -> :ok after - 1000 -> :ok + @node_shutdown_timeout_ms -> :ok end end end @@ -87,12 +91,12 @@ defmodule Test.Utils.MultiCluster do {peer, node} end - defp wait_until_connected(nodes, timeout \\ 2000) do + defp wait_until_connected(nodes, timeout \\ @cluster_wait_timeout_ms) do deadline = System.monotonic_time(:millisecond) + timeout wait_connected_loop(nodes, deadline) end - defp wait_until_disconnected(nodes, timeout \\ 2000) do + defp wait_until_disconnected(nodes, timeout \\ @cluster_wait_timeout_ms) do deadline = System.monotonic_time(:millisecond) + timeout wait_disconnected_loop(nodes, deadline) end @@ -105,7 +109,7 @@ defmodule Test.Utils.MultiCluster do raise "Test timed out waiting for nodes" end - Process.sleep(50) + Process.sleep(@poll_interval_ms) wait_connected_loop(nodes, deadline) end end @@ -118,7 +122,7 @@ defmodule Test.Utils.MultiCluster do raise "Test timed out waiting for node disconnect" end - Process.sleep(50) + Process.sleep(@poll_interval_ms) wait_disconnected_loop(nodes, deadline) end end From 03a47545be07dd73afecffd5e0643f434318a286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Mon, 16 Mar 2026 16:40:12 +0100 Subject: [PATCH 7/7] fix: start HallOrders before FSM.state in hall_orders setup --- test/single/hall_orders_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/single/hall_orders_test.exs b/test/single/hall_orders_test.exs index 64e11d3..127fa5a 100644 --- a/test/single/hall_orders_test.exs +++ b/test/single/hall_orders_test.exs @@ -5,10 +5,10 @@ defmodule Test.Single.HallOrdersTest do @max_continue_iterations 100 setup_all do - start_supervised!(Elevator.FSM.State) start_supervised!(Elevator.Communicator) start_supervised!(Elevator.CabOrders) start_supervised!({Elevator.HallOrders, Elevator.num_floors()}) + start_supervised!(Elevator.FSM.State) :ok end