From 19ebd0ee31e717274ab150a1a29d277a03b84eef Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 11:49:30 +0100 Subject: [PATCH 1/3] feat: arrived state in hall order --- lib/elevator/hall_orders.ex | 30 ++++++----- lib/elevator/hall_orders/order.ex | 82 ++++++++++++++++++------------- lib/elevator/hardware/outputs.ex | 4 +- lib/elevator/types.ex | 7 ++- test/multi/hall_orders_test.exs | 6 +-- test/single/hall_orders_test.exs | 8 +-- 6 files changed, 76 insertions(+), 61 deletions(-) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index fd44860..980c937 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -36,7 +36,7 @@ defmodule Elevator.HallOrders do _ -> [{floor, :hall_up}, {floor, :hall_down}] end end) - |> Enum.map(&{&1, {0, :idle}}) + |> Enum.map(&{&1, :idle}) |> Enum.into(%{}) Process.send_after(self(), :refresh_hall_orders, @hall_order_refresh_period_ms) @@ -104,9 +104,9 @@ defmodule Elevator.HallOrders do @impl true def handle_call(:get_confirmed_orders, _from, order_map) do confirmed_orders = - Enum.filter(order_map, fn {_, {_order_version, order_state}} -> + Enum.filter(order_map, fn {_, order_state} -> case order_state do - {:confirmed, _} -> true + {:handling, _} -> true _ -> false end end) @@ -146,24 +146,23 @@ defmodule Elevator.HallOrders do def handle_cast({:button_press, floor, direction}, order_map) do # If in idle, go to pending. Otherwise, ignore. key = {floor, direction} - old_order_value = order_map[key] - {old_order_version, old_order_state} = old_order_value + old_order_state = order_map[key] new_order_map = case old_order_state do :idle -> - Map.put(order_map, key, {old_order_version + 1, {:pending, MapSet.new([Node.self()])}}) + Map.put(order_map, key, {:pending, MapSet.new([Communicator.my_id()])}) _ -> order_map end - new_order_value = new_order_map[key] + new_order_state = new_order_map[key] - if old_order_value != new_order_value do + if old_order_state != new_order_state do Logger.debug(fn -> - "hall_button_press floor=#{floor} button=#{direction} from=#{inspect(old_order_value)} to=#{inspect(new_order_value)}" + "hall_button_press floor=#{floor} button=#{direction} from=#{inspect(old_order_state)} to=#{inspect(new_order_state)}" end) end @@ -172,16 +171,15 @@ defmodule Elevator.HallOrders do @impl true def handle_cast({:arrived_at_floor, floor, direction}, order_map) do - # If in confirmed, go to idle. Otherwise, ignore. # TODO: Find out if barrier set should be full as well? button_type = [up: :hall_up, down: :hall_down][direction] key = {floor, button_type} - order_value = order_map[key] + order_state = order_map[key] new_order_map = - case order_value do - {order_version, {:confirmed, _}} -> - Map.put(order_map, key, {order_version + 1, :idle}) + case order_state do + {:handling, _} -> + Map.put(order_map, key, {:arrived, MapSet.new([Communicator.my_id()])}) _ -> order_map @@ -224,9 +222,9 @@ defmodule Elevator.HallOrders do end defp my_orders_from_order_map(order_map, alive) do - Enum.filter(order_map, fn {_, {_order_version, order_state}} -> + Enum.filter(order_map, fn {_, order_state} -> case order_state do - {:confirmed, cost_map} -> + {:handling, cost_map} -> Cost.min_alive_cost(cost_map, alive) == Communicator.my_id() _ -> diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index 89cb5cd..dc2fac3 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -15,17 +15,17 @@ defmodule Elevator.HallOrders.Order do alias Elevator.Communicator @type hall_order_key :: Elevator.Types.hall_order_key() - @type hall_order_value :: Elevator.Types.hall_order_value() + @type hall_order_state :: Elevator.Types.hall_order_state() @doc """ Update a hall order based on an incoming hall order from another node. """ - @spec merge_hall_orders(hall_order_key(), hall_order_value(), hall_order_value(), %{ + @spec merge_hall_orders(hall_order_key(), hall_order_state(), hall_order_state(), %{ Types.floor() => MapSet.t(Types.hall_btn()) }) :: - hall_order_value() - 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) + hall_order_state() + def merge_hall_orders(order_key, order_state, other_order_state, my_hall_orders) do + new_order_state = merge_orders(order_state, other_order_state) # Ensure self is in any barrier set. new_order_state = @@ -33,18 +33,21 @@ defmodule Elevator.HallOrders.Order do {:pending, barrier_set} -> {:pending, MapSet.put(barrier_set, Node.self())} + {:arrived, barrier_set} -> + {:arrived, MapSet.put(barrier_set, Node.self())} + _ -> new_order_state end - # Ensure self is in a score map + # Ensure self is in a cost map new_order_state = case new_order_state do - {:confirmed, cost_map} -> + {:handling, cost_map} -> my_id = Communicator.my_id() 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))} + {:handling, Map.put(cost_map, my_id, Cost.compute_cost(order_key, my_hall_orders))} else new_order_state end @@ -53,7 +56,7 @@ defmodule Elevator.HallOrders.Order do new_order_state end - {new_order_version, new_order_state} + new_order_state end @doc """ @@ -61,10 +64,10 @@ defmodule Elevator.HallOrders.Order do 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(), %{ + @spec update_hall_order(hall_order_key(), hall_order_state(), %{ Types.floor() => MapSet.t(Types.hall_btn()) - }) :: {boolean(), hall_order_value()} - def update_hall_order(order_key, {order_version, order_state}, confirmed_hall_orders) do + }) :: {boolean(), hall_order_state()} + def update_hall_order(order_key, order_state, confirmed_hall_orders) do alive = Communicator.who_can_serve() {did_change, new_state} = @@ -72,7 +75,14 @@ defmodule Elevator.HallOrders.Order 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}}} + {true, {:handling, %{Communicator.my_id() => my_cost}}} + else + {false, order_state} + end + + {:arrived, barrier_set} -> + if MapSet.intersection(barrier_set, alive) == alive do + {true, :idle} else {false, order_state} end @@ -81,34 +91,40 @@ defmodule Elevator.HallOrders.Order do {false, order_state} end - {did_change, {order_version, new_state}} + {did_change, new_state} end - defp merge_orders({my_version, my_state}, {other_version, other_state}) do - cond do - my_version > other_version -> - {my_version, my_state} + defp merge_orders(my_state, other_state) do + case {my_state, other_state} do + {:idle, {:arrived, _}} -> + my_state + + {:idle, other_state} -> + other_state + + {{:pending, _}, :idle} -> + my_state + + {{:pending, my_barrier}, {:pending, other_barrier}} -> + {:pending, MapSet.union(my_barrier, other_barrier)} - my_version < other_version -> - {other_version, other_state} + {{:pending, _}, other_state} -> + other_state - true -> - case {my_state, other_state} do - {{:pending, my_barrier}, {:pending, other_barrier}} -> - {my_version, {:pending, MapSet.union(my_barrier, other_barrier)}} + {{:handling, my_cost_map}, {:handling, other_cost_map}} -> + {:handling, Cost.merge_cost(my_cost_map, other_cost_map)} - {{:confirmed, my_cost_map}, {:confirmed, other_cost_map}} -> - {my_version, {:confirmed, Cost.merge_cost(my_cost_map, other_cost_map)}} + {{:handling, _}, {:arrived, _}} -> + other_state - {:idle, _} -> - {other_version, other_state} + {{:handling, _}, _} -> + my_state - {_, {:confirmed, _}} -> - {other_version, other_state} + {{:arrived, _}, :idle} -> + other_state - _ -> - {my_version, my_state} - end + _ -> + my_state end end end diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index d8280e2..72d763f 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -45,7 +45,9 @@ defmodule Elevator.Hardware.Outputs do end defp set_floor_light(state) do - Driver.set_floor_indicator(state.floor) + if state.floor != :unknown do + Driver.set_floor_indicator(state.floor) + end end defp get_light_orders() do diff --git a/lib/elevator/types.ex b/lib/elevator/types.ex index d42bd6b..e92428c 100644 --- a/lib/elevator/types.ex +++ b/lib/elevator/types.ex @@ -24,12 +24,11 @@ defmodule Elevator.Types do @type hall_order_state :: :idle | {:pending, MapSet.t()} - | {:confirmed, %{node_id() => integer()}} - - @type hall_order_value :: {non_neg_integer(), hall_order_state()} + | {:handling, %{node_id() => integer()}} + | {:arrived, MapSet.t()} @type hall_order_map :: %{ - hall_order_key() => hall_order_value() + hall_order_key() => hall_order_state() } @type cab_orders_snapshot :: %{ diff --git a/test/multi/hall_orders_test.exs b/test/multi/hall_orders_test.exs index e666dfd..58c9877 100644 --- a/test/multi/hall_orders_test.exs +++ b/test/multi/hall_orders_test.exs @@ -51,7 +51,7 @@ defmodule Test.Multi.HallOrdersTest do assert :rpc.call(node1, HallOrders, :receive_state, [node3_state]) == :ok node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert {_, {:confirmed, _}} = node1_state[{0, :hall_up}] + assert {_, {:handling, _}} = node1_state[{0, :hall_up}] clique_exchange_states([node1, node2, node3]) @@ -61,7 +61,7 @@ defmodule Test.Multi.HallOrdersTest do # All should have converged on the alive set assert node1_state == node2_state and node2_state == node3_state - assert {_, {:confirmed, _}} = node1_state[{0, :hall_up}] + assert {_, {:handling, _}} = node1_state[{0, :hall_up}] converged_state = node1_state # ... so another exchange run should not affect the result @@ -82,7 +82,7 @@ defmodule Test.Multi.HallOrdersTest do clique_exchange_states(nodes) node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert {_, {:confirmed, _}} = node1_state[{1, :hall_down}] + assert {_, {:handling, _}} = node1_state[{1, :hall_down}] # Assume node3 arrives at the floor :rpc.call(node3, HallOrders, :arrived_at_floor, [1, :down]) diff --git a/test/single/hall_orders_test.exs b/test/single/hall_orders_test.exs index 127fa5a..dfe45a5 100644 --- a/test/single/hall_orders_test.exs +++ b/test/single/hall_orders_test.exs @@ -26,14 +26,14 @@ defmodule Test.Single.HallOrdersTest do test "button press puts single elevator confirmed state" do {:ok, state} = Elevator.HallOrders.init(3) assert {:noreply, final_state} = hallorder_cast_full({:button_press, 0, :hall_up}, state) - assert {_, {:confirmed, _}} = final_state[{0, :hall_up}] + assert {_, {:handling, _}} = final_state[{0, :hall_up}] end @tag :hall_orders_single test "arrive at floor from confirmed state puts elevator in idle state" do {:ok, state} = Elevator.HallOrders.init(3) id = Node.self() - state = Map.put(state, {1, :hall_down}, {1, {:confirmed, %{id => 5}}}) + state = Map.put(state, {1, :hall_down}, {1, {:handling, %{id => 5}}}) assert {:noreply, final_state} = hallorder_cast_full({:arrived_at_floor, 1, :down}, state) assert {_, :idle} = final_state[{1, :hall_down}] end @@ -51,9 +51,9 @@ defmodule Test.Single.HallOrdersTest do test "clear floor from other direction leaves elevator state unchanged" do {:ok, state} = Elevator.HallOrders.init(3) id = Node.self() - state = Map.put(state, {1, :hall_up}, {1, {:confirmed, %{id => 5}}}) + state = Map.put(state, {1, :hall_up}, {1, {:handling, %{id => 5}}}) assert {:noreply, final_state} = hallorder_cast_full({:arrived_at_floor, 1, :down}, state) - assert {_, {:confirmed, _}} = final_state[{1, :hall_up}] + assert {_, {:handling, _}} = final_state[{1, :hall_up}] end @tag :hall_orders_single From fc46ccf2a68b30866ec9b6deb61f899908e04b03 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 12:04:15 +0100 Subject: [PATCH 2/3] fix: bugs causing tests to fail --- lib/elevator/hall_orders.ex | 2 +- lib/elevator/hall_orders/order.ex | 3 +++ test/multi/hall_orders_test.exs | 20 ++++++++++---------- test/single/hall_orders_test.exs | 24 ++++++++++++------------ 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 980c937..cd12fa6 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -185,7 +185,7 @@ defmodule Elevator.HallOrders do order_map end - {:noreply, new_order_map} + {:noreply, new_order_map, {:continue, :hall_update_state}} end @impl true diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index dc2fac3..abf800f 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -120,6 +120,9 @@ defmodule Elevator.HallOrders.Order do {{:handling, _}, _} -> my_state + {{:arrived, my_barrier}, {:arrived, other_barrier}} -> + {:arrived, MapSet.union(my_barrier, other_barrier)} + {{:arrived, _}, :idle} -> other_state diff --git a/test/multi/hall_orders_test.exs b/test/multi/hall_orders_test.exs index 58c9877..f7efcb5 100644 --- a/test/multi/hall_orders_test.exs +++ b/test/multi/hall_orders_test.exs @@ -26,12 +26,12 @@ defmodule Test.Multi.HallOrdersTest do # Node 1 gets a button call :rpc.call(node1, HallOrders, :button_press, [0, :hall_up]) node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert {_, {:pending, _}} = node1_state[{0, :hall_up}] + assert {:pending, _} = node1_state[{0, :hall_up}] # Node 1 sends their orders to node 2 assert :rpc.call(node2, HallOrders, :receive_state, [node1_state]) == :ok node2_state = :rpc.call(node2, HallOrders, :get_state, []) - assert {_, {:pending, barrier2}} = node2_state[{0, :hall_up}] + assert {:pending, barrier2} = node2_state[{0, :hall_up}] assert MapSet.member?(barrier2, node1) assert MapSet.member?(barrier2, node2) @@ -40,7 +40,7 @@ defmodule Test.Multi.HallOrdersTest do # Node 1 sends their orders to node 3 assert :rpc.call(node3, HallOrders, :receive_state, [node1_state]) == :ok node3_state = :rpc.call(node3, HallOrders, :get_state, []) - assert {_, {:pending, barrier3}} = node3_state[{0, :hall_up}] + assert {:pending, barrier3} = node3_state[{0, :hall_up}] assert MapSet.member?(barrier3, node1) assert MapSet.member?(barrier3, node3) @@ -51,7 +51,7 @@ defmodule Test.Multi.HallOrdersTest do assert :rpc.call(node1, HallOrders, :receive_state, [node3_state]) == :ok node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert {_, {:handling, _}} = node1_state[{0, :hall_up}] + assert {:handling, _} = node1_state[{0, :hall_up}] clique_exchange_states([node1, node2, node3]) @@ -61,7 +61,7 @@ defmodule Test.Multi.HallOrdersTest do # All should have converged on the alive set assert node1_state == node2_state and node2_state == node3_state - assert {_, {:handling, _}} = node1_state[{0, :hall_up}] + assert {:handling, _} = node1_state[{0, :hall_up}] converged_state = node1_state # ... so another exchange run should not affect the result @@ -82,7 +82,7 @@ defmodule Test.Multi.HallOrdersTest do clique_exchange_states(nodes) node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert {_, {:handling, _}} = node1_state[{1, :hall_down}] + assert {:handling, _} = node1_state[{1, :hall_down}] # Assume node3 arrives at the floor :rpc.call(node3, HallOrders, :arrived_at_floor, [1, :down]) @@ -93,9 +93,9 @@ defmodule Test.Multi.HallOrdersTest do node2_state = :rpc.call(node2, HallOrders, :get_state, []) node3_state = :rpc.call(node3, HallOrders, :get_state, []) - assert {_, :idle} = node1_state[{1, :hall_down}] - assert {_, :idle} = node2_state[{1, :hall_down}] - assert {_, :idle} = node3_state[{1, :hall_down}] + assert :idle = node1_state[{1, :hall_down}] + assert :idle = node2_state[{1, :hall_down}] + assert :idle = node3_state[{1, :hall_down}] end @tag :manual_sending @@ -143,7 +143,7 @@ defmodule Test.Multi.HallOrdersTest do node3_state = :rpc.call(node3, HallOrders, :get_state, []) assert node1_state == node2_state and node2_state == node3_state - assert {_, :idle} = node1_state[{2, :hall_up}] + assert :idle = node1_state[{2, :hall_up}] end defp clique_exchange_states([node1, node2, node3]) do diff --git a/test/single/hall_orders_test.exs b/test/single/hall_orders_test.exs index dfe45a5..33267ca 100644 --- a/test/single/hall_orders_test.exs +++ b/test/single/hall_orders_test.exs @@ -16,44 +16,44 @@ defmodule Test.Single.HallOrdersTest do test "initializes state with idle values" do {:ok, state} = Elevator.HallOrders.init(3) assert map_size(state) == 4 - assert {_, :idle} = state[{0, :hall_up}] - assert {_, :idle} = state[{1, :hall_up}] - assert {_, :idle} = state[{1, :hall_down}] - assert {_, :idle} = state[{2, :hall_down}] + assert :idle = state[{0, :hall_up}] + assert :idle = state[{1, :hall_up}] + assert :idle = state[{1, :hall_down}] + assert :idle = state[{2, :hall_down}] end @tag :hall_orders_single test "button press puts single elevator confirmed state" do {:ok, state} = Elevator.HallOrders.init(3) assert {:noreply, final_state} = hallorder_cast_full({:button_press, 0, :hall_up}, state) - assert {_, {:handling, _}} = final_state[{0, :hall_up}] + assert {:handling, _} = final_state[{0, :hall_up}] end @tag :hall_orders_single test "arrive at floor from confirmed state puts elevator in idle state" do {:ok, state} = Elevator.HallOrders.init(3) id = Node.self() - state = Map.put(state, {1, :hall_down}, {1, {:handling, %{id => 5}}}) + state = Map.put(state, {1, :hall_down}, {:handling, %{id => 5}}) assert {:noreply, final_state} = hallorder_cast_full({:arrived_at_floor, 1, :down}, state) - assert {_, :idle} = final_state[{1, :hall_down}] + assert :idle = final_state[{1, :hall_down}] end @tag :hall_orders_single - test "clear floor from pending state leaves elevator state unchanged" do + test "clear floor from pending state should not put order in idle" do {:ok, state} = Elevator.HallOrders.init(3) id = Node.self() - state = Map.put(state, {1, :hall_up}, {0, {:pending, MapSet.new([id])}}) + state = Map.put(state, {1, :hall_up}, {: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 {:handling, _} = final_state[{1, :hall_up}] end @tag :hall_orders_single test "clear floor from other direction leaves elevator state unchanged" do {:ok, state} = Elevator.HallOrders.init(3) id = Node.self() - state = Map.put(state, {1, :hall_up}, {1, {:handling, %{id => 5}}}) + state = Map.put(state, {1, :hall_up}, {:handling, %{id => 5}}) assert {:noreply, final_state} = hallorder_cast_full({:arrived_at_floor, 1, :down}, state) - assert {_, {:handling, _}} = final_state[{1, :hall_up}] + assert {:handling, _} = final_state[{1, :hall_up}] end @tag :hall_orders_single From 240a062ec65fdb22e9a56d8131825ea006ceb9f1 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 12:18:58 +0100 Subject: [PATCH 3/3] chore: update doc comment --- lib/elevator/hall_orders/order.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index abf800f..5d9b392 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -3,11 +3,12 @@ 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. An order has both a version number and a state. - State is one of the following: + one of the hall buttons. + The state of an order 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 cost to serve it. Light on. + - arrived: A node is signalling that the order has been served. Light: off """ alias Elevator.Types