From b55ac22c5bc12bcbb87b0559dd4b306482602308 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 16:44:38 +0100 Subject: [PATCH 01/29] fix: config file --- config/runtime.exs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/config/runtime.exs b/config/runtime.exs index 8377305..204a9b2 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -12,18 +12,10 @@ driver_port = _ -> 15_657 end -config :elevator, driver_port: driver_port - -{:ok, cwd} = File.cwd() - -config :elevator, time_to_serve_executable: "#{cwd}/server/time_to_serve" - -# ------------------------------------------------------------------ -# Cluster topology -# ------------------------------------------------------------------ - gossip_secret = env!("GOSSIP_SECRET", :string, "change_me_in_dotenv") +config :elevator, driver_port: driver_port + config :libcluster, topologies: [ elevator: [ From 4d89ccfe8eb25b9e6f04d46216fa0ca78c497bba Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 17:12:00 +0100 Subject: [PATCH 02/29] chore: action -> transition --- lib/elevator/application.ex | 2 +- lib/elevator/fsm/action.ex | 108 ----------------------------- lib/elevator/fsm/state.ex | 17 ++--- lib/elevator/fsm/transition.ex | 112 +++++++++++++++++++++++++++++++ lib/elevator/hardware/outputs.ex | 18 +---- 5 files changed, 121 insertions(+), 136 deletions(-) delete mode 100644 lib/elevator/fsm/action.ex create mode 100644 lib/elevator/fsm/transition.ex diff --git a/lib/elevator/application.ex b/lib/elevator/application.ex index ebc8890..d962326 100644 --- a/lib/elevator/application.ex +++ b/lib/elevator/application.ex @@ -11,7 +11,7 @@ defmodule Elevator.Application do {Elevator.HallOrders, Elevator.num_floors()}, Elevator.CabOrders, Elevator.FSM.State, - Elevator.FSM.Action, + Elevator.FSM.Transition, {Elevator.Hardware.Driver, [{127, 0, 0, 1}, driver_port]}, Elevator.Hardware.InputPoller ] diff --git a/lib/elevator/fsm/action.ex b/lib/elevator/fsm/action.ex deleted file mode 100644 index 48f4c50..0000000 --- a/lib/elevator/fsm/action.ex +++ /dev/null @@ -1,108 +0,0 @@ -defmodule Elevator.FSM.Action do - @moduledoc """ - Module updating the state based on occuring events. - """ - require Logger - - alias Elevator.CabOrders - alias Elevator.FSM.State - alias Elevator.HallOrders - alias Elevator.Decision - - @motor_timeout_ms 4000 - @action_interval_ms 100 - - def start_link(_arg) do - pid = spawn_link(fn -> poll_action() end) - - {:ok, pid} - end - - def child_spec(opts) do - %{ - id: __MODULE__, - start: {__MODULE__, :start_link, [opts]}, - type: :worker, - restart: :permanent, - shutdown: 500 - } - end - - defp poll_action() do - poll_door_timer() - check_motor_timeout() - decide_and_take_action() - Process.sleep(@action_interval_ms) - poll_action() - end - - # Helpers ---------------------------------------------------------- - - defp get_my_orders() do - hall_orders = HallOrders.get_my_orders() - pressed_cab_floors = CabOrders.get_my_orders() - Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) - end - - defp check_motor_timeout() do - state = State.get_state() - - timed_out = - case state.last_floor_time do - nil -> - false - - last_floor_time -> - Time.diff(Time.utc_now(), last_floor_time, :millisecond) > @motor_timeout_ms - end - - State.set_motor_timed_out(timed_out) - end - - @spec decide_and_take_action() :: any() - defp decide_and_take_action() do - state = State.get_state() - - if state.motor_timed_out do - :ok - else - orders = get_my_orders() - - {new_direction, new_behavior} = Decision.next_action(orders, state) - - cond do - state.behavior == :door_open -> - CabOrders.arrived_at_floor(state.floor) - HallOrders.arrived_at_floor(state.floor, new_direction) - - new_behavior == :door_open -> - State.open_door() - State.set_direction(new_direction) - - new_behavior == :moving -> - State.set_direction(new_direction) - State.set_behavior(new_behavior) - - new_behavior == :idle -> - State.set_direction(new_direction) - State.set_behavior(new_behavior) - end - end - end - - defp poll_door_timer() do - state = State.get_state() - - if state.behavior == :door_open and - Time.after?( - Time.utc_now(), - Time.add(state.door_open_time_ms, Elevator.door_open_duration_ms(), :millisecond) - ) do - if state.obstructed do - State.open_door() - else - State.set_behavior(:idle) - end - end - end -end diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index e0de1b7..9c8ed4f 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -3,7 +3,6 @@ defmodule Elevator.FSM.State do Module storing the elevator state. """ require Logger - alias Elevator.Hardware.Outputs alias Elevator.Types defstruct direction: :down, @@ -31,7 +30,7 @@ defmodule Elevator.FSM.State do @impl true def init(_arg) do state = %Elevator.FSM.State{} - {:ok, state, {:continue, :set_outputs}} + {:ok, state} end def start_link(_args) do @@ -72,7 +71,7 @@ defmodule Elevator.FSM.State do %{state | between_floors: false, floor: floor, last_floor_time: Time.utc_now()} end - {:noreply, new_state, {:continue, :set_outputs}} + {:noreply, new_state} end @impl true @@ -82,12 +81,12 @@ defmodule Elevator.FSM.State do @impl true def handle_cast({:set_direction, dir}, state) do - {:noreply, %{state | direction: dir}, {:continue, :set_outputs}} + {:noreply, %{state | direction: dir}} end @impl true def handle_cast({:set_behavior, behavior}, state) do - {:noreply, %{state | behavior: behavior}, {:continue, :set_outputs}} + {:noreply, %{state | behavior: behavior}} end @impl true @@ -99,13 +98,7 @@ defmodule Elevator.FSM.State do %{state | behavior: :door_open, door_open_time_ms: Time.utc_now()} end - {:noreply, new_state, {:continue, :set_outputs}} - end - - @impl true - def handle_continue(:set_outputs, state) do - Outputs.set_outputs(state) - {:noreply, state} + {:noreply, new_state} end # Calls ---------------------------------------- diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex new file mode 100644 index 0000000..8a6eb63 --- /dev/null +++ b/lib/elevator/fsm/transition.ex @@ -0,0 +1,112 @@ +defmodule Elevator.FSM.Transition do + @moduledoc """ + Loop handling FSM transitions. + One iteration of the loop does the following: + - Checks door and motor timeouts + - Reads and updates state and orders + - Sets hardware outputs + """ + require Logger + + alias Elevator.Types + alias Elevator.CabOrders + alias Elevator.FSM.State + alias Elevator.HallOrders + alias Elevator.Decision + alias Elevator.Hardware.Outputs + + @motor_timeout_ms 3500 + @transition_interval_ms 100 + + def start_link(_arg) do + pid = spawn_link(fn -> loop() end) + + {:ok, pid} + end + + def child_spec(opts) do + %{ + id: __MODULE__, + start: {__MODULE__, :start_link, [opts]}, + type: :worker, + restart: :permanent, + shutdown: 500 + } + end + + defp loop() do + check_door_timer(State.get_state()) + check_motor_timeout(State.get_state()) + decide_and_update_state(State.get_state(), get_my_orders()) + Outputs.set_outputs(State.get_state(), get_light_orders()) + + Process.sleep(@transition_interval_ms) + loop() + end + + # Helpers ---------------------------------------------------------- + + defp get_my_orders() do + hall_orders = HallOrders.get_my_orders() + pressed_cab_floors = CabOrders.get_my_orders() + Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) + end + + defp get_light_orders() do + hall_orders = HallOrders.get_confirmed_orders() + pressed_cab_floors = CabOrders.get_my_orders() + Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) + end + + @spec decide_and_update_state(Elevator.FSM.State.t(), Types.combined_order_map()) :: any() + defp decide_and_update_state(state, orders) when not state.motor_timed_out do + {new_direction, new_behavior} = Decision.next_action(orders, state) + + cond do + state.behavior == :door_open -> + CabOrders.arrived_at_floor(state.floor) + HallOrders.arrived_at_floor(state.floor, new_direction) + + new_behavior == :door_open -> + State.open_door() + State.set_direction(new_direction) + + new_behavior == :moving -> + State.set_direction(new_direction) + State.set_behavior(new_behavior) + + new_behavior == :idle -> + State.set_direction(new_direction) + State.set_behavior(new_behavior) + end + end + + defp decide_and_update_state(_state, _orders), do: :ok + + defp check_motor_timeout(state) do + timed_out = + case state.last_floor_time do + nil -> + false + + last_floor_time -> + Time.diff(Time.utc_now(), last_floor_time, :millisecond) > @motor_timeout_ms + end + + State.set_motor_timed_out(timed_out) + end + + defp check_door_timer(state) do + if state.behavior == :door_open and + Time.after?( + Time.utc_now(), + Time.add(state.door_open_time_ms, Elevator.door_open_duration_ms(), :millisecond) + ) do + if state.obstructed do + State.open_door() + else + State.set_behavior(:idle) + end + end + end +end diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index 72d763f..037d162 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -4,9 +4,6 @@ defmodule Elevator.Hardware.Outputs do """ require Logger - alias Elevator.CabOrders - alias Elevator.Decision - alias Elevator.HallOrders alias Elevator.Communicator alias Elevator.Hardware.Driver alias Elevator.Types @@ -18,8 +15,8 @@ defmodule Elevator.Hardware.Outputs do Driver.set_motor_direction(:stop) end - @spec set_outputs(FSM.State.t()) :: any() - def set_outputs(state) do + @spec set_outputs(FSM.State.t(), Types.combined_order_map()) :: any() + def set_outputs(state, light_orders) do set_door_light(state) set_motors(state) set_floor_light(state) @@ -28,10 +25,7 @@ defmodule Elevator.Hardware.Outputs do operational = not (door_blocked or state.motor_timed_out) Communicator.update_operation_status(operational) - Task.start(fn -> - orders = get_light_orders() - set_order_lights(orders) - end) + set_order_lights(light_orders) end defp set_motors(elev_state) do @@ -50,12 +44,6 @@ defmodule Elevator.Hardware.Outputs do end end - defp get_light_orders() do - hall_orders = HallOrders.get_confirmed_orders() - pressed_cab_floors = CabOrders.get_my_orders() - Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) - end - defp set_order_lights(orders) do for floor <- 0..(Elevator.num_floors() - 1), btn <- Types.btn_types() do lights = Map.get(orders, floor, MapSet.new()) From 7bd8a7f5142db78e590054785476a9f611999ae3 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 17:48:12 +0100 Subject: [PATCH 03/29] fix: obstruction handling --- lib/elevator/fsm/state.ex | 4 ++-- lib/elevator/fsm/transition.ex | 33 +++++++++++++++----------------- lib/elevator/hall_orders.ex | 13 +++++++++++++ lib/elevator/hall_orders/cost.ex | 11 +++++++++++ lib/elevator/hardware/outputs.ex | 3 +-- 5 files changed, 42 insertions(+), 22 deletions(-) diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 9c8ed4f..3363f84 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -12,7 +12,7 @@ defmodule Elevator.FSM.State do obstructed: false, motor_timed_out: false, door_open_time_ms: Time.utc_now(), - last_floor_time: nil + last_floor_time: Time.utc_now() @type t :: %__MODULE__{ direction: Types.elev_dir(), @@ -22,7 +22,7 @@ defmodule Elevator.FSM.State do obstructed: boolean(), motor_timed_out: boolean(), door_open_time_ms: Time.t(), - last_floor_time: Time.t() | nil + last_floor_time: Time.t() } use GenServer diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 8a6eb63..6da3d4f 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -84,29 +84,26 @@ defmodule Elevator.FSM.Transition do defp decide_and_update_state(_state, _orders), do: :ok defp check_motor_timeout(state) do - timed_out = - case state.last_floor_time do - nil -> - false - - last_floor_time -> - Time.diff(Time.utc_now(), last_floor_time, :millisecond) > @motor_timeout_ms - end - + timed_out = Time.diff(Time.utc_now(), state.last_floor_time, :millisecond) > @motor_timeout_ms State.set_motor_timed_out(timed_out) end - defp check_door_timer(state) do - if state.behavior == :door_open and - Time.after?( - Time.utc_now(), - Time.add(state.door_open_time_ms, Elevator.door_open_duration_ms(), :millisecond) - ) do - if state.obstructed do + defp check_door_timer(state) when state.behavior == :door_open do + timed_out = + Time.diff(Time.utc_now(), state.door_open_time_ms, :millisecond) > + Elevator.door_open_duration_ms() + + cond do + timed_out and state.obstructed -> State.open_door() - else + + timed_out -> State.set_behavior(:idle) - end + + true -> + :ok end end + + defp check_door_timer(_state), do: :ok end diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index fd1fce1..c760999 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -207,10 +207,23 @@ defmodule Elevator.HallOrders do alive = Communicator.who_can_serve() my_orders = my_orders_from_order_map(order_map, alive) + elev_state = Elevator.FSM.State.get_state() + {any_did_change, new_order_map} = Enum.reduce(order_map, {false, %{}}, fn {key, button_state}, {acc_did_change, acc_order_map} -> {did_change, new_button_state} = Order.update_hall_order(key, button_state, my_orders) + + new_button_state = + case new_button_state do + {:handling, cost_map} -> + new_cost_map = Cost.update_obstructed_cost(cost_map, elev_state) + {:handling, new_cost_map} + + _ -> + new_button_state + end + {acc_did_change or did_change, Map.put(acc_order_map, key, new_button_state)} end) diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 3ed1ac7..d36472b 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -5,6 +5,7 @@ defmodule Elevator.HallOrders.Cost do Cost is estimated by simulating the local elevator with current requests plus the candidate hall request. """ + alias Elevator.Communicator alias Elevator.CabOrders alias Elevator.Decision alias Elevator.FSM.State @@ -96,10 +97,20 @@ defmodule Elevator.HallOrders.Cost do end end + def update_obstructed_cost(cost_map, state) + when state.obstructed and state.behavior == :door_open do + Map.update!(cost_map, Communicator.my_id(), fn _ -> @unreachable_cost end) + end + + def update_obstructed_cost(cost_map, _state), do: cost_map + @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, %{obstructed: true, behavior: :door_open}, _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} diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index 037d162..3f83084 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -21,8 +21,7 @@ defmodule Elevator.Hardware.Outputs do set_motors(state) set_floor_light(state) - door_blocked = state.behavior == :door_open and state.obstructed - operational = not (door_blocked or state.motor_timed_out) + operational = not state.motor_timed_out Communicator.update_operation_status(operational) set_order_lights(light_orders) From 06e7fa3bbe4b9fbc68a5020613150e5bf8e12024 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 18:00:23 +0100 Subject: [PATCH 04/29] fix: obstructed only when door open --- lib/elevator/fsm/state.ex | 3 ++- lib/elevator/fsm/transition.ex | 4 +--- lib/elevator/hall_orders/cost.ex | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 3363f84..454ac08 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -75,7 +75,8 @@ defmodule Elevator.FSM.State do end @impl true - def handle_cast({:set_obstruction, obstructed}, state) do + def handle_cast({:set_obstruction, obstruction_switch}, state) do + obstructed = obstruction_switch and state.behavior == :door_open {:noreply, %{state | obstructed: obstructed}} end diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 6da3d4f..a2798e4 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -88,7 +88,7 @@ defmodule Elevator.FSM.Transition do State.set_motor_timed_out(timed_out) end - defp check_door_timer(state) when state.behavior == :door_open do + defp check_door_timer(state) do timed_out = Time.diff(Time.utc_now(), state.door_open_time_ms, :millisecond) > Elevator.door_open_duration_ms() @@ -104,6 +104,4 @@ defmodule Elevator.FSM.Transition do :ok end end - - defp check_door_timer(_state), do: :ok end diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index d36472b..344372e 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -98,7 +98,7 @@ defmodule Elevator.HallOrders.Cost do end def update_obstructed_cost(cost_map, state) - when state.obstructed and state.behavior == :door_open do + when state.obstructed do Map.update!(cost_map, Communicator.my_id(), fn _ -> @unreachable_cost end) end @@ -108,7 +108,7 @@ defmodule Elevator.HallOrders.Cost do non_neg_integer() defp simulate_cost_until_served(_orders, %{floor: :unknown}, _target), do: @unreachable_cost - defp simulate_cost_until_served(_orders, %{obstructed: true, behavior: :door_open}, _target), + defp simulate_cost_until_served(_orders, %{obstructed: true}, _target), do: @unreachable_cost defp simulate_cost_until_served(orders, state, target) do From 67817458d2e5e3ae87803335bec41626fdfd28d2 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 18:10:02 +0100 Subject: [PATCH 05/29] fix: differentiate between who is alive and who can serve --- lib/elevator/communicator.ex | 20 ++++++++++++++++++++ lib/elevator/hall_orders.ex | 16 ++-------------- lib/elevator/hall_orders/cost.ex | 7 ------- lib/elevator/hall_orders/order.ex | 2 +- lib/elevator/hardware/outputs.ex | 2 +- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 9ac3398..a897ace 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -59,6 +59,10 @@ defmodule Elevator.Communicator do GenServer.call(__MODULE__, :who_can_serve) end + def who_is_alive do + GenServer.call(__MODULE__, :who_is_alive) + end + @doc """ Updates the `operational` part of the state. Signals to peers whether this node can serve orders. @@ -150,6 +154,22 @@ defmodule Elevator.Communicator do {:reply, operational_nodes, state} end + @impl true + def handle_call(:who_is_alive, _from, state) do + cutoff_ms = Elevator.msg_cutoff_ms() + + communicating_nodes = + state.connected_nodes + |> Map.filter(fn {_k, %{timestamp: timestamp}} -> + Time.diff(Time.utc_now(), timestamp, :millisecond) < cutoff_ms + end) + |> Map.keys() + |> MapSet.new() + + communicating_nodes = MapSet.put(communicating_nodes, my_id()) + {:reply, communicating_nodes, state} + end + # --- Handle casts --- @doc """ diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index c760999..8cd7f81 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -126,8 +126,8 @@ defmodule Elevator.HallOrders do @spec handle_cast({:receive_state, hall_order_map()}, hall_order_map()) :: {:noreply, hall_order_map(), {:continue, :hall_update_state}} def handle_cast({:receive_state, other_order_map}, order_map) do - alive = Communicator.who_can_serve() - my_orders = my_orders_from_order_map(order_map, alive) + who_can_serve = Communicator.who_can_serve() + my_orders = my_orders_from_order_map(order_map, who_can_serve) new_order_map = Map.keys(order_map) @@ -207,23 +207,11 @@ defmodule Elevator.HallOrders do alive = Communicator.who_can_serve() my_orders = my_orders_from_order_map(order_map, alive) - elev_state = Elevator.FSM.State.get_state() - {any_did_change, new_order_map} = Enum.reduce(order_map, {false, %{}}, fn {key, button_state}, {acc_did_change, acc_order_map} -> {did_change, new_button_state} = Order.update_hall_order(key, button_state, my_orders) - new_button_state = - case new_button_state do - {:handling, cost_map} -> - new_cost_map = Cost.update_obstructed_cost(cost_map, elev_state) - {:handling, new_cost_map} - - _ -> - new_button_state - end - {acc_did_change or did_change, Map.put(acc_order_map, key, new_button_state)} end) diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 344372e..93cc8d2 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -97,13 +97,6 @@ defmodule Elevator.HallOrders.Cost do end end - def update_obstructed_cost(cost_map, state) - when state.obstructed do - Map.update!(cost_map, Communicator.my_id(), fn _ -> @unreachable_cost end) - end - - def update_obstructed_cost(cost_map, _state), do: cost_map - @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 diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index e2c1f01..9737baf 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -69,7 +69,7 @@ defmodule Elevator.HallOrders.Order do Types.floor() => MapSet.t(Types.hall_btn()) }) :: {boolean(), hall_order_state()} def update_hall_order(order_key, order_state, confirmed_hall_orders) do - alive = Communicator.who_can_serve() + alive = Communicator.who_is_alive() {did_change, new_state} = case order_state do diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index 3f83084..f5186d8 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -21,7 +21,7 @@ defmodule Elevator.Hardware.Outputs do set_motors(state) set_floor_light(state) - operational = not state.motor_timed_out + operational = not (state.motor_timed_out or state.obstructed) Communicator.update_operation_status(operational) set_order_lights(light_orders) From 7ea42094a39efca92e27c149ae064e75e5767ee5 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 18:30:53 +0100 Subject: [PATCH 06/29] fix: don't go to idle if door not open --- lib/elevator/fsm/state.ex | 12 ++++++------ lib/elevator/fsm/transition.ex | 4 +++- lib/elevator/hall_orders/cost.ex | 1 - 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 454ac08..c6434e5 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -6,7 +6,7 @@ defmodule Elevator.FSM.State do alias Elevator.Types defstruct direction: :down, - behavior: :idle, + behavior: :moving, floor: :unknown, between_floors: true, obstructed: false, @@ -54,7 +54,7 @@ defmodule Elevator.FSM.State do def open_door(), do: GenServer.cast(__MODULE__, :open_door) def set_motor_timed_out(timed_out), - do: GenServer.call(__MODULE__, {:set_motor_timed_out, timed_out}) + do: GenServer.cast(__MODULE__, {:set_motor_timed_out, timed_out}) def get_state(), do: GenServer.call(__MODULE__, :get_state) @@ -102,14 +102,14 @@ defmodule Elevator.FSM.State do {:noreply, new_state} end - # Calls ---------------------------------------- - @impl true - def handle_call({:set_motor_timed_out, timed_out}, _from, state) do + def handle_cast({:set_motor_timed_out, timed_out}, state) do new_state = %{state | motor_timed_out: timed_out} - {:reply, :ok, new_state} + {:noreply, new_state} end + # Calls ---------------------------------------- + @impl true def handle_call(:get_state, _from, state) do {:reply, state, state} diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index a2798e4..215edea 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -88,7 +88,7 @@ defmodule Elevator.FSM.Transition do State.set_motor_timed_out(timed_out) end - defp check_door_timer(state) do + defp check_door_timer(state) when state.behavior == :door_open do timed_out = Time.diff(Time.utc_now(), state.door_open_time_ms, :millisecond) > Elevator.door_open_duration_ms() @@ -104,4 +104,6 @@ defmodule Elevator.FSM.Transition do :ok end end + + defp check_door_timer(_), do: :ok end diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 93cc8d2..5e2065c 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -5,7 +5,6 @@ defmodule Elevator.HallOrders.Cost do Cost is estimated by simulating the local elevator with current requests plus the candidate hall request. """ - alias Elevator.Communicator alias Elevator.CabOrders alias Elevator.Decision alias Elevator.FSM.State From c56d88b7b443ba39d0b8072caac34a194abeb1f0 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 18:37:41 +0100 Subject: [PATCH 07/29] chore; update state moddoc --- lib/elevator/fsm/state.ex | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index c6434e5..cebd324 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -1,6 +1,9 @@ defmodule Elevator.FSM.State do @moduledoc """ - Module storing the elevator state. + GenServer holding the physical state of the elevator. + + Acts as the single source of truth for what the elevator *is* right now - + its floor, direction, behavior, and fault conditions. """ require Logger alias Elevator.Types From 7c321b7b66643dfdf85f73bc66facd0010f31312 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 18:52:42 +0100 Subject: [PATCH 08/29] chore: add missing specs --- lib/elevator.ex | 4 ++++ lib/elevator/application.ex | 1 + lib/elevator/cab_orders.ex | 1 + lib/elevator/communicator.ex | 2 ++ lib/elevator/decision.ex | 2 ++ lib/elevator/fsm/state.ex | 17 +++++++++++++++-- lib/elevator/fsm/transition.ex | 2 ++ lib/elevator/hall_orders.ex | 1 + lib/elevator/hardware/input_poller.ex | 1 + lib/elevator/hardware/outputs.ex | 1 + 10 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index cc37a40..e416c90 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -4,18 +4,22 @@ defmodule Elevator do @msg_cutoff_ms 10000 @door_open_duration_ms 1000 + @spec num_floors() :: pos_integer() def num_floors do @num_floors end + @spec door_open_duration_ms() :: pos_integer() def door_open_duration_ms do @door_open_duration_ms end + @spec resend_period_ms() :: pos_integer() def resend_period_ms do @resend_period_ms end + @spec msg_cutoff_ms() :: pos_integer() def msg_cutoff_ms do @msg_cutoff_ms end diff --git a/lib/elevator/application.ex b/lib/elevator/application.ex index d962326..57e7552 100644 --- a/lib/elevator/application.ex +++ b/lib/elevator/application.ex @@ -1,6 +1,7 @@ defmodule Elevator.Application do use Application + @spec start(Application.start_type(), term()) :: {:ok, pid()} | {:error, term()} def start(_start_type, _start_args) do topologies = Application.fetch_env!(:libcluster, :topologies) driver_port = Application.fetch_env!(:elevator, :driver_port) diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index d486a4b..0fd4e34 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -8,6 +8,7 @@ defmodule Elevator.CabOrders do @type state_t :: Elevator.Types.cab_order_map() @type floor_t :: Elevator.Types.floor() + @spec start_link(any()) :: GenServer.on_start() def start_link(arg) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) end diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index a897ace..1772aa0 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -17,6 +17,7 @@ defmodule Elevator.Communicator do @type communicator_options :: [do_resend: boolean(), do_logging: boolean()] + @spec start_link(communicator_options()) :: GenServer.on_start() def start_link(arg \\ [do_resend: true, do_logging: false]) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) end @@ -59,6 +60,7 @@ defmodule Elevator.Communicator do GenServer.call(__MODULE__, :who_can_serve) end + @spec who_is_alive() :: MapSet.t(node_id_t()) def who_is_alive do GenServer.call(__MODULE__, :who_is_alive) end diff --git a/lib/elevator/decision.ex b/lib/elevator/decision.ex index 9ce5508..dc898d6 100644 --- a/lib/elevator/decision.ex +++ b/lib/elevator/decision.ex @@ -5,10 +5,12 @@ defmodule Elevator.Decision do These functions are intentionally pure to make them easy to unit test. """ + @spec requests_above?(Elevator.Types.combined_order_map(), Elevator.Types.floor()) :: boolean() def requests_above?(reqs, floor) do Enum.any?(reqs, fn {f, _} -> f > floor end) end + @spec requests_below?(Elevator.Types.combined_order_map(), Elevator.Types.floor()) :: boolean() def requests_below?(reqs, floor) do Enum.any?(reqs, fn {f, _} -> f < floor end) end diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index cebd324..6b9d8d8 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -32,33 +32,46 @@ defmodule Elevator.FSM.State do @impl true def init(_arg) do - state = %Elevator.FSM.State{} + state = %__MODULE__{} {:ok, state} end + @spec start_link(any()) :: GenServer.on_start() def start_link(_args) do GenServer.start_link(__MODULE__, %{}, name: __MODULE__) end # User API ---------------------------------------- + @spec set_floor(:between_floors | Types.floor()) :: :ok def set_floor(floor), do: GenServer.cast(__MODULE__, {:set_floor, floor}) - def set_obstruction(obstructed), do: GenServer.cast(__MODULE__, {:set_obstruction, obstructed}) + @doc """ + Sets obstructed to true if door is open and obstruction switch is on. + Otherwise, obstructed is set to false. + """ + @spec set_obstruction(boolean()) :: :ok + def set_obstruction(obstruction_switch), + do: GenServer.cast(__MODULE__, {:set_obstruction, obstruction_switch}) + @spec set_direction(Types.elev_dir()) :: :ok def set_direction(dir), do: GenServer.cast(__MODULE__, {:set_direction, dir}) + @spec set_behavior(Types.elev_behavior()) :: :ok 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. """ + @spec open_door() :: :ok def open_door(), do: GenServer.cast(__MODULE__, :open_door) + @spec set_motor_timed_out(boolean()) :: :ok def set_motor_timed_out(timed_out), do: GenServer.cast(__MODULE__, {:set_motor_timed_out, timed_out}) + @spec get_state() :: t() def get_state(), do: GenServer.call(__MODULE__, :get_state) # Casts ---------------------------------------- diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 215edea..0b8224e 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -18,12 +18,14 @@ defmodule Elevator.FSM.Transition do @motor_timeout_ms 3500 @transition_interval_ms 100 + @spec start_link(any()) :: {:ok, pid()} def start_link(_arg) do pid = spawn_link(fn -> loop() end) {:ok, pid} end + @spec child_spec(any()) :: Supervisor.child_spec() def child_spec(opts) do %{ id: __MODULE__, diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 8cd7f81..e8d580e 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -18,6 +18,7 @@ defmodule Elevator.HallOrders do @hall_order_refresh_period_ms 1000 + @spec start_link(any()) :: GenServer.on_start() def start_link(arg) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) end diff --git a/lib/elevator/hardware/input_poller.ex b/lib/elevator/hardware/input_poller.ex index d1ee06f..0018de8 100644 --- a/lib/elevator/hardware/input_poller.ex +++ b/lib/elevator/hardware/input_poller.ex @@ -15,6 +15,7 @@ defmodule Elevator.Hardware.InputPoller do @obstruction_poll_interval_ms 500 # Public API + @spec start_link(any()) :: GenServer.on_start() def start_link(_opts) do GenServer.start_link(__MODULE__, %{}, name: __MODULE__) end diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index f5186d8..645e6ad 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -9,6 +9,7 @@ defmodule Elevator.Hardware.Outputs do alias Elevator.Types alias Elevator.FSM + @spec init() :: :ok def init() do Driver.set_stop_button_light(:off) Driver.set_door_open_light(:off) From 297df9a8d2fb8285b4fa7788b16761b2ead2e103 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 18:56:40 +0100 Subject: [PATCH 09/29] chore: standardize separator comments --- lib/elevator/cab_orders.ex | 4 ++-- lib/elevator/communicator.ex | 4 ++-- lib/elevator/fsm/state.ex | 6 +++--- lib/elevator/fsm/transition.ex | 2 +- lib/elevator/hall_orders.ex | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index 0fd4e34..ae72900 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -51,7 +51,7 @@ defmodule Elevator.CabOrders do GenServer.cast(__MODULE__, {:arrived_at_floor, floor}) end - # --- Handle calls --- + # Calls -------------------------------------------------- @impl true def handle_call(:get_my_orders, _from, state) do orders = state[Communicator.my_id()].orders @@ -63,7 +63,7 @@ defmodule Elevator.CabOrders do {:reply, state, state} end - # --- Handle casts --- + # Casts -------------------------------------------------- @impl true @spec handle_cast({:receive_state, state_t()}, state_t()) :: {:noreply, state_t()} diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 1772aa0..341a235 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -132,7 +132,7 @@ defmodule Elevator.Communicator do {:noreply, state} end - # --- Handle calls --- + # Calls -------------------------------------------------- @impl true def handle_call(:who_can_serve, _from, state) do @@ -172,7 +172,7 @@ defmodule Elevator.Communicator do {:reply, communicating_nodes, state} end - # --- Handle casts --- + # Casts -------------------------------------------------- @doc """ Sends received hall and cab orders to respective modules, and updates timestamps for when the connected nodes last sent something. diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 6b9d8d8..342f186 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -41,7 +41,7 @@ defmodule Elevator.FSM.State do GenServer.start_link(__MODULE__, %{}, name: __MODULE__) end - # User API ---------------------------------------- + # User API -------------------------------------------------- @spec set_floor(:between_floors | Types.floor()) :: :ok def set_floor(floor), do: GenServer.cast(__MODULE__, {:set_floor, floor}) @@ -74,7 +74,7 @@ defmodule Elevator.FSM.State do @spec get_state() :: t() def get_state(), do: GenServer.call(__MODULE__, :get_state) - # Casts ---------------------------------------- + # Casts -------------------------------------------------- @impl true def handle_cast({:set_floor, floor}, state) do @@ -124,7 +124,7 @@ defmodule Elevator.FSM.State do {:noreply, new_state} end - # Calls ---------------------------------------- + # Calls -------------------------------------------------- @impl true def handle_call(:get_state, _from, state) do diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 0b8224e..83993cc 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -46,7 +46,7 @@ defmodule Elevator.FSM.Transition do loop() end - # Helpers ---------------------------------------------------------- + # Helpers -------------------------------------------------- defp get_my_orders() do hall_orders = HallOrders.get_my_orders() diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index e8d580e..3074f88 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -92,7 +92,7 @@ defmodule Elevator.HallOrders do GenServer.call(__MODULE__, :get_confirmed_orders) end - # Calls ------------------------------------------------------------ + # Calls -------------------------------------------------- @impl true def handle_call(:get_my_orders, _from, order_map) do @@ -121,7 +121,7 @@ defmodule Elevator.HallOrders do {:reply, order_map, order_map} end - # Casts ------------------------------------------------------------ + # Casts -------------------------------------------------- @impl true @spec handle_cast({:receive_state, hall_order_map()}, hall_order_map()) :: @@ -195,7 +195,7 @@ defmodule Elevator.HallOrders do {:noreply, order_map, {:continue, :hall_update_state}} end - # Continue ------------------------------------------------------------ + # Continue -------------------------------------------------- @doc """ May advance some states, in which case continue is called until convergence. From b8ab58866b5753dc544c5cb865182093b35da5a3 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 19:31:42 +0100 Subject: [PATCH 10/29] chore: remove communicator my_id --- lib/elevator.ex | 6 ---- lib/elevator/cab_orders.ex | 8 ++--- lib/elevator/communicator.ex | 46 +++++++++++---------------- lib/elevator/fsm/state.ex | 3 ++ lib/elevator/hall_orders.ex | 12 +++---- lib/elevator/hall_orders/order.ex | 8 ++--- lib/elevator/hardware/input_poller.ex | 33 ++++++------------- lib/elevator/types.ex | 8 ++--- test/single/cab_orders_test.exs | 10 +++--- test/utils/test_compiled.ex | 2 +- 10 files changed, 54 insertions(+), 82 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index e416c90..6b30d62 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -1,6 +1,5 @@ defmodule Elevator do @num_floors 4 - @resend_period_ms 50 @msg_cutoff_ms 10000 @door_open_duration_ms 1000 @@ -14,11 +13,6 @@ defmodule Elevator do @door_open_duration_ms end - @spec resend_period_ms() :: pos_integer() - def resend_period_ms do - @resend_period_ms - end - @spec msg_cutoff_ms() :: pos_integer() def msg_cutoff_ms do @msg_cutoff_ms diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index ae72900..28283a7 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -16,7 +16,7 @@ defmodule Elevator.CabOrders do @impl true @spec init(any()) :: {:ok, state_t()} def init(_arg \\ []) do - state = %{Communicator.my_id() => %{version: 0, orders: MapSet.new()}} + state = %{Node.self() => %{version: 0, orders: MapSet.new()}} {:ok, state} end @@ -54,7 +54,7 @@ defmodule Elevator.CabOrders do # Calls -------------------------------------------------- @impl true def handle_call(:get_my_orders, _from, state) do - orders = state[Communicator.my_id()].orders + orders = state[Node.self()].orders {:reply, orders, state} end @@ -86,7 +86,7 @@ defmodule Elevator.CabOrders do @spec handle_cast({:button_press, floor_t()}, state_t()) :: {:noreply, state_t()} def handle_cast({:button_press, floor}, state) do new_state = - Map.update!(state, Communicator.my_id(), fn %{version: old_version, orders: old_orders} -> + Map.update!(state, Node.self(), fn %{version: old_version, orders: old_orders} -> %{version: old_version + 1, orders: MapSet.put(old_orders, floor)} end) @@ -97,7 +97,7 @@ defmodule Elevator.CabOrders do @spec handle_cast({:arrived_at_floor, floor_t()}, state_t()) :: {:noreply, state_t()} def handle_cast({:arrived_at_floor, floor}, state) do new_state = - Map.update!(state, Communicator.my_id(), fn %{version: old_version, orders: old_orders} -> + Map.update!(state, Node.self(), fn %{version: old_version, orders: old_orders} -> %{version: old_version + 1, orders: MapSet.delete(old_orders, floor)} end) diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 341a235..4441c9b 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -10,13 +10,14 @@ defmodule Elevator.Communicator do require Logger use GenServer - @type node_id_t :: Elevator.Types.node_id() @type hall_orders_t :: Elevator.Types.hall_order_map() @type cab_orders_t :: Elevator.Types.cab_order_map() @type state_t :: Elevator.Types.communicator_state_map() @type communicator_options :: [do_resend: boolean(), do_logging: boolean()] + @resend_period_ms 50 + @spec start_link(communicator_options()) :: GenServer.on_start() def start_link(arg \\ [do_resend: true, do_logging: false]) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) @@ -43,49 +44,38 @@ defmodule Elevator.Communicator do {:ok, state} end - @doc """ - Returns the ID of this node. - """ - @spec my_id() :: node_id_t() - def my_id, do: Node.self() - @doc """ Returns a set of alive nodes that are both: a) Connected AND b) Have sent a message within the cutoff period """ - @spec who_can_serve() :: MapSet.t() - def who_can_serve do - GenServer.call(__MODULE__, :who_can_serve) - end + @spec who_can_serve() :: MapSet.t(Node.t()) + def who_can_serve, do: GenServer.call(__MODULE__, :who_can_serve) - @spec who_is_alive() :: MapSet.t(node_id_t()) - def who_is_alive do - GenServer.call(__MODULE__, :who_is_alive) - end + @spec who_is_alive() :: MapSet.t(Node.t()) + def who_is_alive, do: GenServer.call(__MODULE__, :who_is_alive) @doc """ 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 - GenServer.cast(__MODULE__, {:update_operation_status, status}) - end + def update_operation_status(status), + do: GenServer.cast(__MODULE__, {:update_operation_status, status}) # Updates the timestamp when a message is received from a node - @spec update_state_map(state_t(), node_id_t(), boolean()) :: state_t() + @spec update_state_map(state_t(), Node.t(), boolean()) :: state_t() defp update_state_map(state, from_node, operational) do from_node_map = %{operational: operational, timestamp: Time.utc_now()} %{state | connected_nodes: Map.put(state.connected_nodes, from_node, from_node_map)} end # Schedules another round of state broadcasting. - defp schedule_state_broadcast do - time_ms = Elevator.resend_period_ms() - Process.send_after(self(), :broadcast_state, time_ms) - end + defp schedule_state_broadcast, + do: Process.send_after(self(), :broadcast_state, @resend_period_ms) + + # Infos -------------------------------------------------- @doc """ Sends the cab and hall state to all connected nodes. @@ -103,7 +93,7 @@ defmodule Elevator.Communicator do Node.list(:connected) |> GenServer.abcast( __MODULE__, - {:state_update, my_id(), state.operational, hall_state, cab_state} + {:state_update, Node.self(), state.operational, hall_state, cab_state} ) end) end @@ -126,7 +116,7 @@ defmodule Elevator.Communicator do @impl true def handle_info(:log_debug, state) do Process.send_after(self(), :log_debug, 1000) - Logger.debug("My id: #{my_id()}") + Logger.debug("My id: #{Node.self()}") others = who_can_serve() |> Enum.map(fn node -> "#{node}" end) |> Enum.join(", ") Logger.debug("Others: #{others}") {:noreply, state} @@ -148,7 +138,7 @@ defmodule Elevator.Communicator do operational_nodes = if state.operational do - MapSet.put(communicating_nodes, my_id()) + MapSet.put(communicating_nodes, Node.self()) else communicating_nodes end @@ -168,7 +158,7 @@ defmodule Elevator.Communicator do |> Map.keys() |> MapSet.new() - communicating_nodes = MapSet.put(communicating_nodes, my_id()) + communicating_nodes = MapSet.put(communicating_nodes, Node.self()) {:reply, communicating_nodes, state} end @@ -179,7 +169,7 @@ defmodule Elevator.Communicator do """ @impl true @spec handle_cast( - {:state_update, node_id_t(), boolean(), hall_orders_t(), cab_orders_t()}, + {:state_update, Node.t(), boolean(), hall_orders_t(), cab_orders_t()}, state_t() ) :: {:noreply, state_t()} diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 342f186..eec8223 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -43,6 +43,9 @@ defmodule Elevator.FSM.State do # User API -------------------------------------------------- + @doc """ + Updates floor and between_floors status. + """ @spec set_floor(:between_floors | Types.floor()) :: :ok def set_floor(floor), do: GenServer.cast(__MODULE__, {:set_floor, floor}) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 3074f88..0ece3d3 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -153,7 +153,7 @@ defmodule Elevator.HallOrders do new_order_map = case old_order_state do :idle -> - Map.put(order_map, key, {:pending, MapSet.new([Communicator.my_id()])}) + Map.put(order_map, key, {:pending, MapSet.new([Node.self()])}) _ -> order_map @@ -180,7 +180,7 @@ defmodule Elevator.HallOrders do new_order_map = case order_state do {:handling, _} -> - Map.put(order_map, key, {:arrived, MapSet.new([Communicator.my_id()])}) + Map.put(order_map, key, {:arrived, MapSet.new([Node.self()])}) _ -> order_map @@ -195,7 +195,7 @@ defmodule Elevator.HallOrders do {:noreply, order_map, {:continue, :hall_update_state}} end - # Continue -------------------------------------------------- + # Continues -------------------------------------------------- @doc """ May advance some states, in which case continue is called until convergence. @@ -227,13 +227,13 @@ defmodule Elevator.HallOrders do Enum.filter(order_map, fn {_, order_state} -> case order_state do {:handling, cost_map} -> - if Cost.min_alive_cost(cost_map, alive) == Communicator.my_id() do + if Cost.min_alive_cost(cost_map, alive) == Node.self() do Logger.debug( - "\nCost map: #{inspect(cost_map)}\nAlive: #{inspect(alive)}\nI (#{inspect(Communicator.my_id())}) am the one to serve" + "\nCost map: #{inspect(cost_map)}\nAlive: #{inspect(alive)}\nI (#{inspect(Node.self())}) am the one to serve" ) end - Cost.min_alive_cost(cost_map, alive) == Communicator.my_id() + Cost.min_alive_cost(cost_map, alive) == Node.self() _ -> false diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index 9737baf..3c3242b 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -32,10 +32,10 @@ defmodule Elevator.HallOrders.Order do new_order_state = case new_order_state do {:pending, barrier_set} -> - {:pending, MapSet.put(barrier_set, Communicator.my_id())} + {:pending, MapSet.put(barrier_set, Node.self())} {:arrived, barrier_set} -> - {:arrived, MapSet.put(barrier_set, Communicator.my_id())} + {:arrived, MapSet.put(barrier_set, Node.self())} _ -> new_order_state @@ -45,7 +45,7 @@ defmodule Elevator.HallOrders.Order do new_order_state = case new_order_state do {:handling, cost_map} -> - my_id = Communicator.my_id() + my_id = Node.self() if not Map.has_key?(cost_map, my_id) do {:handling, Map.put(cost_map, my_id, Cost.compute_cost(order_key, my_hall_orders))} @@ -76,7 +76,7 @@ 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, {:handling, %{Communicator.my_id() => my_cost}}} + {true, {:handling, %{Node.self() => my_cost}}} else {false, order_state} end diff --git a/lib/elevator/hardware/input_poller.ex b/lib/elevator/hardware/input_poller.ex index 0018de8..496c368 100644 --- a/lib/elevator/hardware/input_poller.ex +++ b/lib/elevator/hardware/input_poller.ex @@ -1,6 +1,7 @@ defmodule Elevator.Hardware.InputPoller do @moduledoc """ - Polls buttons, floor sensor and external order updates, casts to stateful modules when new info + Polls buttons, floor sensor and obstruction switch, + casts to stateful modules when new data arrives from the driver. """ use GenServer @@ -14,28 +15,25 @@ defmodule Elevator.Hardware.InputPoller do @button_poll_interval_ms 20 @obstruction_poll_interval_ms 500 - # Public API @spec start_link(any()) :: GenServer.on_start() def start_link(_opts) do GenServer.start_link(__MODULE__, %{}, name: __MODULE__) end - # GenServer callbacks @impl true def init(_state) do schedule_button_poll() schedule_floor_poll() schedule_obstruction_poll() - {:ok, %{prev_buttons: MapSet.new()}} + {:ok, nil} end @impl true def handle_info(:poll_floor, state) do schedule_floor_poll() - floor = Driver.get_floor_sensor_state() - Elevator.FSM.State.set_floor(floor) + Elevator.FSM.State.set_floor(Driver.get_floor_sensor_state()) {:noreply, state} end @@ -44,9 +42,8 @@ defmodule Elevator.Hardware.InputPoller do def handle_info(:poll_obstruction, state) do schedule_obstruction_poll() - switch_state = Driver.get_obstruction_switch_state() - obstructed = switch_state == :active - Elevator.FSM.State.set_obstruction(obstructed) + obstruction_switch = Driver.get_obstruction_switch_state() == :active + Elevator.FSM.State.set_obstruction(obstruction_switch) {:noreply, state} end @@ -56,17 +53,7 @@ defmodule Elevator.Hardware.InputPoller do schedule_button_poll() # Polls button and notifies Cab- and HallOrders if any are pressed - current_buttons = - for floor <- 0..(Elevator.num_floors() - 1), - btn <- get_pressed_buttons_at_floor(floor), - into: MapSet.new() do - {floor, btn} - end - - # Only notify on new presses (in current but not in previous) - new_presses = MapSet.difference(current_buttons, state.prev_buttons) - - Enum.each(new_presses, fn {floor, btn} -> + for floor <- 0..(Elevator.num_floors() - 1), btn <- get_pressed_buttons_at_floor(floor) do case btn do :cab -> CabOrders.button_press(floor) @@ -74,12 +61,12 @@ defmodule Elevator.Hardware.InputPoller do hall_btn -> HallOrders.button_press(floor, hall_btn) end - end) + end - {:noreply, %{state | prev_buttons: current_buttons}} + {:noreply, state} end - # Helpers + # Helpers -------------------------------------------------- defp get_pressed_buttons_at_floor(floor) do Elevator.Types.btn_types() diff --git a/lib/elevator/types.ex b/lib/elevator/types.ex index e92428c..b9f73a7 100644 --- a/lib/elevator/types.ex +++ b/lib/elevator/types.ex @@ -14,8 +14,6 @@ defmodule Elevator.Types do @type btn_type :: :cab | hall_btn() - @type node_id :: Node.t() - @spec btn_types() :: [btn_type()] def btn_types(), do: [:hall_up, :hall_down, :cab] @@ -24,7 +22,7 @@ defmodule Elevator.Types do @type hall_order_state :: :idle | {:pending, MapSet.t()} - | {:handling, %{node_id() => integer()}} + | {:handling, %{Node.t() => integer()}} | {:arrived, MapSet.t()} @type hall_order_map :: %{ @@ -37,7 +35,7 @@ defmodule Elevator.Types do } @type cab_order_map :: %{ - node_id() => cab_orders_snapshot() + Node.t() => cab_orders_snapshot() } @type combined_order_map :: %{ @@ -46,6 +44,6 @@ defmodule Elevator.Types do @type communicator_state_map :: %{ operational: boolean(), - connected_nodes: %{node_id() => %{operational: boolean(), timestamp: Time.t()}} + connected_nodes: %{Node.t() => %{operational: boolean(), timestamp: Time.t()}} } end diff --git a/test/single/cab_orders_test.exs b/test/single/cab_orders_test.exs index 867bf06..f70b0d4 100644 --- a/test/single/cab_orders_test.exs +++ b/test/single/cab_orders_test.exs @@ -28,7 +28,7 @@ defmodule Test.Single.CabOrdersTest do {:ok, state} = CabOrders.init() state = - Map.update(state, Communicator.my_id(), Communicator.my_id(), fn _old -> + Map.update(state, Node.self(), Node.self(), fn _old -> %{version: 1, orders: MapSet.new([1])} end) @@ -49,12 +49,12 @@ defmodule Test.Single.CabOrdersTest do test "version number increments correctly" do {:ok, state} = CabOrders.init() - assert state[Communicator.my_id()].version == 0 + assert state[Node.self()].version == 0 assert {:noreply, state} = CabOrders.handle_cast({:arrived_at_floor, 1}, state) - assert state[Communicator.my_id()].version == 1 + assert state[Node.self()].version == 1 assert {:noreply, state} = CabOrders.handle_cast({:button_press, 1}, state) - assert state[Communicator.my_id()].version == 2 + assert state[Node.self()].version == 2 assert {:noreply, state} = CabOrders.handle_cast({:button_press, 1}, state) - assert state[Communicator.my_id()].version == 3 + assert state[Node.self()].version == 3 end end diff --git a/test/utils/test_compiled.ex b/test/utils/test_compiled.ex index 503f51c..a464224 100644 --- a/test/utils/test_compiled.ex +++ b/test/utils/test_compiled.ex @@ -3,7 +3,7 @@ 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 convergence_wait_ms, do: 150 def start_order_modules(num_floors, do_resend) do children = [ From c2e24cbc9824fa4844045857acaff867352263c3 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 20:03:48 +0100 Subject: [PATCH 11/29] chore: optimise communicator --- lib/elevator.ex | 6 -- lib/elevator/cab_orders.ex | 10 +-- lib/elevator/communicator.ex | 137 ++++++++++++++++------------------- lib/elevator/hall_orders.ex | 9 ++- lib/elevator/types.ex | 7 ++ 5 files changed, 79 insertions(+), 90 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index 6b30d62..7c9a298 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -1,6 +1,5 @@ defmodule Elevator do @num_floors 4 - @msg_cutoff_ms 10000 @door_open_duration_ms 1000 @spec num_floors() :: pos_integer() @@ -12,9 +11,4 @@ defmodule Elevator do def door_open_duration_ms do @door_open_duration_ms end - - @spec msg_cutoff_ms() :: pos_integer() - def msg_cutoff_ms do - @msg_cutoff_ms - end end diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index 28283a7..28b06ef 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -36,9 +36,9 @@ defmodule Elevator.CabOrders do GenServer.call(__MODULE__, :get_my_orders) end - @spec receive_state(state_t()) :: :ok - def receive_state(other_state) do - GenServer.cast(__MODULE__, {:receive_state, other_state}) + @spec receive_external(state_t()) :: :ok + def receive_external(other_order_map) do + GenServer.cast(__MODULE__, {:receive_external, other_order_map}) end @spec button_press(floor_t()) :: :ok @@ -66,8 +66,8 @@ defmodule Elevator.CabOrders do # Casts -------------------------------------------------- @impl true - @spec handle_cast({:receive_state, state_t()}, state_t()) :: {:noreply, state_t()} - def handle_cast({:receive_state, other_state}, state) do + @spec handle_cast({:receive_external, state_t()}, state_t()) :: {:noreply, state_t()} + def handle_cast({:receive_external, other_state}, state) do new_state = Enum.reduce(other_state, state, fn {node_id, received}, acc -> current = Map.get(state, node_id, %{version: -1, orders: MapSet.new()}) diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 4441c9b..9a52730 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -10,27 +10,23 @@ defmodule Elevator.Communicator do require Logger use GenServer - @type hall_orders_t :: Elevator.Types.hall_order_map() - @type cab_orders_t :: Elevator.Types.cab_order_map() - @type state_t :: Elevator.Types.communicator_state_map() + @type state_map :: Elevator.Types.communicator_state_map() - @type communicator_options :: [do_resend: boolean(), do_logging: boolean()] + @type communicator_message :: Elevator.Types.communicator_message() + @type communicator_options :: [do_resend: boolean()] @resend_period_ms 50 + @msg_cutoff_ms 10000 @spec start_link(communicator_options()) :: GenServer.on_start() - def start_link(arg \\ [do_resend: true, do_logging: false]) do + def start_link(arg \\ [do_resend: true]) do GenServer.start_link(__MODULE__, arg, name: __MODULE__) end @impl true - def init(opts \\ [do_resend: true, do_logging: false]) do + def init(opts \\ [do_resend: true]) do if Keyword.get(opts, :do_resend, true) do - schedule_state_broadcast() - end - - if Keyword.get(opts, :do_logging, false) do - Process.send_after(self(), :log_debug, 1000) + schedule_broadcast_orders() end :net_kernel.monitor_nodes(true) @@ -47,12 +43,17 @@ defmodule Elevator.Communicator do @doc """ Returns a set of alive nodes that are both: a) Connected - AND b) Have sent a message within the cutoff period + c) Have operational == true """ @spec who_can_serve() :: MapSet.t(Node.t()) def who_can_serve, do: GenServer.call(__MODULE__, :who_can_serve) + @doc """ + Returns a set of alive nodes that are both: + a) Connected + b) Have sent a message within the cutoff period + """ @spec who_is_alive() :: MapSet.t(Node.t()) def who_is_alive, do: GenServer.call(__MODULE__, :who_is_alive) @@ -64,37 +65,27 @@ defmodule Elevator.Communicator do def update_operation_status(status), do: GenServer.cast(__MODULE__, {:update_operation_status, status}) - # Updates the timestamp when a message is received from a node - @spec update_state_map(state_t(), Node.t(), boolean()) :: state_t() - defp update_state_map(state, from_node, operational) do - from_node_map = %{operational: operational, timestamp: Time.utc_now()} - %{state | connected_nodes: Map.put(state.connected_nodes, from_node, from_node_map)} - end - - # Schedules another round of state broadcasting. - defp schedule_state_broadcast, - do: Process.send_after(self(), :broadcast_state, @resend_period_ms) - # Infos -------------------------------------------------- @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() + def handle_info(:broadcast_orders, state) do + schedule_broadcast_orders() if Process.whereis(CabOrders) && Process.whereis(HallOrders) do + # Start a separate task to avoid this blocking the communicator. Task.start(fn -> - cab_state = CabOrders.get_state() - hall_state = HallOrders.get_state() + message = %{ + from: Node.self(), + operational: state.operational, + hall_order_map: HallOrders.get_state(), + cab_order_map: CabOrders.get_state() + } Node.list(:connected) - |> GenServer.abcast( - __MODULE__, - {:state_update, Node.self(), state.operational, hall_state, cab_state} - ) + |> GenServer.abcast(__MODULE__, {:receive_external_message, message}) end) end @@ -104,7 +95,7 @@ defmodule Elevator.Communicator do # 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)} + {:noreply, update_node_timestamp(state, node, true)} end # Delete node from state map on disconnect @@ -113,51 +104,26 @@ defmodule Elevator.Communicator 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: #{Node.self()}") - others = who_can_serve() |> Enum.map(fn node -> "#{node}" end) |> Enum.join(", ") - Logger.debug("Others: #{others}") - {:noreply, state} - end - # Calls -------------------------------------------------- @impl true def handle_call(:who_can_serve, _from, state) do - cutoff_ms = Elevator.msg_cutoff_ms() - - communicating_nodes = - state.connected_nodes - |> Map.filter(fn {_k, %{operational: operational, timestamp: timestamp}} -> - Time.diff(Time.utc_now(), timestamp, :millisecond) < cutoff_ms and operational - end) - |> Map.keys() + operational_nodes = + get_communicating_nodes(state) + |> Enum.filter(fn node -> state.connected_nodes[node].operational end) |> MapSet.new() operational_nodes = - if state.operational do - MapSet.put(communicating_nodes, Node.self()) - else - communicating_nodes - end + if state.operational, + do: MapSet.put(operational_nodes, Node.self()), + else: operational_nodes {:reply, operational_nodes, state} end @impl true def handle_call(:who_is_alive, _from, state) do - cutoff_ms = Elevator.msg_cutoff_ms() - - communicating_nodes = - state.connected_nodes - |> Map.filter(fn {_k, %{timestamp: timestamp}} -> - Time.diff(Time.utc_now(), timestamp, :millisecond) < cutoff_ms - end) - |> Map.keys() - |> MapSet.new() - + communicating_nodes = get_communicating_nodes(state) communicating_nodes = MapSet.put(communicating_nodes, Node.self()) {:reply, communicating_nodes, state} end @@ -168,21 +134,42 @@ defmodule Elevator.Communicator do 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.t(), boolean(), hall_orders_t(), cab_orders_t()}, - state_t() - ) :: - {:noreply, state_t()} - def handle_cast({:state_update, from, operational, hall_orders, cab_orders}, state) do - HallOrders.receive_state(hall_orders) - CabOrders.receive_state(cab_orders) - new_state = update_state_map(state, from, operational) + @spec handle_cast({:receive_external_message, communicator_message()}, state_map()) :: + {:noreply, state_map()} + def handle_cast({:receive_external_message, msg}, state) do + HallOrders.receive_external(msg.hall_order_map) + CabOrders.receive_external(msg.cab_order_map) + + new_state = update_node_timestamp(state, msg.from, msg.operational) + {:noreply, new_state} end @impl true - @spec handle_cast({:update_operation_status, boolean()}, state_t()) :: {:noreply, state_t()} + @spec handle_cast({:update_operation_status, boolean()}, state_map()) :: {:noreply, state_map()} def handle_cast({:update_operation_status, status}, state) do {:noreply, %{state | operational: status}} end + + # Helpers -------------------------------------------------- + + @spec get_communicating_nodes(state_map()) :: MapSet.t(Node.t()) + defp get_communicating_nodes(state) do + state.connected_nodes + |> Map.filter(fn {_k, %{timestamp: timestamp}} -> + Time.diff(Time.utc_now(), timestamp, :millisecond) < @msg_cutoff_ms + end) + |> Map.keys() + |> MapSet.new() + end + + # Updates the timestamp when a message is received from a node + @spec update_node_timestamp(state_map(), Node.t(), boolean()) :: state_map() + defp update_node_timestamp(state, from_node, operational) do + from_node_map = %{operational: operational, timestamp: Time.utc_now()} + %{state | connected_nodes: Map.put(state.connected_nodes, from_node, from_node_map)} + end + + defp schedule_broadcast_orders, + do: Process.send_after(self(), :broadcast_orders, @resend_period_ms) end diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 0ece3d3..45181e6 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -49,8 +49,9 @@ defmodule Elevator.HallOrders do 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}) + @spec receive_external(hall_order_map()) :: :ok + def receive_external(other_order_map), + do: GenServer.cast(__MODULE__, {:receive_external, other_order_map}) @doc """ Places the corresponding order in pending state if it is in idle. @@ -124,9 +125,9 @@ defmodule Elevator.HallOrders do # Casts -------------------------------------------------- @impl true - @spec handle_cast({:receive_state, hall_order_map()}, hall_order_map()) :: + @spec handle_cast({:receive_external, hall_order_map()}, hall_order_map()) :: {:noreply, hall_order_map(), {:continue, :hall_update_state}} - def handle_cast({:receive_state, other_order_map}, order_map) do + def handle_cast({:receive_external, other_order_map}, order_map) do who_can_serve = Communicator.who_can_serve() my_orders = my_orders_from_order_map(order_map, who_can_serve) diff --git a/lib/elevator/types.ex b/lib/elevator/types.ex index b9f73a7..6a0853a 100644 --- a/lib/elevator/types.ex +++ b/lib/elevator/types.ex @@ -46,4 +46,11 @@ defmodule Elevator.Types do operational: boolean(), connected_nodes: %{Node.t() => %{operational: boolean(), timestamp: Time.t()}} } + + @type communicator_message :: %{ + from: Node.t(), + operational: boolean(), + hall_order_map: hall_order_map(), + cab_order_map: cab_order_map() + } end From b154255b56e7b5add23a463221bea93b5f9e2713 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 20:20:20 +0100 Subject: [PATCH 12/29] chore: refactor some state names and cab order logic --- lib/elevator/cab_orders.ex | 104 +++++++++++++++---------------- lib/elevator/communicator.ex | 4 +- lib/elevator/hall_orders.ex | 8 +-- lib/elevator/hall_orders/cost.ex | 16 ++--- lib/elevator/types.ex | 4 +- 5 files changed, 66 insertions(+), 70 deletions(-) diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index 28b06ef..d47c65f 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -2,11 +2,10 @@ defmodule Elevator.CabOrders do @moduledoc """ Module responsible for all changes occuring to the cab_order part of the state. """ - alias Elevator.Communicator use GenServer - @type state_t :: Elevator.Types.cab_order_map() - @type floor_t :: Elevator.Types.floor() + @type cab_order_map :: Elevator.Types.cab_order_map() + @type floor :: Elevator.Types.floor() @spec start_link(any()) :: GenServer.on_start() def start_link(arg) do @@ -14,93 +13,90 @@ defmodule Elevator.CabOrders do end @impl true - @spec init(any()) :: {:ok, state_t()} + @spec init(any()) :: {:ok, cab_order_map()} def init(_arg \\ []) do state = %{Node.self() => %{version: 0, orders: MapSet.new()}} {:ok, state} end + # User API -------------------------------------------------- + + @spec get_order_map() :: cab_order_map() + def get_order_map, do: GenServer.call(__MODULE__, :get_order_map) + @doc """ - Callback for getting the current cab orders state. + Retrieve *this* node's current cab orders. """ - @spec get_state() :: state_t() - def get_state do - GenServer.call(__MODULE__, :get_state) - end + @spec get_my_orders() :: MapSet.t(floor()) + def get_my_orders, do: GenServer.call(__MODULE__, :get_my_orders) @doc """ - Callback for getting this nodes current cab orders. + Receive cab order information from another node. + Maps are merged according to highest version numbers. """ - @spec get_my_orders() :: MapSet.t(floor_t()) - def get_my_orders do - GenServer.call(__MODULE__, :get_my_orders) - end - - @spec receive_external(state_t()) :: :ok - def receive_external(other_order_map) do - GenServer.cast(__MODULE__, {:receive_external, other_order_map}) - end + @spec receive_external(cab_order_map()) :: :ok + def receive_external(other_order_map), + do: GenServer.cast(__MODULE__, {:receive_external, other_order_map}) - @spec button_press(floor_t()) :: :ok - def button_press(floor) do - GenServer.cast(__MODULE__, {:button_press, floor}) - end + @doc """ + Add a cab order and increment our own version number. + """ + @spec button_press(floor()) :: :ok + def button_press(floor), do: GenServer.cast(__MODULE__, {:button_press, floor}) - @spec arrived_at_floor(floor_t()) :: :ok - def arrived_at_floor(floor) do - GenServer.cast(__MODULE__, {:arrived_at_floor, floor}) - end + @doc """ + Remove a cab order and increment our own version number. + """ + @spec arrived_at_floor(floor()) :: :ok + def arrived_at_floor(floor), do: GenServer.cast(__MODULE__, {:arrived_at_floor, floor}) # Calls -------------------------------------------------- @impl true - def handle_call(:get_my_orders, _from, state) do - orders = state[Node.self()].orders - {:reply, orders, state} + def handle_call(:get_my_orders, _from, order_map) do + orders = order_map[Node.self()].orders + {:reply, orders, order_map} end @impl true - def handle_call(:get_state, _from, state) do - {:reply, state, state} + def handle_call(:get_order_map, _from, order_map) do + {:reply, order_map, order_map} end # Casts -------------------------------------------------- @impl true - @spec handle_cast({:receive_external, state_t()}, state_t()) :: {:noreply, state_t()} - def handle_cast({:receive_external, other_state}, state) do - new_state = - Enum.reduce(other_state, state, fn {node_id, received}, acc -> - current = Map.get(state, node_id, %{version: -1, orders: MapSet.new()}) - - if received[:version] > current[:version] do - Map.put(acc, node_id, received) - else - acc - end + @spec handle_cast({:receive_external, cab_order_map()}, cab_order_map()) :: + {:noreply, cab_order_map()} + def handle_cast({:receive_external, other_order_map}, order_map) do + new_order_map = + Map.merge(order_map, other_order_map, fn _, current, received -> + if received.version > current.version, + do: received, + else: current end) - {:noreply, new_state} + {:noreply, new_order_map} 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 = - Map.update!(state, Node.self(), fn %{version: old_version, orders: old_orders} -> + @spec handle_cast({:button_press, floor()}, cab_order_map()) :: {:noreply, cab_order_map()} + def handle_cast({:button_press, floor}, order_map) do + new_order_map = + Map.update!(order_map, Node.self(), fn %{version: old_version, orders: old_orders} -> %{version: old_version + 1, orders: MapSet.put(old_orders, floor)} end) - {:noreply, new_state} + {:noreply, new_order_map} 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 = - Map.update!(state, Node.self(), fn %{version: old_version, orders: old_orders} -> + @spec handle_cast({:arrived_at_floor, floor()}, cab_order_map()) :: {:noreply, cab_order_map()} + def handle_cast({:arrived_at_floor, floor}, order_map) do + new_order_map = + Map.update!(order_map, Node.self(), fn %{version: old_version, orders: old_orders} -> %{version: old_version + 1, orders: MapSet.delete(old_orders, floor)} end) - {:noreply, new_state} + {:noreply, new_order_map} end end diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 9a52730..124c2db 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -80,8 +80,8 @@ defmodule Elevator.Communicator do message = %{ from: Node.self(), operational: state.operational, - hall_order_map: HallOrders.get_state(), - cab_order_map: CabOrders.get_state() + hall_order_map: HallOrders.get_order_map(), + cab_order_map: CabOrders.get_order_map() } Node.list(:connected) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 45181e6..5714af4 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -71,10 +71,8 @@ defmodule Elevator.HallOrders do @doc """ Retrieve the full hall order state map """ - @spec get_state() :: hall_order_map() - def get_state do - GenServer.call(__MODULE__, :get_state) - end + @spec get_order_map() :: hall_order_map() + def get_order_map(), do: GenServer.call(__MODULE__, :get_order_map) @doc """ Retrieve only the orders we are going to take. @@ -118,7 +116,7 @@ defmodule Elevator.HallOrders do end @impl true - def handle_call(:get_state, _, order_map) do + def handle_call(:get_order_map, _, order_map) do {:reply, order_map, order_map} end diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 5e2065c..e142366 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -14,12 +14,12 @@ defmodule Elevator.HallOrders.Cost do @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()} + @type floor :: Elevator.Types.floor() + @type hall_btn :: Elevator.Types.hall_btn() + @type combined_order_map :: Elevator.Types.combined_order_map() + @type cost_map :: Elevator.Types.hall_order_cost_map() - @spec compute_cost({floor_t(), hall_btn_t()}, %{floor_t() => MapSet.t(hall_btn_t())}) :: + @spec compute_cost({floor(), hall_btn()}, %{floor() => MapSet.t(hall_btn())}) :: non_neg_integer() def compute_cost({floor, btn_dir}, my_hall_orders) do try do @@ -52,7 +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() + @spec merge_cost(cost_map(), cost_map()) :: cost_map() 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 -> @@ -75,7 +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() + @spec min_alive_cost(cost_map(), 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) @@ -96,7 +96,7 @@ defmodule Elevator.HallOrders.Cost do end end - @spec simulate_cost_until_served(combined_orders_t(), State.t(), {floor_t(), hall_btn_t()}) :: + @spec simulate_cost_until_served(combined_order_map(), State.t(), {floor(), hall_btn()}) :: non_neg_integer() defp simulate_cost_until_served(_orders, %{floor: :unknown}, _target), do: @unreachable_cost diff --git a/lib/elevator/types.ex b/lib/elevator/types.ex index 6a0853a..0d6d201 100644 --- a/lib/elevator/types.ex +++ b/lib/elevator/types.ex @@ -19,10 +19,12 @@ defmodule Elevator.Types do @type hall_order_key :: {floor(), hall_btn()} + @type hall_order_cost_map :: %{Node.t() => non_neg_integer()} + @type hall_order_state :: :idle | {:pending, MapSet.t()} - | {:handling, %{Node.t() => integer()}} + | {:handling, hall_order_map()} | {:arrived, MapSet.t()} @type hall_order_map :: %{ From 94b15670f535e24529675a63bef9802a49f4a4c8 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 20:41:21 +0100 Subject: [PATCH 13/29] chore: refactor all Types --- lib/elevator.ex | 11 +++++ lib/elevator/cab_orders.ex | 10 ++++- lib/elevator/communicator.ex | 16 +++++++- lib/elevator/decision.ex | 34 ++++------------ lib/elevator/fsm/state.ex | 16 ++++---- lib/elevator/fsm/transition.ex | 3 +- lib/elevator/hall_orders.ex | 36 +++++++++++------ lib/elevator/hall_orders/cost.ex | 12 +++--- lib/elevator/hall_orders/order.ex | 16 ++++---- lib/elevator/hardware/input_poller.ex | 2 +- lib/elevator/hardware/outputs.ex | 5 +-- lib/elevator/types.ex | 58 --------------------------- 12 files changed, 90 insertions(+), 129 deletions(-) delete mode 100644 lib/elevator/types.ex diff --git a/lib/elevator.ex b/lib/elevator.ex index 7c9a298..41f9462 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -1,4 +1,12 @@ defmodule Elevator do + @type floor :: non_neg_integer() + + @type button_type :: :cab | Elevator.HallOrders.hall_button_type() + + @type combined_order_map :: %{ + floor() => MapSet.t(button_type()) + } + @num_floors 4 @door_open_duration_ms 1000 @@ -11,4 +19,7 @@ defmodule Elevator do def door_open_duration_ms do @door_open_duration_ms end + + @spec button_types() :: [button_type()] + def button_types(), do: [:hall_up, :hall_down, :cab] end diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index d47c65f..d436338 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -4,8 +4,14 @@ defmodule Elevator.CabOrders do """ use GenServer - @type cab_order_map :: Elevator.Types.cab_order_map() - @type floor :: Elevator.Types.floor() + @type floor :: Elevator.floor() + + @type cab_orders_snapshot :: %{ + version: non_neg_integer(), + orders: MapSet.t(floor()) + } + + @type cab_order_map :: %{Node.t() => cab_orders_snapshot()} @spec start_link(any()) :: GenServer.on_start() def start_link(arg) do diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 124c2db..2411e1a 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -10,9 +10,21 @@ defmodule Elevator.Communicator do require Logger use GenServer - @type state_map :: Elevator.Types.communicator_state_map() + @type hall_order_map :: Elevator.HallOrders.hall_order_map() + @type cab_order_map :: Elevator.CabOrders.cab_order_map() + + @type state_map :: %{ + operational: boolean(), + connected_nodes: %{Node.t() => %{operational: boolean(), timestamp: Time.t()}} + } + + @type communicator_message :: %{ + from: Node.t(), + operational: boolean(), + hall_order_map: hall_order_map(), + cab_order_map: cab_order_map() + } - @type communicator_message :: Elevator.Types.communicator_message() @type communicator_options :: [do_resend: boolean()] @resend_period_ms 50 diff --git a/lib/elevator/decision.ex b/lib/elevator/decision.ex index dc898d6..afef037 100644 --- a/lib/elevator/decision.ex +++ b/lib/elevator/decision.ex @@ -5,40 +5,20 @@ defmodule Elevator.Decision do These functions are intentionally pure to make them easy to unit test. """ - @spec requests_above?(Elevator.Types.combined_order_map(), Elevator.Types.floor()) :: boolean() + @spec requests_above?(Elevator.combined_order_map(), Elevator.floor()) :: boolean() def requests_above?(reqs, floor) do Enum.any?(reqs, fn {f, _} -> f > floor end) end - @spec requests_below?(Elevator.Types.combined_order_map(), Elevator.Types.floor()) :: boolean() + @spec requests_below?(Elevator.combined_order_map(), Elevator.floor()) :: boolean() def requests_below?(reqs, floor) do Enum.any?(reqs, fn {f, _} -> f < floor end) end - @doc "Should a request at `btn_floor` and `btn_type` be cleared immediately given elevator state?" - @spec should_clear_immediately?( - Elevator.FSM.State.t(), - Elevator.Types.floor(), - Elevator.Types.btn_type() - ) :: boolean() - def should_clear_immediately?( - %Elevator.FSM.State{floor: floor, direction: direction}, - btn_floor, - btn_type - ) do - cond do - floor != btn_floor -> false - btn_type == :cab -> true - direction == :up and btn_type == :hall_up -> true - direction == :down and btn_type == :hall_down -> true - true -> false - end - end - @spec combine_hall_and_cab( - Elevator.Types.combined_order_map(), - MapSet.t(Elevator.Types.floor()) - ) :: Elevator.Types.combined_order_map() + Elevator.combined_order_map(), + MapSet.t(Elevator.floor()) + ) :: Elevator.combined_order_map() def combine_hall_and_cab(hall_orders, cab_floors) do Enum.reduce(cab_floors, hall_orders, fn floor, acc -> Map.update(acc, floor, MapSet.new([:cab]), &MapSet.put(&1, :cab)) @@ -47,8 +27,8 @@ 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()) :: - {Elevator.Types.elev_dir(), :moving | :door_open | :idle} + @spec next_action(Elevator.combined_order_map(), Elevator.FSM.State.t()) :: + {:up | :down, :moving | :door_open | :idle} def next_action( orders, %Elevator.FSM.State{ diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index eec8223..d6b3404 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -6,7 +6,6 @@ defmodule Elevator.FSM.State do its floor, direction, behavior, and fault conditions. """ require Logger - alias Elevator.Types defstruct direction: :down, behavior: :moving, @@ -17,10 +16,13 @@ defmodule Elevator.FSM.State do door_open_time_ms: Time.utc_now(), last_floor_time: Time.utc_now() + @type floor :: Elevator.floor() + @type elev_behavior :: :moving | :idle | :door_open + @type t :: %__MODULE__{ - direction: Types.elev_dir(), - behavior: Types.elev_behavior(), - floor: :unknown | Types.floor(), + direction: :up | :down, + behavior: elev_behavior(), + floor: :unknown | Elevator.floor(), between_floors: boolean(), obstructed: boolean(), motor_timed_out: boolean(), @@ -46,7 +48,7 @@ defmodule Elevator.FSM.State do @doc """ Updates floor and between_floors status. """ - @spec set_floor(:between_floors | Types.floor()) :: :ok + @spec set_floor(:between_floors | floor()) :: :ok def set_floor(floor), do: GenServer.cast(__MODULE__, {:set_floor, floor}) @doc """ @@ -57,10 +59,10 @@ defmodule Elevator.FSM.State do def set_obstruction(obstruction_switch), do: GenServer.cast(__MODULE__, {:set_obstruction, obstruction_switch}) - @spec set_direction(Types.elev_dir()) :: :ok + @spec set_direction(:up | :down) :: :ok def set_direction(dir), do: GenServer.cast(__MODULE__, {:set_direction, dir}) - @spec set_behavior(Types.elev_behavior()) :: :ok + @spec set_behavior(elev_behavior()) :: :ok def set_behavior(behavior), do: GenServer.cast(__MODULE__, {:set_behavior, behavior}) @doc """ diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 83993cc..a523f1f 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -8,7 +8,6 @@ defmodule Elevator.FSM.Transition do """ require Logger - alias Elevator.Types alias Elevator.CabOrders alias Elevator.FSM.State alias Elevator.HallOrders @@ -60,7 +59,7 @@ defmodule Elevator.FSM.Transition do Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) end - @spec decide_and_update_state(Elevator.FSM.State.t(), Types.combined_order_map()) :: any() + @spec decide_and_update_state(Elevator.FSM.State.t(), Elevator.combined_order_map()) :: any() defp decide_and_update_state(state, orders) when not state.motor_timed_out do {new_direction, new_behavior} = Decision.next_action(orders, state) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 5714af4..b76b5ca 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -12,9 +12,19 @@ defmodule Elevator.HallOrders do require Logger use GenServer - @type hall_order_map :: Elevator.Types.hall_order_map() - @type floor :: Elevator.Types.floor() - @type hall_btn :: Elevator.Types.hall_btn() + @type floor :: Elevator.floor() + + @type hall_button_type :: :hall_down | :hall_up + @type hall_button :: {floor(), hall_button_type()} + + @type hall_order_cost_map :: %{Node.t() => non_neg_integer()} + @type hall_order_state :: + :idle + | {:pending, MapSet.t()} + | {:handling, hall_order_map()} + | {:arrived, MapSet.t()} + + @type hall_order_map :: %{hall_button() => hall_order_state()} @hall_order_refresh_period_ms 1000 @@ -56,7 +66,7 @@ defmodule Elevator.HallOrders do @doc """ Places the corresponding order in pending state if it is in idle. """ - @spec button_press(floor(), hall_btn()) :: :ok + @spec button_press(floor(), hall_button_type()) :: :ok def button_press(floor, button_type), do: GenServer.cast(__MODULE__, {:button_press, floor, button_type}) @@ -77,7 +87,7 @@ defmodule Elevator.HallOrders do @doc """ Retrieve only the orders we are going to take. """ - @spec get_my_orders() :: %{Elevator.Types.floor() => MapSet.t(Elevator.Types.hall_btn())} + @spec get_my_orders() :: %{floor() => MapSet.t(hall_button_type())} def get_my_orders do GenServer.call(__MODULE__, :get_my_orders) end @@ -86,7 +96,7 @@ defmodule Elevator.HallOrders do 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())} + @spec get_confirmed_orders() :: %{floor() => MapSet.t(hall_button_type())} def get_confirmed_orders do GenServer.call(__MODULE__, :get_confirmed_orders) end @@ -141,7 +151,7 @@ defmodule Elevator.HallOrders do end @impl true - @spec handle_cast({:button_press, floor(), hall_btn()}, hall_order_map()) :: + @spec handle_cast({:button_press, floor(), hall_button()}, hall_order_map()) :: {:noreply, hall_order_map(), {:continue, :hall_update_state}} def handle_cast({:button_press, floor, direction}, order_map) do # If in idle, go to pending. Otherwise, ignore. @@ -242,16 +252,16 @@ defmodule Elevator.HallOrders do end @type enum_orders :: - Elevator.Types.hall_order_map() - | Enumerable.t({Elevator.Types.hall_order_key(), Elevator.Types.hall_order_state()}) - @spec orders_by_floor(enum_orders()) :: %{floor() => MapSet.t(hall_btn())} + hall_order_map() + | Enumerable.t({hall_button(), hall_order_state()}) + @spec orders_by_floor(enum_orders()) :: %{floor() => MapSet.t(hall_button())} defp orders_by_floor(orders) do - # Restructure order map to the format floor => MapSet(order) + # Restructure order map to the format floor => MapSet(button_type) orders - |> Enum.map(fn {{floor, btn_type}, _} -> {floor, btn_type} end) + |> Enum.map(fn {{floor, button_type}, _} -> {floor, button_type} end) |> Enum.group_by(fn {floor, _} -> floor end) |> Enum.map(fn {floor, order_list} -> - {floor, MapSet.new(Enum.map(order_list, fn {_, btn_type} -> btn_type end))} + {floor, MapSet.new(Enum.map(order_list, fn {_, button_type} -> button_type end))} end) |> Enum.into(%{}) end diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index e142366..e637330 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -14,12 +14,12 @@ defmodule Elevator.HallOrders.Cost do @max_simulation_steps 256 @unreachable_cost 30000 - @type floor :: Elevator.Types.floor() - @type hall_btn :: Elevator.Types.hall_btn() - @type combined_order_map :: Elevator.Types.combined_order_map() - @type cost_map :: Elevator.Types.hall_order_cost_map() + @type floor :: Elevator.floor() + @type hall_button_type :: Elevator.HallOrders.hall_button_type() + @type combined_order_map :: Elevator.combined_order_map() + @type cost_map :: Elevator.HallOrders.hall_order_cost_map() - @spec compute_cost({floor(), hall_btn()}, %{floor() => MapSet.t(hall_btn())}) :: + @spec compute_cost({floor(), hall_button_type()}, %{floor() => MapSet.t(hall_button_type())}) :: non_neg_integer() def compute_cost({floor, btn_dir}, my_hall_orders) do try do @@ -96,7 +96,7 @@ defmodule Elevator.HallOrders.Cost do end end - @spec simulate_cost_until_served(combined_order_map(), State.t(), {floor(), hall_btn()}) :: + @spec simulate_cost_until_served(combined_order_map(), State.t(), {floor(), hall_button_type()}) :: non_neg_integer() defp simulate_cost_until_served(_orders, %{floor: :unknown}, _target), do: @unreachable_cost diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index 3c3242b..5ef83a2 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -10,19 +10,19 @@ defmodule Elevator.HallOrders.Order do - 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 alias Elevator.HallOrders.Cost alias Elevator.Communicator - @type hall_order_key :: Elevator.Types.hall_order_key() - @type hall_order_state :: Elevator.Types.hall_order_state() + @type floor :: Elevator.floor() + @type hall_button :: Elevator.HallOrders.hall_button() + @type hall_button_type :: Elevator.HallOrders.hall_button_type() + @type hall_order_state :: Elevator.HallOrders.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_state(), hall_order_state(), %{ - Types.floor() => MapSet.t(Types.hall_btn()) + @spec merge_hall_orders(hall_button(), hall_order_state(), hall_order_state(), %{ + floor() => MapSet.t(hall_button_type()) }) :: hall_order_state() def merge_hall_orders(order_key, order_state, other_order_state, my_hall_orders) do @@ -65,8 +65,8 @@ 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_state(), %{ - Types.floor() => MapSet.t(Types.hall_btn()) + @spec update_hall_order(hall_button(), hall_order_state(), %{ + floor() => MapSet.t(hall_button_type()) }) :: {boolean(), hall_order_state()} def update_hall_order(order_key, order_state, confirmed_hall_orders) do alive = Communicator.who_is_alive() diff --git a/lib/elevator/hardware/input_poller.ex b/lib/elevator/hardware/input_poller.ex index 496c368..b8a892a 100644 --- a/lib/elevator/hardware/input_poller.ex +++ b/lib/elevator/hardware/input_poller.ex @@ -69,7 +69,7 @@ defmodule Elevator.Hardware.InputPoller do # Helpers -------------------------------------------------- defp get_pressed_buttons_at_floor(floor) do - Elevator.Types.btn_types() + Elevator.button_types() |> Enum.filter(fn btn -> Driver.get_order_button_state(floor, btn) == :active end) diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index 645e6ad..fadd240 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -6,7 +6,6 @@ defmodule Elevator.Hardware.Outputs do require Logger alias Elevator.Communicator alias Elevator.Hardware.Driver - alias Elevator.Types alias Elevator.FSM @spec init() :: :ok @@ -16,7 +15,7 @@ defmodule Elevator.Hardware.Outputs do Driver.set_motor_direction(:stop) end - @spec set_outputs(FSM.State.t(), Types.combined_order_map()) :: any() + @spec set_outputs(FSM.State.t(), Elevator.combined_order_map()) :: any() def set_outputs(state, light_orders) do set_door_light(state) set_motors(state) @@ -45,7 +44,7 @@ defmodule Elevator.Hardware.Outputs do end defp set_order_lights(orders) do - for floor <- 0..(Elevator.num_floors() - 1), btn <- Types.btn_types() do + for floor <- 0..(Elevator.num_floors() - 1), btn <- Elevator.button_types() do lights = Map.get(orders, floor, MapSet.new()) state = if MapSet.member?(lights, btn), do: :on, else: :off Driver.set_order_button_light(btn, floor, state) diff --git a/lib/elevator/types.ex b/lib/elevator/types.ex deleted file mode 100644 index 0d6d201..0000000 --- a/lib/elevator/types.ex +++ /dev/null @@ -1,58 +0,0 @@ -defmodule Elevator.Types do - @moduledoc """ - Different type definitions for the elevator - """ - @type floor :: non_neg_integer() - - @type btn_dir :: :up | :down - - @type elev_dir :: :up | :down - - @type elev_behavior :: :moving | :idle | :door_open - - @type hall_btn :: :hall_down | :hall_up - - @type btn_type :: :cab | hall_btn() - - @spec btn_types() :: [btn_type()] - def btn_types(), do: [:hall_up, :hall_down, :cab] - - @type hall_order_key :: {floor(), hall_btn()} - - @type hall_order_cost_map :: %{Node.t() => non_neg_integer()} - - @type hall_order_state :: - :idle - | {:pending, MapSet.t()} - | {:handling, hall_order_map()} - | {:arrived, MapSet.t()} - - @type hall_order_map :: %{ - hall_order_key() => hall_order_state() - } - - @type cab_orders_snapshot :: %{ - version: non_neg_integer(), - orders: MapSet.t(floor()) - } - - @type cab_order_map :: %{ - Node.t() => cab_orders_snapshot() - } - - @type combined_order_map :: %{ - floor() => MapSet.t(btn_type()) - } - - @type communicator_state_map :: %{ - operational: boolean(), - connected_nodes: %{Node.t() => %{operational: boolean(), timestamp: Time.t()}} - } - - @type communicator_message :: %{ - from: Node.t(), - operational: boolean(), - hall_order_map: hall_order_map(), - cab_order_map: cab_order_map() - } -end From 1476e53d511977c33c26473e46507a485d948958 Mon Sep 17 00:00:00 2001 From: Magnus Date: Wed, 18 Mar 2026 20:58:22 +0100 Subject: [PATCH 14/29] fix: make tests pass --- test/multi/cab_orders_test.exs | 30 +++++++------- test/multi/hall_orders_test.exs | 72 ++++++++++++++++----------------- test/single/cab_orders_test.exs | 1 - test/single/decision_test.exs | 22 ---------- 4 files changed, 51 insertions(+), 74 deletions(-) diff --git a/test/multi/cab_orders_test.exs b/test/multi/cab_orders_test.exs index c935805..dcd1967 100644 --- a/test/multi/cab_orders_test.exs +++ b/test/multi/cab_orders_test.exs @@ -35,7 +35,7 @@ defmodule Test.Multi.CabOrdersTest do node1_orders = :rpc.call(node1, CabOrders, :get_my_orders, []) assert node1_orders == MapSet.new() - :rpc.call(node2, CabOrders, :receive_state, [ + :rpc.call(node2, CabOrders, :receive_external, [ %{node1 => %{version: 420, orders: MapSet.new([3])}} ]) @@ -47,46 +47,46 @@ defmodule Test.Multi.CabOrdersTest 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] + :rpc.call(node1, CabOrders, :get_order_map, [])[node1] assert node1_version == 0 assert node1_orders == MapSet.new() # Higher version number should overwrite - :rpc.cast(node2, CabOrders, :receive_state, [ + :rpc.cast(node2, CabOrders, :receive_external, [ %{node1 => %{version: 69, orders: MapSet.new([1])}} ]) Process.sleep(TestUtils.convergence_wait_ms()) %{version: node1_version, orders: node1_orders} = - :rpc.call(node1, CabOrders, :get_state, [])[node1] + :rpc.call(node1, CabOrders, :get_order_map, [])[node1] assert node1_version == 69 assert node1_orders == MapSet.new([1]) # Lower version number should be ignored - :rpc.cast(node3, CabOrders, :receive_state, [ + :rpc.cast(node3, CabOrders, :receive_external, [ %{node1 => %{version: 67, orders: MapSet.new([1, 2])}} ]) Process.sleep(TestUtils.convergence_wait_ms()) %{version: node1_version, orders: node1_orders} = - :rpc.call(node1, CabOrders, :get_state, [])[node1] + :rpc.call(node1, CabOrders, :get_order_map, [])[node1] assert node1_version == 69 assert node1_orders == MapSet.new([1]) # Same version number should be ignored - :rpc.cast(node3, CabOrders, :receive_state, [ + :rpc.cast(node3, CabOrders, :receive_external, [ %{node1 => %{version: 69, orders: MapSet.new([1, 2])}} ]) Process.sleep(TestUtils.convergence_wait_ms()) %{version: node1_version, orders: node1_orders} = - :rpc.call(node1, CabOrders, :get_state, [])[node1] + :rpc.call(node1, CabOrders, :get_order_map, [])[node1] assert node1_version == 69 assert node1_orders == MapSet.new([1]) @@ -94,13 +94,13 @@ defmodule Test.Multi.CabOrdersTest do test "cab order states propagate", %{nodes: [node1, node2, node3]} do %{version: node1_version, orders: node1_orders} = - :rpc.call(node1, CabOrders, :get_state, [])[node1] + :rpc.call(node1, CabOrders, :get_order_map, [])[node1] %{version: node2_version, orders: node2_orders} = - :rpc.call(node2, CabOrders, :get_state, [])[node2] + :rpc.call(node2, CabOrders, :get_order_map, [])[node2] %{version: node3_version, orders: node3_orders} = - :rpc.call(node3, CabOrders, :get_state, [])[node3] + :rpc.call(node3, CabOrders, :get_order_map, [])[node3] assert node1_version == 0 and node2_version == 0 and node3_version == 0 @@ -112,14 +112,14 @@ defmodule Test.Multi.CabOrdersTest do # Ensure that node1's version and order map has propagated across all nodes %{version: node1_version, orders: node1_orders} = - :rpc.call(node1, CabOrders, :get_state, [])[node1] + :rpc.call(node1, CabOrders, :get_order_map, [])[node1] assert node1_version == 1 assert MapSet.member?(node1_orders, 1) - node1_state = :rpc.call(node1, CabOrders, :get_state, []) - node2_state = :rpc.call(node2, CabOrders, :get_state, []) - node3_state = :rpc.call(node3, CabOrders, :get_state, []) + node1_state = :rpc.call(node1, CabOrders, :get_order_map, []) + node2_state = :rpc.call(node2, CabOrders, :get_order_map, []) + node3_state = :rpc.call(node3, CabOrders, :get_order_map, []) assert Map.equal?(node1_state, node2_state) assert Map.equal?(node2_state, node3_state) diff --git a/test/multi/hall_orders_test.exs b/test/multi/hall_orders_test.exs index f7efcb5..0c35753 100644 --- a/test/multi/hall_orders_test.exs +++ b/test/multi/hall_orders_test.exs @@ -25,12 +25,12 @@ defmodule Test.Multi.HallOrdersTest do test "button convergence to confirmed", %{nodes: [node1, node2, node3]} do # Node 1 gets a button call :rpc.call(node1, HallOrders, :button_press, [0, :hall_up]) - node1_state = :rpc.call(node1, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) 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 :rpc.call(node2, HallOrders, :receive_external, [node1_state]) == :ok + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) assert {:pending, barrier2} = node2_state[{0, :hall_up}] assert MapSet.member?(barrier2, node1) @@ -38,8 +38,8 @@ defmodule Test.Multi.HallOrdersTest do assert not MapSet.member?(barrier2, node3) # 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 :rpc.call(node3, HallOrders, :receive_external, [node1_state]) == :ok + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) assert {:pending, barrier3} = node3_state[{0, :hall_up}] assert MapSet.member?(barrier3, node1) @@ -47,17 +47,17 @@ defmodule Test.Multi.HallOrdersTest do assert not MapSet.member?(barrier3, node2) # Node 1 receives orders from both 2 and 3 - assert :rpc.call(node1, HallOrders, :receive_state, [node2_state]) == :ok - assert :rpc.call(node1, HallOrders, :receive_state, [node3_state]) == :ok + assert :rpc.call(node1, HallOrders, :receive_external, [node2_state]) == :ok + assert :rpc.call(node1, HallOrders, :receive_external, [node3_state]) == :ok - node1_state = :rpc.call(node1, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) assert {:handling, _} = node1_state[{0, :hall_up}] clique_exchange_states([node1, node2, node3]) - node1_state = :rpc.call(node1, HallOrders, :get_state, []) - node2_state = :rpc.call(node2, HallOrders, :get_state, []) - node3_state = :rpc.call(node3, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) # All should have converged on the alive set assert node1_state == node2_state and node2_state == node3_state @@ -67,9 +67,9 @@ defmodule Test.Multi.HallOrdersTest do # ... so another exchange run should not affect the result clique_exchange_states([node1, node2, node3]) - node1_state = :rpc.call(node1, HallOrders, :get_state, []) - node2_state = :rpc.call(node2, HallOrders, :get_state, []) - node3_state = :rpc.call(node3, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) assert node1_state == converged_state and node1_state == node2_state and node2_state == node3_state @@ -81,7 +81,7 @@ defmodule Test.Multi.HallOrdersTest do clique_exchange_states(nodes) clique_exchange_states(nodes) - node1_state = :rpc.call(node1, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) assert {:handling, _} = node1_state[{1, :hall_down}] # Assume node3 arrives at the floor @@ -89,9 +89,9 @@ defmodule Test.Multi.HallOrdersTest do clique_exchange_states(nodes) - node1_state = :rpc.call(node1, HallOrders, :get_state, []) - node2_state = :rpc.call(node2, HallOrders, :get_state, []) - node3_state = :rpc.call(node3, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) assert :idle = node1_state[{1, :hall_down}] assert :idle = node2_state[{1, :hall_down}] @@ -138,9 +138,9 @@ defmodule Test.Multi.HallOrdersTest do Process.sleep(TestUtils.convergence_wait_ms()) - node1_state = :rpc.call(node1, HallOrders, :get_state, []) - node2_state = :rpc.call(node2, HallOrders, :get_state, []) - node3_state = :rpc.call(node3, HallOrders, :get_state, []) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) assert node1_state == node2_state and node2_state == node3_state assert :idle = node1_state[{2, :hall_up}] @@ -148,31 +148,31 @@ defmodule Test.Multi.HallOrdersTest do defp clique_exchange_states([node1, node2, node3]) do # 1 -> 2 - node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert :rpc.call(node2, HallOrders, :receive_state, [node1_state]) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) + assert :rpc.call(node2, HallOrders, :receive_external, [node1_state]) # 2 -> 3 - node2_state = :rpc.call(node2, HallOrders, :get_state, []) - assert :rpc.call(node3, HallOrders, :receive_state, [node2_state]) + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) + assert :rpc.call(node3, HallOrders, :receive_external, [node2_state]) # 3 -> 1 - node3_state = :rpc.call(node3, HallOrders, :get_state, []) - assert :rpc.call(node1, HallOrders, :receive_state, [node3_state]) + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) + assert :rpc.call(node1, HallOrders, :receive_external, [node3_state]) # 1 -> 2, 3 - node1_state = :rpc.call(node1, HallOrders, :get_state, []) - assert :rpc.call(node2, HallOrders, :receive_state, [node1_state]) - assert :rpc.call(node3, HallOrders, :receive_state, [node1_state]) + node1_state = :rpc.call(node1, HallOrders, :get_order_map, []) + assert :rpc.call(node2, HallOrders, :receive_external, [node1_state]) + assert :rpc.call(node3, HallOrders, :receive_external, [node1_state]) # 2 -> 1, 3 - node2_state = :rpc.call(node2, HallOrders, :get_state, []) - assert :rpc.call(node1, HallOrders, :receive_state, [node2_state]) - assert :rpc.call(node3, HallOrders, :receive_state, [node2_state]) + node2_state = :rpc.call(node2, HallOrders, :get_order_map, []) + assert :rpc.call(node1, HallOrders, :receive_external, [node2_state]) + assert :rpc.call(node3, HallOrders, :receive_external, [node2_state]) # 3 -> 1, 2 - node3_state = :rpc.call(node3, HallOrders, :get_state, []) - assert :rpc.call(node1, HallOrders, :receive_state, [node3_state]) - assert :rpc.call(node2, HallOrders, :receive_state, [node3_state]) + node3_state = :rpc.call(node3, HallOrders, :get_order_map, []) + assert :rpc.call(node1, HallOrders, :receive_external, [node3_state]) + assert :rpc.call(node2, HallOrders, :receive_external, [node3_state]) # Yay! end diff --git a/test/single/cab_orders_test.exs b/test/single/cab_orders_test.exs index f70b0d4..252d7d3 100644 --- a/test/single/cab_orders_test.exs +++ b/test/single/cab_orders_test.exs @@ -1,5 +1,4 @@ defmodule Test.Single.CabOrdersTest do - alias Elevator.Communicator alias Elevator.CabOrders use ExUnit.Case, async: false diff --git a/test/single/decision_test.exs b/test/single/decision_test.exs index 7151b34..179a7c2 100644 --- a/test/single/decision_test.exs +++ b/test/single/decision_test.exs @@ -68,26 +68,4 @@ defmodule Test.Single.DecisionTest do state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} assert Decision.next_action(orders, state) == {:up, :door_open} end - - test "should_clear_immediately?" do - door_open_up = %Elevator.FSM.State{floor: 2, direction: :up, behavior: :door_open} - - assert Decision.should_clear_immediately?(door_open_up, 2, :cab) - assert Decision.should_clear_immediately?(door_open_up, 2, :hall_up) - - refute Decision.should_clear_immediately?(door_open_up, 2, :hall_down) - refute Decision.should_clear_immediately?(door_open_up, 3, :cab) - - moving = %Elevator.FSM.State{floor: 2, direction: :up, behavior: :moving} - assert Decision.should_clear_immediately?(moving, 2, :cab) - - door_open_down = %Elevator.FSM.State{ - floor: 2, - direction: :down, - behavior: :door_open, - between_floors: false - } - - assert Decision.should_clear_immediately?(door_open_down, 2, :hall_down) - end end From 3b4f7d20a8adddb857501ca46e83cece656c9c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Thu, 19 Mar 2026 11:19:19 +0100 Subject: [PATCH 15/29] chore: trivial cleanup in hall_orders and cost --- lib/elevator.ex | 8 ++------ lib/elevator/hall_orders.ex | 26 ++++++++++---------------- lib/elevator/hall_orders/order.ex | 10 +++------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index 41f9462..28b56ff 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -11,14 +11,10 @@ defmodule Elevator do @door_open_duration_ms 1000 @spec num_floors() :: pos_integer() - def num_floors do - @num_floors - end + def num_floors, do: @num_floors @spec door_open_duration_ms() :: pos_integer() - def door_open_duration_ms do - @door_open_duration_ms - end + def door_open_duration_ms, do: @door_open_duration_ms @spec button_types() :: [button_type()] def button_types(), do: [:hall_up, :hall_down, :cab] diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index b76b5ca..14aa38a 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -29,15 +29,14 @@ defmodule Elevator.HallOrders do @hall_order_refresh_period_ms 1000 @spec start_link(any()) :: GenServer.on_start() - def start_link(arg) do - GenServer.start_link(__MODULE__, arg, name: __MODULE__) - end + def start_link(arg), do: GenServer.start_link(__MODULE__, arg, name: __MODULE__) @impl true @spec init(any()) :: {:ok, hall_order_map()} def init(num_floors) do top_floor = num_floors - 1 + # Initialize all orders to :idle state = Range.new(0, top_floor) |> Enum.flat_map(fn floor -> @@ -64,19 +63,18 @@ defmodule Elevator.HallOrders do do: GenServer.cast(__MODULE__, {:receive_external, other_order_map}) @doc """ - Places the corresponding order in pending state if it is in idle. + Places the corresponding order in pending state if it is in idle. """ @spec button_press(floor(), hall_button_type()) :: :ok def button_press(floor, button_type), do: GenServer.cast(__MODULE__, {:button_press, floor, button_type}) @doc """ - Goes back to idle if the order is confirmed. + Goes back to idle if the order is in handling. """ @spec arrived_at_floor(floor(), :up | :down) :: :ok - def arrived_at_floor(floor, direction) do - GenServer.cast(__MODULE__, {:arrived_at_floor, floor, direction}) - end + def arrived_at_floor(floor, direction), + do: GenServer.cast(__MODULE__, {:arrived_at_floor, floor, direction}) @doc """ Retrieve the full hall order state map @@ -88,18 +86,14 @@ defmodule Elevator.HallOrders do Retrieve only the orders we are going to take. """ @spec get_my_orders() :: %{floor() => MapSet.t(hall_button_type())} - def get_my_orders do - GenServer.call(__MODULE__, :get_my_orders) - end + def get_my_orders(), do: GenServer.call(__MODULE__, :get_my_orders) @doc """ 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() :: %{floor() => MapSet.t(hall_button_type())} - def get_confirmed_orders do - GenServer.call(__MODULE__, :get_confirmed_orders) - end + def get_confirmed_orders, do: GenServer.call(__MODULE__, :get_confirmed_orders) # Calls -------------------------------------------------- @@ -171,9 +165,9 @@ defmodule Elevator.HallOrders do new_order_state = new_order_map[key] if old_order_state != new_order_state do - Logger.debug(fn -> + Logger.debug( "hall_button_press floor=#{floor} button=#{direction} from=#{inspect(old_order_state)} to=#{inspect(new_order_state)}" - end) + ) end {:noreply, new_order_map, {:continue, :hall_update_state}} diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index 5ef83a2..a9ba81a 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -27,15 +27,13 @@ defmodule Elevator.HallOrders.Order do 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) + my_id = Node.self() # Ensure self is in any barrier set. new_order_state = case new_order_state do - {:pending, barrier_set} -> - {:pending, MapSet.put(barrier_set, Node.self())} - - {:arrived, barrier_set} -> - {:arrived, MapSet.put(barrier_set, Node.self())} + {state, barrier_set} when state in [:pending, :arrived] -> + {state, MapSet.put(barrier_set, my_id)} _ -> new_order_state @@ -45,8 +43,6 @@ defmodule Elevator.HallOrders.Order do new_order_state = case new_order_state do {:handling, cost_map} -> - my_id = Node.self() - if not Map.has_key?(cost_map, my_id) do {:handling, Map.put(cost_map, my_id, Cost.compute_cost(order_key, my_hall_orders))} else From 4be3ba8efdbe212c4448eccc98d5b16a8891660a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Dings=C3=B8r?= Date: Thu, 19 Mar 2026 11:35:28 +0100 Subject: [PATCH 16/29] chore: idiomatic elixir --- lib/elevator/hall_orders.ex | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 14aa38a..8c0063e 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -108,12 +108,7 @@ defmodule Elevator.HallOrders do @impl true def handle_call(:get_confirmed_orders, _from, order_map) do confirmed_orders = - Enum.filter(order_map, fn {_, order_state} -> - case order_state do - {:handling, _} -> true - _ -> false - end - end) + Enum.filter(order_map, &match?({_, {:handling, _}}, &1)) |> orders_by_floor() {:reply, confirmed_orders, order_map} From 673844843537f3159a9374a5045c582538cb09c7 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 11:45:42 +0100 Subject: [PATCH 17/29] chore: confirmed -> handling --- lib/elevator/fsm/transition.ex | 2 +- lib/elevator/hall_orders.ex | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index a523f1f..27acf22 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -54,7 +54,7 @@ defmodule Elevator.FSM.Transition do end defp get_light_orders() do - hall_orders = HallOrders.get_confirmed_orders() + hall_orders = HallOrders.get_handling_orders() pressed_cab_floors = CabOrders.get_my_orders() Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) end diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 8c0063e..76dee6c 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -89,11 +89,11 @@ defmodule Elevator.HallOrders do def get_my_orders(), do: GenServer.call(__MODULE__, :get_my_orders) @doc """ - Get all confirmed orders in same format as get_my_orders. + Get all orders in handling state, in the same format as get_my_orders. These are the orders we turn the light on for. """ - @spec get_confirmed_orders() :: %{floor() => MapSet.t(hall_button_type())} - def get_confirmed_orders, do: GenServer.call(__MODULE__, :get_confirmed_orders) + @spec get_handling_orders() :: %{floor() => MapSet.t(hall_button_type())} + def get_handling_orders(), do: GenServer.call(__MODULE__, :get_handling_orders) # Calls -------------------------------------------------- @@ -106,7 +106,7 @@ defmodule Elevator.HallOrders do end @impl true - def handle_call(:get_confirmed_orders, _from, order_map) do + def handle_call(:get_handling_orders, _from, order_map) do confirmed_orders = Enum.filter(order_map, &match?({_, {:handling, _}}, &1)) |> orders_by_floor() From 02c79748098146eefd776edf077016868a6f80e4 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 11:47:27 +0100 Subject: [PATCH 18/29] chore: consistent parentheses usage --- lib/elevator.ex | 4 ++-- lib/elevator/cab_orders.ex | 4 ++-- lib/elevator/communicator.ex | 4 ++-- test/utils/test_compiled.ex | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/elevator.ex b/lib/elevator.ex index 28b56ff..3ff73cf 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -11,10 +11,10 @@ defmodule Elevator do @door_open_duration_ms 1000 @spec num_floors() :: pos_integer() - def num_floors, do: @num_floors + def num_floors(), do: @num_floors @spec door_open_duration_ms() :: pos_integer() - def door_open_duration_ms, do: @door_open_duration_ms + def door_open_duration_ms(), do: @door_open_duration_ms @spec button_types() :: [button_type()] def button_types(), do: [:hall_up, :hall_down, :cab] diff --git a/lib/elevator/cab_orders.ex b/lib/elevator/cab_orders.ex index d436338..3a0ec42 100644 --- a/lib/elevator/cab_orders.ex +++ b/lib/elevator/cab_orders.ex @@ -28,13 +28,13 @@ defmodule Elevator.CabOrders do # User API -------------------------------------------------- @spec get_order_map() :: cab_order_map() - def get_order_map, do: GenServer.call(__MODULE__, :get_order_map) + def get_order_map(), do: GenServer.call(__MODULE__, :get_order_map) @doc """ Retrieve *this* node's current cab orders. """ @spec get_my_orders() :: MapSet.t(floor()) - def get_my_orders, do: GenServer.call(__MODULE__, :get_my_orders) + def get_my_orders(), do: GenServer.call(__MODULE__, :get_my_orders) @doc """ Receive cab order information from another node. diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index 2411e1a..a72a19d 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -59,7 +59,7 @@ defmodule Elevator.Communicator do c) Have operational == true """ @spec who_can_serve() :: MapSet.t(Node.t()) - def who_can_serve, do: GenServer.call(__MODULE__, :who_can_serve) + def who_can_serve(), do: GenServer.call(__MODULE__, :who_can_serve) @doc """ Returns a set of alive nodes that are both: @@ -67,7 +67,7 @@ defmodule Elevator.Communicator do b) Have sent a message within the cutoff period """ @spec who_is_alive() :: MapSet.t(Node.t()) - def who_is_alive, do: GenServer.call(__MODULE__, :who_is_alive) + def who_is_alive(), do: GenServer.call(__MODULE__, :who_is_alive) @doc """ Updates the `operational` part of the state. diff --git a/test/utils/test_compiled.ex b/test/utils/test_compiled.ex index a464224..2802e5c 100644 --- a/test/utils/test_compiled.ex +++ b/test/utils/test_compiled.ex @@ -3,7 +3,7 @@ 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: 150 + def convergence_wait_ms(), do: 150 def start_order_modules(num_floors, do_resend) do children = [ From 3c3f3555452f8187060134f1a2e0bdcec3316943 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 12:13:18 +0100 Subject: [PATCH 19/29] feat: simplify communicator state --- lib/elevator/communicator.ex | 88 +++++++++++++------------------- lib/elevator/fsm/state.ex | 8 +++ lib/elevator/hardware/outputs.ex | 5 -- 3 files changed, 43 insertions(+), 58 deletions(-) diff --git a/lib/elevator/communicator.ex b/lib/elevator/communicator.ex index a72a19d..b613850 100644 --- a/lib/elevator/communicator.ex +++ b/lib/elevator/communicator.ex @@ -3,6 +3,7 @@ defmodule Elevator.Communicator do Module responsible for all communication with other elevators. """ + alias Elevator.FSM.State alias Elevator alias Elevator.CabOrders alias Elevator.HallOrders @@ -13,9 +14,8 @@ defmodule Elevator.Communicator do @type hall_order_map :: Elevator.HallOrders.hall_order_map() @type cab_order_map :: Elevator.CabOrders.cab_order_map() - @type state_map :: %{ - operational: boolean(), - connected_nodes: %{Node.t() => %{operational: boolean(), timestamp: Time.t()}} + @type peer_status_map :: %{ + Node.t() => %{operational: boolean(), timestamp: Time.t()} } @type communicator_message :: %{ @@ -43,13 +43,10 @@ defmodule Elevator.Communicator do :net_kernel.monitor_nodes(true) - state = %{ - operational: true, - connected_nodes: - Map.from_keys(Node.list(:connected), %{operational: true, timestamp: Time.utc_now()}) - } + peer_status_map = + Map.from_keys(Node.list(:connected), %{operational: true, timestamp: Time.utc_now()}) - {:ok, state} + {:ok, peer_status_map} end @doc """ @@ -69,21 +66,13 @@ defmodule Elevator.Communicator do @spec who_is_alive() :: MapSet.t(Node.t()) def who_is_alive(), do: GenServer.call(__MODULE__, :who_is_alive) - @doc """ - 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: GenServer.cast(__MODULE__, {:update_operation_status, status}) - # Infos -------------------------------------------------- @doc """ Sends the cab and hall state to all connected nodes. """ @impl true - def handle_info(:broadcast_orders, state) do + def handle_info(:broadcast_orders, peer_status_map) do schedule_broadcast_orders() if Process.whereis(CabOrders) && Process.whereis(HallOrders) do @@ -91,7 +80,7 @@ defmodule Elevator.Communicator do Task.start(fn -> message = %{ from: Node.self(), - operational: state.operational, + operational: State.operational?(), hall_order_map: HallOrders.get_order_map(), cab_order_map: CabOrders.get_order_map() } @@ -101,43 +90,43 @@ defmodule Elevator.Communicator do end) end - {:noreply, state} + {:noreply, peer_status_map} end - # Update the state map when a new node connects + # Update the status map when a new node connects @impl true - def handle_info({:nodeup, node}, state) do - {:noreply, update_node_timestamp(state, node, true)} + def handle_info({:nodeup, node}, peer_status_map) do + {:noreply, update_node_timestamp(peer_status_map, node, true)} end - # Delete node from state map on disconnect + # Delete node from status map on disconnect @impl true - def handle_info({:nodedown, node}, state) do - {:noreply, %{state | connected_nodes: Map.delete(state.connected_nodes, node)}} + def handle_info({:nodedown, node}, peer_status_map) do + {:noreply, %{peer_status_map | connected_nodes: Map.delete(peer_status_map, node)}} end # Calls -------------------------------------------------- @impl true - def handle_call(:who_can_serve, _from, state) do + def handle_call(:who_can_serve, _from, peer_status_map) do operational_nodes = - get_communicating_nodes(state) - |> Enum.filter(fn node -> state.connected_nodes[node].operational end) + get_communicating_nodes(peer_status_map) + |> Enum.filter(fn node -> peer_status_map[node].operational end) |> MapSet.new() operational_nodes = - if state.operational, + if State.operational?(), do: MapSet.put(operational_nodes, Node.self()), else: operational_nodes - {:reply, operational_nodes, state} + {:reply, operational_nodes, peer_status_map} end @impl true - def handle_call(:who_is_alive, _from, state) do - communicating_nodes = get_communicating_nodes(state) + def handle_call(:who_is_alive, _from, peer_status_map) do + communicating_nodes = get_communicating_nodes(peer_status_map) communicating_nodes = MapSet.put(communicating_nodes, Node.self()) - {:reply, communicating_nodes, state} + {:reply, communicating_nodes, peer_status_map} end # Casts -------------------------------------------------- @@ -146,28 +135,21 @@ defmodule Elevator.Communicator do 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({:receive_external_message, communicator_message()}, state_map()) :: - {:noreply, state_map()} - def handle_cast({:receive_external_message, msg}, state) do + @spec handle_cast({:receive_external_message, communicator_message()}, peer_status_map()) :: + {:noreply, peer_status_map()} + def handle_cast({:receive_external_message, msg}, peer_status_map) do HallOrders.receive_external(msg.hall_order_map) CabOrders.receive_external(msg.cab_order_map) - new_state = update_node_timestamp(state, msg.from, msg.operational) - - {:noreply, new_state} - end - - @impl true - @spec handle_cast({:update_operation_status, boolean()}, state_map()) :: {:noreply, state_map()} - def handle_cast({:update_operation_status, status}, state) do - {:noreply, %{state | operational: status}} + new_peer_status_map = update_node_timestamp(peer_status_map, msg.from, msg.operational) + {:noreply, new_peer_status_map} end # Helpers -------------------------------------------------- - @spec get_communicating_nodes(state_map()) :: MapSet.t(Node.t()) - defp get_communicating_nodes(state) do - state.connected_nodes + @spec get_communicating_nodes(peer_status_map()) :: MapSet.t(Node.t()) + defp get_communicating_nodes(peer_status_map) do + peer_status_map |> Map.filter(fn {_k, %{timestamp: timestamp}} -> Time.diff(Time.utc_now(), timestamp, :millisecond) < @msg_cutoff_ms end) @@ -176,10 +158,10 @@ defmodule Elevator.Communicator do end # Updates the timestamp when a message is received from a node - @spec update_node_timestamp(state_map(), Node.t(), boolean()) :: state_map() - defp update_node_timestamp(state, from_node, operational) do - from_node_map = %{operational: operational, timestamp: Time.utc_now()} - %{state | connected_nodes: Map.put(state.connected_nodes, from_node, from_node_map)} + @spec update_node_timestamp(peer_status_map(), Node.t(), boolean()) :: peer_status_map() + defp update_node_timestamp(peer_status_map, from_node, operational) do + new_peer_status = %{operational: operational, timestamp: Time.utc_now()} + Map.put(peer_status_map, from_node, new_peer_status) end defp schedule_broadcast_orders, diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index d6b3404..6d610d4 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -79,6 +79,9 @@ defmodule Elevator.FSM.State do @spec get_state() :: t() def get_state(), do: GenServer.call(__MODULE__, :get_state) + @spec operational?() :: boolean() + def operational?(), do: GenServer.call(__MODULE__, :operational?) + # Casts -------------------------------------------------- @impl true @@ -135,4 +138,9 @@ defmodule Elevator.FSM.State do def handle_call(:get_state, _from, state) do {:reply, state, state} end + + @impl true + def handle_call(:operational?, _from, state) do + {:reply, not (state.motor_timed_out or state.obstructed), state} + end end diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index fadd240..aeb9993 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -4,7 +4,6 @@ defmodule Elevator.Hardware.Outputs do """ require Logger - alias Elevator.Communicator alias Elevator.Hardware.Driver alias Elevator.FSM @@ -20,10 +19,6 @@ defmodule Elevator.Hardware.Outputs do set_door_light(state) set_motors(state) set_floor_light(state) - - operational = not (state.motor_timed_out or state.obstructed) - Communicator.update_operation_status(operational) - set_order_lights(light_orders) end From 76d2d0e5267332a60deb0188c0fd06234057a1a0 Mon Sep 17 00:00:00 2001 From: DxMarch Date: Thu, 19 Mar 2026 12:16:36 +0100 Subject: [PATCH 20/29] refactor(hall-orders): move cost simulation to separate module and simplify flow extract simulation from Cost into dedicated Simulation module reduce nesting and shorten transition/clear-order logic improve naming clarity for simulation state and order-map variables keep behavior aligned with current-direction clearing policy --- lib/elevator/decision.ex | 4 - lib/elevator/hall_orders/cost.ex | 223 +++---------------------- lib/elevator/hall_orders/simulation.ex | 166 ++++++++++++++++++ test/single/cost_test.exs | 13 +- 4 files changed, 200 insertions(+), 206 deletions(-) create mode 100644 lib/elevator/hall_orders/simulation.ex diff --git a/lib/elevator/decision.ex b/lib/elevator/decision.ex index afef037..186e123 100644 --- a/lib/elevator/decision.ex +++ b/lib/elevator/decision.ex @@ -38,7 +38,6 @@ 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 -> @@ -82,9 +81,6 @@ defmodule Elevator.Decision do true -> {:down, :idle} end - - true -> - {:down, :idle} end end end diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index e637330..109ee0d 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -8,44 +8,37 @@ defmodule Elevator.HallOrders.Cost do alias Elevator.CabOrders alias Elevator.Decision alias Elevator.FSM.State + alias Elevator.HallOrders.Simulation require Logger - @travel_duration_ms 2500 - @max_simulation_steps 256 - @unreachable_cost 30000 - @type floor :: Elevator.floor() @type hall_button_type :: Elevator.HallOrders.hall_button_type() - @type combined_order_map :: Elevator.combined_order_map() @type cost_map :: Elevator.HallOrders.hall_order_cost_map() @spec compute_cost({floor(), hall_button_type()}, %{floor() => MapSet.t(hall_button_type())}) :: non_neg_integer() - def compute_cost({floor, btn_dir}, my_hall_orders) do - try do - state = State.get_state() - 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)) + def compute_cost({floor, hall_button_type}, my_hall_orders) do + state = State.get_state() + cab_orders = CabOrders.get_my_orders() - combined_orders = Decision.combine_hall_and_cab(hall_orders_with_target, cab_orders) + hall_orders_with_target = + Map.update( + my_hall_orders, + floor, + MapSet.new([hall_button_type]), + &MapSet.put(&1, hall_button_type) + ) - result = simulate_cost_until_served(combined_orders, state, {floor, btn_dir}) + combined_orders = Decision.combine_hall_and_cab(hall_orders_with_target, cab_orders) - Logger.debug(fn -> - "hall_cost request=#{inspect({floor, btn_dir})} state=#{state.behavior}@#{inspect(state.floor)} dir=#{state.direction} result=#{result}" - end) + result = + Simulation.simulate_time_until_served(combined_orders, state, {floor, hall_button_type}) - result - rescue - error -> - Logger.warning( - "Failed to compute hall cost for #{inspect({floor, btn_dir})}: #{inspect(error)}" - ) + Logger.debug(fn -> + "hall_cost request=#{inspect({floor, hall_button_type})} state=#{state.behavior}@#{inspect(state.floor)} dir=#{state.direction} result=#{result}" + end) - @unreachable_cost - end + result end @doc """ @@ -53,20 +46,20 @@ defmodule Elevator.HallOrders.Cost do Uses pessimistic merge: If two conflicting costs for the same node are found, keep the higher one. """ @spec merge_cost(cost_map(), cost_map()) :: cost_map() - def merge_cost(cost_map_1, cost_map_2) do - MapSet.new(Map.keys(cost_map_1) ++ Map.keys(cost_map_2)) + def merge_cost(cost_map, other_cost_map) do + MapSet.new(Map.keys(cost_map) ++ Map.keys(other_cost_map)) |> Enum.map(fn node -> cond do - Map.has_key?(cost_map_1, node) and Map.has_key?(cost_map_2, node) -> - cost_1 = cost_map_1[node] - cost_2 = cost_map_2[node] - {node, max(cost_1, cost_2)} + Map.has_key?(cost_map, node) and Map.has_key?(other_cost_map, node) -> + cost = cost_map[node] + other_cost = other_cost_map[node] + {node, max(cost, other_cost)} - Map.has_key?(cost_map_1, node) -> - {node, cost_map_1[node]} + Map.has_key?(cost_map, node) -> + {node, cost_map[node]} true -> - {node, cost_map_2[node]} + {node, other_cost_map[node]} end end) |> Enum.into(%{}) @@ -95,166 +88,4 @@ defmodule Elevator.HallOrders.Cost do min_node end end - - @spec simulate_cost_until_served(combined_order_map(), State.t(), {floor(), hall_button_type()}) :: - non_neg_integer() - defp simulate_cost_until_served(_orders, %{floor: :unknown}, _target), do: @unreachable_cost - - defp simulate_cost_until_served(_orders, %{obstructed: true}, _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} - - initial_time_ms = - if normalized_state.behavior == :door_open and elem(target, 0) != normalized_state.floor do - Elevator.door_open_duration_ms() - else - 0 - end - - if target_cleared?(orders, target) do - 0 - else - do_simulate(orders, normalized_state, target, initial_time_ms, @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 -> - # No reason to continue up: clear hall down when turning around - 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/simulation.ex b/lib/elevator/hall_orders/simulation.ex new file mode 100644 index 0000000..7500b71 --- /dev/null +++ b/lib/elevator/hall_orders/simulation.ex @@ -0,0 +1,166 @@ +defmodule Elevator.HallOrders.Simulation do + @moduledoc """ + Pure hall-order cost simulation. + """ + + alias Elevator.Decision + alias Elevator.FSM.State + + @travel_duration_ms 2500 + @max_simulation_steps 256 + @unreachable_cost 30000 + + @type floor :: Elevator.floor() + @type hall_button_type :: Elevator.HallOrders.hall_button_type() + @type combined_order_map :: Elevator.combined_order_map() + + defmodule SimState do + @enforce_keys [:orders, :elevator_state, :target, :time_ms, :steps_left] + defstruct [:orders, :elevator_state, :target, :time_ms, :steps_left] + end + + @type simulation :: %SimState{ + orders: combined_order_map(), + elevator_state: State.t(), + target: {floor(), hall_button_type()}, + time_ms: non_neg_integer(), + steps_left: non_neg_integer() + } + + @spec travel_duration_ms() :: non_neg_integer() + def travel_duration_ms, do: @travel_duration_ms + + @spec unreachable_cost() :: non_neg_integer() + def unreachable_cost, do: @unreachable_cost + + @spec initial_time_ms(State.t(), floor()) :: number() + def initial_time_ms(elevator_state, target_floor) do + cond do + elevator_state.behavior == :door_open and target_floor != elevator_state.floor -> + Elevator.door_open_duration_ms() / 2 + + elevator_state.behavior == :moving -> + @travel_duration_ms / 2 + + true -> + 0 + end + end + + @spec simulate_time_until_served(combined_order_map(), State.t(), {floor(), hall_button_type()}) :: + non_neg_integer() + def simulate_time_until_served(_orders, %{floor: :unknown}, _target), + do: @unreachable_cost + + def simulate_time_until_served(_orders, %{obstructed: true}, _target), + do: @unreachable_cost + + def simulate_time_until_served(orders, elevator_state, target) do + initial_time_ms = initial_time_ms(elevator_state, elem(target, 0)) + + if target_cleared?(orders, target) do + 0 + else + sim_state = %SimState{ + orders: orders, + elevator_state: elevator_state, + target: target, + time_ms: initial_time_ms, + steps_left: @max_simulation_steps + } + + do_simulate(sim_state) + end + end + + @spec do_simulate(simulation()) :: non_neg_integer() + defp do_simulate(%SimState{steps_left: 0}), do: @unreachable_cost + + defp do_simulate(%SimState{orders: orders, target: target, time_ms: time_ms} = sim_state) do + if target_cleared?(orders, target) do + time_ms + else + do_simulate_step(sim_state) + end + end + + @spec do_simulate_step(simulation()) :: non_neg_integer() + defp do_simulate_step(%SimState{orders: orders, elevator_state: elevator_state} = sim_state) do + {direction, behavior} = Decision.next_action(orders, elevator_state) + + {next_orders, next_elevator_state, delta_ms} = + case behavior do + :idle -> + raise( + "Invalid simulation transition: got :idle while target still pending. sim_state=#{inspect(sim_state)}" + ) + + :moving -> + {:ok, next_floor} = move_one_floor(elevator_state.floor, direction) + + {orders, + %{ + elevator_state + | floor: next_floor, + between_floors: false, + direction: direction, + behavior: :moving + }, @travel_duration_ms} + + :door_open -> + {clear_requests_at_floor_in_direction(orders, elevator_state.floor, direction), + %{ + elevator_state + | direction: direction, + behavior: :idle, + between_floors: false + }, Elevator.door_open_duration_ms()} + end + + next_sim_state = %SimState{ + sim_state + | orders: next_orders, + elevator_state: next_elevator_state, + time_ms: sim_state.time_ms + delta_ms, + steps_left: sim_state.steps_left - 1 + } + + do_simulate(next_sim_state) + end + + defp target_cleared?(orders, {floor, hall_button_type}) do + orders + |> Map.get(floor, MapSet.new()) + |> MapSet.member?(hall_button_type) + |> Kernel.not() + end + + defp move_one_floor(floor, :up) do + if floor < Elevator.num_floors() - 1, + do: {:ok, floor + 1}, + else: raise("Invalid simulation transition: cannot move up from floor #{inspect(floor)}") + end + + defp move_one_floor(floor, :down) do + if floor > 0, + do: {:ok, floor - 1}, + else: raise("Invalid simulation transition: cannot move down from floor #{inspect(floor)}") + end + + defp clear_requests_at_floor_in_direction(orders, floor, direction) do + floor_orders_after_cab_clear = + orders + |> Map.get(floor, MapSet.new()) + |> MapSet.delete(:cab) + + remaining_floor_orders = + case direction do + :up -> MapSet.delete(floor_orders_after_cab_clear, :hall_up) + :down -> MapSet.delete(floor_orders_after_cab_clear, :hall_down) + end + + if MapSet.size(remaining_floor_orders) == 0, + do: Map.delete(orders, floor), + else: Map.put(orders, floor, remaining_floor_orders) + end +end diff --git a/test/single/cost_test.exs b/test/single/cost_test.exs index 02e7b2e..784792f 100644 --- a/test/single/cost_test.exs +++ b/test/single/cost_test.exs @@ -4,9 +4,8 @@ defmodule Test.Single.CostTest do alias Elevator.CabOrders alias Elevator.FSM.State alias Elevator.HallOrders.Cost + alias Elevator.HallOrders.Simulation - @travel_duration_ms 2500 - @unreachable_cost 30000 @state_settle_ms 10 setup do @@ -33,20 +32,22 @@ defmodule Test.Single.CostTest do set_state(floor: 0, direction: :up, behavior: :idle) assert Cost.compute_cost({1, :hall_up}, %{}) == - @travel_duration_ms + Elevator.door_open_duration_ms() + Simulation.travel_duration_ms() + Elevator.door_open_duration_ms() end - test "one floor away request includes current open-door delay" do + test "one floor away request includes half current open-door delay" do set_state(floor: 0, direction: :up, behavior: :door_open) + state = State.get_state() assert Cost.compute_cost({1, :hall_up}, %{}) == - 2 * Elevator.door_open_duration_ms() + @travel_duration_ms + Simulation.initial_time_ms(state, 1) + + Elevator.door_open_duration_ms() + Simulation.travel_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}, %{}) == @unreachable_cost + assert Cost.compute_cost({1, :hall_up}, %{}) == Simulation.unreachable_cost() end defp set_state(opts) do From 6d6406170a57d1879e7f51b1dce2ddc955b3b250 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 13:33:22 +0100 Subject: [PATCH 21/29] chore: refactor order.ex for coherency reasons --- lib/elevator/hall_orders.ex | 60 ++--------- lib/elevator/hall_orders/order.ex | 166 +++++++++++++++--------------- test/single/hall_orders_test.exs | 19 +--- 3 files changed, 96 insertions(+), 149 deletions(-) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 76dee6c..35b7671 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -36,7 +36,7 @@ defmodule Elevator.HallOrders do def init(num_floors) do top_floor = num_floors - 1 - # Initialize all orders to :idle + # Initialize all orders to idle state = Range.new(0, top_floor) |> Enum.flat_map(fn floor -> @@ -131,7 +131,9 @@ defmodule Elevator.HallOrders do new_order_map = Map.keys(order_map) |> Enum.map(fn key -> - new_value = Order.merge_hall_orders(key, order_map[key], other_order_map[key], my_orders) + new_value = + Order.update_from_incoming(key, order_map[key], other_order_map[key], my_orders) + {key, new_value} end) |> Enum.into(%{}) @@ -143,28 +145,7 @@ defmodule Elevator.HallOrders do @spec handle_cast({:button_press, floor(), hall_button()}, hall_order_map()) :: {:noreply, hall_order_map(), {:continue, :hall_update_state}} def handle_cast({:button_press, floor, direction}, order_map) do - # If in idle, go to pending. Otherwise, ignore. - key = {floor, direction} - - old_order_state = order_map[key] - - new_order_map = - case old_order_state do - :idle -> - Map.put(order_map, key, {:pending, MapSet.new([Node.self()])}) - - _ -> - order_map - end - - new_order_state = new_order_map[key] - - if old_order_state != new_order_state do - Logger.debug( - "hall_button_press floor=#{floor} button=#{direction} from=#{inspect(old_order_state)} to=#{inspect(new_order_state)}" - ) - end - + new_order_map = Map.update!(order_map, {floor, direction}, &Order.update_from_button_press/1) {:noreply, new_order_map, {:continue, :hall_update_state}} end @@ -172,17 +153,9 @@ defmodule Elevator.HallOrders do def handle_cast({:arrived_at_floor, floor, direction}, order_map) do # 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_state = order_map[key] new_order_map = - case order_state do - {:handling, _} -> - Map.put(order_map, key, {:arrived, MapSet.new([Node.self()])}) - - _ -> - order_map - end + Map.update!(order_map, {floor, button_type}, &Order.update_from_arrived_at_floor/1) {:noreply, new_order_map, {:continue, :hall_update_state}} end @@ -206,31 +179,18 @@ defmodule Elevator.HallOrders do alive = Communicator.who_can_serve() my_orders = my_orders_from_order_map(order_map, alive) - {any_did_change, new_order_map} = - Enum.reduce(order_map, {false, %{}}, fn {key, button_state}, - {acc_did_change, acc_order_map} -> - {did_change, new_button_state} = Order.update_hall_order(key, button_state, my_orders) - - {acc_did_change or did_change, Map.put(acc_order_map, key, new_button_state)} + new_order_map = + Map.new(order_map, fn {key, order_state} -> + {key, Order.update_from_barrier_state(key, order_state, my_orders)} end) - if any_did_change do - {:noreply, new_order_map, {:continue, :hall_update_state}} - else - {:noreply, new_order_map} - end + {:noreply, new_order_map} end defp my_orders_from_order_map(order_map, alive) do Enum.filter(order_map, fn {_, order_state} -> case order_state do {:handling, cost_map} -> - if Cost.min_alive_cost(cost_map, alive) == Node.self() do - Logger.debug( - "\nCost map: #{inspect(cost_map)}\nAlive: #{inspect(alive)}\nI (#{inspect(Node.self())}) am the one to serve" - ) - end - Cost.min_alive_cost(cost_map, alive) == Node.self() _ -> diff --git a/lib/elevator/hall_orders/order.ex b/lib/elevator/hall_orders/order.ex index a9ba81a..1d7b347 100644 --- a/lib/elevator/hall_orders/order.ex +++ b/lib/elevator/hall_orders/order.ex @@ -2,12 +2,12 @@ defmodule Elevator.HallOrders.Order do @moduledoc """ 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. + A hall order is tied to a floor and direction (up/down). + It is essentially 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. + - handling: 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.HallOrders.Cost @@ -21,110 +21,114 @@ defmodule Elevator.HallOrders.Order do @doc """ Update a hall order based on an incoming hall order from another node. """ - @spec merge_hall_orders(hall_button(), hall_order_state(), hall_order_state(), %{ - floor() => MapSet.t(hall_button_type()) - }) :: - 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) - my_id = Node.self() - - # Ensure self is in any barrier set. - new_order_state = - case new_order_state do - {state, barrier_set} when state in [:pending, :arrived] -> - {state, MapSet.put(barrier_set, my_id)} - - _ -> - new_order_state - end - - # Ensure self is in a cost map - new_order_state = - case new_order_state do - {:handling, cost_map} -> - if not Map.has_key?(cost_map, my_id) do - {:handling, 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_state + @spec update_from_incoming( + hall_button(), + hall_order_state(), + hall_order_state(), + %{floor() => MapSet.t(hall_button_type())} + ) :: hall_order_state() + def update_from_incoming(order_key, order_state, incoming_order_state, my_hall_orders) do + order_state + |> merge_with_incoming(incoming_order_state) + |> ensure_self_in_barriers() + |> ensure_self_in_cost_map(order_key, my_hall_orders) end @doc """ - 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. + Advances a pending or arrived order if the respective barrier set is full. + Returns `{true, new_value}` if the state changed, `{false, old_value}` otherwise. """ - @spec update_hall_order(hall_button(), hall_order_state(), %{ - floor() => MapSet.t(hall_button_type()) - }) :: {boolean(), hall_order_state()} - def update_hall_order(order_key, order_state, confirmed_hall_orders) do - alive = Communicator.who_is_alive() - - {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, {:handling, %{Node.self() => 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 - - _ -> - {false, order_state} - end - - {did_change, new_state} + @spec update_from_barrier_state( + hall_button(), + hall_order_state(), + %{floor() => MapSet.t(hall_button_type())} + ) :: hall_order_state() + def update_from_barrier_state(order_key, order_state, my_hall_orders) do + order_state + |> transition_from_barrier_state(Communicator.who_is_alive()) + |> ensure_self_in_barriers() + |> ensure_self_in_cost_map(order_key, my_hall_orders) end - defp merge_orders(my_state, other_state) do - case {my_state, other_state} do + @spec update_from_button_press(hall_order_state()) :: hall_order_state() + def update_from_button_press(:idle) do + ensure_self_in_barriers({:pending, MapSet.new()}) + end + + def update_from_button_press(order_state), do: order_state + + def update_from_arrived_at_floor({:handling, _}) do + ensure_self_in_barriers({:arrived, MapSet.new()}) + end + + def update_from_arrived_at_floor(order_state), do: order_state + + # Helpers -------------------------------------------------- + + # Cyclic counter logic for updating a state when receiving from another node. + defp merge_with_incoming(my_state, incoming_state) do + case {my_state, incoming_state} do {:idle, {:arrived, _}} -> my_state - {:idle, other_state} -> - other_state + {:idle, incoming_state} -> + incoming_state {{:pending, _}, :idle} -> my_state - {{:pending, my_barrier}, {:pending, other_barrier}} -> - {:pending, MapSet.union(my_barrier, other_barrier)} + {{:pending, my_barrier}, {:pending, incoming_barrier}} -> + {:pending, MapSet.union(my_barrier, incoming_barrier)} - {{:pending, _}, other_state} -> - other_state + {{:pending, _}, incoming_state} -> + incoming_state - {{:handling, my_cost_map}, {:handling, other_cost_map}} -> - {:handling, Cost.merge_cost(my_cost_map, other_cost_map)} + {{:handling, my_cost_map}, {:handling, incoming_cost_map}} -> + {:handling, Cost.merge_cost(my_cost_map, incoming_cost_map)} {{:handling, _}, {:arrived, _}} -> - other_state + incoming_state {{:handling, _}, _} -> my_state - {{:arrived, my_barrier}, {:arrived, other_barrier}} -> - {:arrived, MapSet.union(my_barrier, other_barrier)} + {{:arrived, my_barrier}, {:arrived, incoming_barrier}} -> + {:arrived, MapSet.union(my_barrier, incoming_barrier)} {{:arrived, _}, :idle} -> - other_state + incoming_state _ -> my_state end end + + defp transition_from_barrier_state(order_state = {:pending, barrier_set}, alive) do + if MapSet.subset?(alive, barrier_set), + do: {:handling, %{}}, + else: order_state + end + + defp transition_from_barrier_state(order_state = {:arrived, barrier_set}, alive) do + if MapSet.subset?(alive, barrier_set), + do: :idle, + else: order_state + end + + defp transition_from_barrier_state(order_state, _), do: order_state + + defp ensure_self_in_barriers({state, barrier_set}) when state in [:pending, :arrived] do + {state, MapSet.put(barrier_set, Node.self())} + end + + defp ensure_self_in_barriers(order_state), do: order_state + + defp ensure_self_in_cost_map({:handling, cost_map}, order_key, my_hall_orders) do + {:handling, + Map.put_new_lazy(cost_map, Node.self(), fn -> + Cost.compute_cost(order_key, my_hall_orders) + end)} + end + + defp ensure_self_in_cost_map(order_state, _, _), do: order_state end diff --git a/test/single/hall_orders_test.exs b/test/single/hall_orders_test.exs index 33267ca..c0e12d7 100644 --- a/test/single/hall_orders_test.exs +++ b/test/single/hall_orders_test.exs @@ -2,8 +2,6 @@ defmodule Test.Single.HallOrdersTest do use ExUnit.Case, async: false # TODO: Maybe doctest - @max_continue_iterations 100 - setup_all do start_supervised!(Elevator.Communicator) start_supervised!(Elevator.CabOrders) @@ -89,22 +87,7 @@ defmodule Test.Single.HallOrdersTest do case ret do {:noreply, new_state, {:continue, continue_arg}} -> - hallorder_continue_full(continue_arg, new_state) - - _ -> - ret - end - end - - defp hallorder_continue_full(continue_arg, state, continue_counter \\ 0) do - # Prevent infinite continue loop - assert continue_counter < @max_continue_iterations - - ret = Elevator.HallOrders.handle_continue(continue_arg, state) - - case ret do - {:noreply, new_state, {:continue, continue_arg}} -> - hallorder_continue_full(continue_arg, new_state, continue_counter + 1) + Elevator.HallOrders.handle_continue(continue_arg, new_state) _ -> ret From 739c22ce32acde57ae4d94c0dc76b1f6e001cfb5 Mon Sep 17 00:00:00 2001 From: DxMarch Date: Thu, 19 Mar 2026 14:34:45 +0100 Subject: [PATCH 22/29] chore: more readable and shorter --- lib/elevator/hall_orders/cost.ex | 25 ++++++++----------------- lib/elevator/hall_orders/simulation.ex | 15 ++++++--------- test/single/cost_test.exs | 20 ++++++++++++++++++++ 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 109ee0d..0b8db61 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -15,13 +15,17 @@ defmodule Elevator.HallOrders.Cost do @type hall_button_type :: Elevator.HallOrders.hall_button_type() @type cost_map :: Elevator.HallOrders.hall_order_cost_map() + @doc """ + Comupte the cost (time to serve) of a canditate hall order by simulating single elevator logic. + """ @spec compute_cost({floor(), hall_button_type()}, %{floor() => MapSet.t(hall_button_type())}) :: non_neg_integer() def compute_cost({floor, hall_button_type}, my_hall_orders) do state = State.get_state() cab_orders = CabOrders.get_my_orders() - hall_orders_with_target = + # Include the candidate hall request in our local hall-order snapshot before simulating. + hall_orders_with_candidate = Map.update( my_hall_orders, floor, @@ -29,7 +33,7 @@ defmodule Elevator.HallOrders.Cost do &MapSet.put(&1, hall_button_type) ) - combined_orders = Decision.combine_hall_and_cab(hall_orders_with_target, cab_orders) + combined_orders = Decision.combine_hall_and_cab(hall_orders_with_candidate, cab_orders) result = Simulation.simulate_time_until_served(combined_orders, state, {floor, hall_button_type}) @@ -47,22 +51,9 @@ defmodule Elevator.HallOrders.Cost do """ @spec merge_cost(cost_map(), cost_map()) :: cost_map() def merge_cost(cost_map, other_cost_map) do - MapSet.new(Map.keys(cost_map) ++ Map.keys(other_cost_map)) - |> Enum.map(fn node -> - cond do - Map.has_key?(cost_map, node) and Map.has_key?(other_cost_map, node) -> - cost = cost_map[node] - other_cost = other_cost_map[node] - {node, max(cost, other_cost)} - - Map.has_key?(cost_map, node) -> - {node, cost_map[node]} - - true -> - {node, other_cost_map[node]} - end + Map.merge(cost_map, other_cost_map, fn _node, cost, other_cost -> + max(cost, other_cost) end) - |> Enum.into(%{}) end @doc """ diff --git a/lib/elevator/hall_orders/simulation.ex b/lib/elevator/hall_orders/simulation.ex index 7500b71..bef5e45 100644 --- a/lib/elevator/hall_orders/simulation.ex +++ b/lib/elevator/hall_orders/simulation.ex @@ -49,10 +49,10 @@ defmodule Elevator.HallOrders.Simulation do @spec simulate_time_until_served(combined_order_map(), State.t(), {floor(), hall_button_type()}) :: non_neg_integer() - def simulate_time_until_served(_orders, %{floor: :unknown}, _target), + def simulate_time_until_served(_orders, %{floor: :unknown} = _elevator_state, _target), do: @unreachable_cost - def simulate_time_until_served(_orders, %{obstructed: true}, _target), + def simulate_time_until_served(_orders, %{obstructed: true} = _elevator_state, _target), do: @unreachable_cost def simulate_time_until_served(orders, elevator_state, target) do @@ -148,16 +148,13 @@ defmodule Elevator.HallOrders.Simulation do end defp clear_requests_at_floor_in_direction(orders, floor, direction) do - floor_orders_after_cab_clear = + hall_button_to_clear = if(direction == :up, do: :hall_up, else: :hall_down) + + remaining_floor_orders = orders |> Map.get(floor, MapSet.new()) |> MapSet.delete(:cab) - - remaining_floor_orders = - case direction do - :up -> MapSet.delete(floor_orders_after_cab_clear, :hall_up) - :down -> MapSet.delete(floor_orders_after_cab_clear, :hall_down) - end + |> MapSet.delete(hall_button_to_clear) if MapSet.size(remaining_floor_orders) == 0, do: Map.delete(orders, floor), diff --git a/test/single/cost_test.exs b/test/single/cost_test.exs index 784792f..ed46345 100644 --- a/test/single/cost_test.exs +++ b/test/single/cost_test.exs @@ -50,6 +50,26 @@ defmodule Test.Single.CostTest do assert Cost.compute_cost({1, :hall_up}, %{}) == Simulation.unreachable_cost() end + test "merge_cost keeps higher cost for overlapping nodes" do + merged = + Cost.merge_cost( + %{node1: 100, node2: 200}, + %{node1: 150, node3: 50} + ) + + assert merged == %{node1: 150, node2: 200, node3: 50} + end + + test "merge_cost includes all non-overlapping nodes" do + merged = + Cost.merge_cost( + %{node1: 10}, + %{node2: 20} + ) + + assert merged == %{node1: 10, node2: 20} + end + defp set_state(opts) do State.set_direction(Keyword.fetch!(opts, :direction)) State.set_behavior(Keyword.fetch!(opts, :behavior)) From 023e38024610ed07ec228f7fce616ec0671cdf35 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 13:42:05 +0100 Subject: [PATCH 23/29] chore: remove handle_* specs --- lib/elevator/hall_orders.ex | 43 ++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 35b7671..846f4cd 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -5,6 +5,7 @@ defmodule Elevator.HallOrders do - Button is pressed. - Arrived at floor. - Received hall orders from another node. + - A barrier set gets full, which advances the state. """ alias Elevator.HallOrders.Order alias Elevator.HallOrders.Cost @@ -70,7 +71,7 @@ defmodule Elevator.HallOrders do do: GenServer.cast(__MODULE__, {:button_press, floor, button_type}) @doc """ - Goes back to idle if the order is in handling. + Advances the order to arrived if it is in handling. """ @spec arrived_at_floor(floor(), :up | :down) :: :ok def arrived_at_floor(floor, direction), @@ -99,9 +100,7 @@ defmodule Elevator.HallOrders do @impl true def handle_call(:get_my_orders, _from, order_map) do - alive = Communicator.who_can_serve() - - my_orders = my_orders_from_order_map(order_map, alive) + my_orders = my_orders_from_order_map(order_map) {:reply, my_orders, order_map} end @@ -122,28 +121,26 @@ defmodule Elevator.HallOrders do # Casts -------------------------------------------------- @impl true - @spec handle_cast({:receive_external, hall_order_map()}, hall_order_map()) :: - {:noreply, hall_order_map(), {:continue, :hall_update_state}} def handle_cast({:receive_external, other_order_map}, order_map) do - who_can_serve = Communicator.who_can_serve() - my_orders = my_orders_from_order_map(order_map, who_can_serve) + my_orders = my_orders_from_order_map(order_map) new_order_map = - Map.keys(order_map) - |> Enum.map(fn key -> + Map.new(order_map, fn {hall_button, order_state} -> new_value = - Order.update_from_incoming(key, order_map[key], other_order_map[key], my_orders) - - {key, new_value} + Order.update_from_incoming( + hall_button, + order_state, + other_order_map[hall_button], + my_orders + ) + + {hall_button, new_value} end) - |> Enum.into(%{}) {:noreply, new_order_map, {:continue, :hall_update_state}} end @impl true - @spec handle_cast({:button_press, floor(), hall_button()}, hall_order_map()) :: - {:noreply, hall_order_map(), {:continue, :hall_update_state}} def handle_cast({:button_press, floor, direction}, order_map) do new_order_map = Map.update!(order_map, {floor, direction}, &Order.update_from_button_press/1) {:noreply, new_order_map, {:continue, :hall_update_state}} @@ -151,7 +148,6 @@ defmodule Elevator.HallOrders do @impl true def handle_cast({:arrived_at_floor, floor, direction}, order_map) do - # TODO: Find out if barrier set should be full as well? button_type = [up: :hall_up, down: :hall_down][direction] new_order_map = @@ -172,12 +168,8 @@ defmodule Elevator.HallOrders do May advance some states, in which case continue is called until convergence. """ @impl true - @spec handle_continue(:hall_update_state, hall_order_map()) :: - {:noreply, hall_order_map()} - | {:noreply, hall_order_map(), {:continue, :hall_update_state}} def handle_continue(:hall_update_state, order_map) do - alive = Communicator.who_can_serve() - my_orders = my_orders_from_order_map(order_map, alive) + my_orders = my_orders_from_order_map(order_map) new_order_map = Map.new(order_map, fn {key, order_state} -> @@ -187,11 +179,14 @@ defmodule Elevator.HallOrders do {:noreply, new_order_map} end - defp my_orders_from_order_map(order_map, alive) do + @spec my_orders_from_order_map(hall_order_map()) :: %{floor() => MapSet.t(hall_button())} + defp my_orders_from_order_map(order_map) do + who_can_serve = Communicator.who_can_serve() + Enum.filter(order_map, fn {_, order_state} -> case order_state do {:handling, cost_map} -> - Cost.min_alive_cost(cost_map, alive) == Node.self() + Cost.min_alive_cost(cost_map, who_can_serve) == Node.self() _ -> false From 4b3117c4202af8b8eac7b32e2d3e0af441b3fcbc Mon Sep 17 00:00:00 2001 From: DxMarch Date: Thu, 19 Mar 2026 14:40:03 +0100 Subject: [PATCH 24/29] fix: integer division --- lib/elevator/hall_orders/simulation.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/elevator/hall_orders/simulation.ex b/lib/elevator/hall_orders/simulation.ex index bef5e45..57e5316 100644 --- a/lib/elevator/hall_orders/simulation.ex +++ b/lib/elevator/hall_orders/simulation.ex @@ -37,10 +37,10 @@ defmodule Elevator.HallOrders.Simulation do def initial_time_ms(elevator_state, target_floor) do cond do elevator_state.behavior == :door_open and target_floor != elevator_state.floor -> - Elevator.door_open_duration_ms() / 2 + div(Elevator.door_open_duration_ms(), 2) elevator_state.behavior == :moving -> - @travel_duration_ms / 2 + div(@travel_duration_ms, 2) true -> 0 From 9649434d17afa3a1caa634ea6b4885649cbaf00a Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 15:39:06 +0100 Subject: [PATCH 25/29] chore: refactor --- lib/elevator/fsm/state.ex | 12 ++--- lib/elevator/hall_orders.ex | 20 ++++--- lib/elevator/hall_orders/cost.ex | 27 +++------- lib/elevator/hall_orders/simulation.ex | 72 ++++++++------------------ 4 files changed, 49 insertions(+), 82 deletions(-) diff --git a/lib/elevator/fsm/state.ex b/lib/elevator/fsm/state.ex index 6d610d4..4fc5ecb 100644 --- a/lib/elevator/fsm/state.ex +++ b/lib/elevator/fsm/state.ex @@ -7,14 +7,14 @@ defmodule Elevator.FSM.State do """ require Logger - defstruct direction: :down, - behavior: :moving, - floor: :unknown, + defstruct behavior: :moving, between_floors: true, - obstructed: false, - motor_timed_out: false, + direction: :down, door_open_time_ms: Time.utc_now(), - last_floor_time: Time.utc_now() + floor: :unknown, + last_floor_time: Time.utc_now(), + motor_timed_out: false, + obstructed: false @type floor :: Elevator.floor() @type elev_behavior :: :moving | :idle | :door_open diff --git a/lib/elevator/hall_orders.ex b/lib/elevator/hall_orders.ex index 846f4cd..21637c3 100644 --- a/lib/elevator/hall_orders.ex +++ b/lib/elevator/hall_orders.ex @@ -149,9 +149,12 @@ defmodule Elevator.HallOrders do @impl true def handle_cast({:arrived_at_floor, floor, direction}, order_map) do button_type = [up: :hall_up, down: :hall_down][direction] + hall_button = {floor, button_type} new_order_map = - Map.update!(order_map, {floor, button_type}, &Order.update_from_arrived_at_floor/1) + if Map.has_key?(order_map, hall_button), + do: Map.update!(order_map, {floor, button_type}, &Order.update_from_arrived_at_floor/1), + else: order_map {:noreply, new_order_map, {:continue, :hall_update_state}} end @@ -179,17 +182,18 @@ defmodule Elevator.HallOrders do {:noreply, new_order_map} end + # Return the orders where we have the lowest cost among serving nodes. + # Only consider orders where all serving nodes have a cost. @spec my_orders_from_order_map(hall_order_map()) :: %{floor() => MapSet.t(hall_button())} defp my_orders_from_order_map(order_map) do who_can_serve = Communicator.who_can_serve() Enum.filter(order_map, fn {_, order_state} -> - case order_state do - {:handling, cost_map} -> - Cost.min_alive_cost(cost_map, who_can_serve) == Node.self() - - _ -> - false + with {:handling, cost_map} <- order_state do + MapSet.subset?(who_can_serve, MapSet.new(Map.keys(cost_map))) and + Cost.assigned_to_me?(cost_map, who_can_serve) + else + _ -> false end end) |> orders_by_floor() @@ -197,7 +201,7 @@ defmodule Elevator.HallOrders do @type enum_orders :: hall_order_map() - | Enumerable.t({hall_button(), hall_order_state()}) + | Enumerable.t({hall_button(), any()}) @spec orders_by_floor(enum_orders()) :: %{floor() => MapSet.t(hall_button())} defp orders_by_floor(orders) do # Restructure order map to the format floor => MapSet(button_type) diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index 0b8db61..a1d1dac 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -57,26 +57,15 @@ defmodule Elevator.HallOrders.Cost do end @doc """ - Returns the node with the lowest cost for a given cost map and alive set. + Returns if we are supposed to take the order given the cost map. + Assumes who_can_serve is a subset of cost_map keys. """ - @spec min_alive_cost(cost_map(), 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) + @spec assigned_to_me?(cost_map(), MapSet.t(node())) :: node() + def assigned_to_me?(cost_map, who_can_serve) do + {min_node, _} = + Enum.filter(cost_map, fn {node, _} -> MapSet.member?(who_can_serve, node) end) + |> Enum.min_by(fn {node, cost} -> {cost, node} end, fn -> {:infinity, :nonode@nohost} end) - if Enum.count(alive_costs) != MapSet.size(alive_set) do - nil - else - {min_node, _} = - Enum.min( - alive_costs, - fn {node1, cost1}, {node2, cost2} -> - cost1 < cost2 or (cost1 == cost2 and node1 < node2) - end, - # Fallback when no alive costs exist - fn -> {:nonode@nohost, :infinity} end - ) - - min_node - end + min_node == Node.self() end end diff --git a/lib/elevator/hall_orders/simulation.ex b/lib/elevator/hall_orders/simulation.ex index 57e5316..5b77039 100644 --- a/lib/elevator/hall_orders/simulation.ex +++ b/lib/elevator/hall_orders/simulation.ex @@ -58,30 +58,24 @@ defmodule Elevator.HallOrders.Simulation do def simulate_time_until_served(orders, elevator_state, target) do initial_time_ms = initial_time_ms(elevator_state, elem(target, 0)) - if target_cleared?(orders, target) do - 0 - else - sim_state = %SimState{ - orders: orders, - elevator_state: elevator_state, - target: target, - time_ms: initial_time_ms, - steps_left: @max_simulation_steps - } - - do_simulate(sim_state) - end + sim_state = %SimState{ + orders: orders, + elevator_state: elevator_state, + target: target, + time_ms: initial_time_ms, + steps_left: @max_simulation_steps + } + + do_simulate(sim_state) end @spec do_simulate(simulation()) :: non_neg_integer() defp do_simulate(%SimState{steps_left: 0}), do: @unreachable_cost defp do_simulate(%SimState{orders: orders, target: target, time_ms: time_ms} = sim_state) do - if target_cleared?(orders, target) do - time_ms - else - do_simulate_step(sim_state) - end + if target_cleared?(orders, target), + do: time_ms, + else: do_simulate_step(sim_state) end @spec do_simulate_step(simulation()) :: non_neg_integer() @@ -90,30 +84,26 @@ defmodule Elevator.HallOrders.Simulation do {next_orders, next_elevator_state, delta_ms} = case behavior do - :idle -> - raise( - "Invalid simulation transition: got :idle while target still pending. sim_state=#{inspect(sim_state)}" - ) - :moving -> - {:ok, next_floor} = move_one_floor(elevator_state.floor, direction) + step = [up: 1, down: -1][direction] + next_floor = elevator_state.floor + step {orders, %{ elevator_state - | floor: next_floor, + | behavior: :moving, between_floors: false, direction: direction, - behavior: :moving + floor: next_floor }, @travel_duration_ms} :door_open -> {clear_requests_at_floor_in_direction(orders, elevator_state.floor, direction), %{ elevator_state - | direction: direction, - behavior: :idle, - between_floors: false + | behavior: :idle, + between_floors: false, + direction: direction }, Elevator.door_open_duration_ms()} end @@ -135,29 +125,13 @@ defmodule Elevator.HallOrders.Simulation do |> Kernel.not() end - defp move_one_floor(floor, :up) do - if floor < Elevator.num_floors() - 1, - do: {:ok, floor + 1}, - else: raise("Invalid simulation transition: cannot move up from floor #{inspect(floor)}") - end - - defp move_one_floor(floor, :down) do - if floor > 0, - do: {:ok, floor - 1}, - else: raise("Invalid simulation transition: cannot move down from floor #{inspect(floor)}") - end - defp clear_requests_at_floor_in_direction(orders, floor, direction) do - hall_button_to_clear = if(direction == :up, do: :hall_up, else: :hall_down) + hall_button_to_clear = [up: :hall_up, down: :hall_down][direction] - remaining_floor_orders = - orders - |> Map.get(floor, MapSet.new()) + Map.update(orders, floor, MapSet.new(), fn floor_orders -> + floor_orders |> MapSet.delete(:cab) |> MapSet.delete(hall_button_to_clear) - - if MapSet.size(remaining_floor_orders) == 0, - do: Map.delete(orders, floor), - else: Map.put(orders, floor, remaining_floor_orders) + end) end end From 1f7147f08a7439765926fd70163595c02b703208 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 16:05:14 +0100 Subject: [PATCH 26/29] chore: Decision -> Transition + OrderUtils --- lib/elevator.ex | 4 -- lib/elevator/decision.ex | 86 -------------------------- lib/elevator/fsm/transition.ex | 78 ++++++++++++++++++++--- lib/elevator/hall_orders/cost.ex | 19 ++---- lib/elevator/hall_orders/simulation.ex | 10 +-- lib/elevator/hardware/outputs.ex | 2 +- lib/elevator/order_utils.ex | 31 ++++++++++ test/single/cost_test.exs | 8 +-- test/single/decision_test.exs | 28 ++++----- 9 files changed, 132 insertions(+), 134 deletions(-) delete mode 100644 lib/elevator/decision.ex create mode 100644 lib/elevator/order_utils.ex diff --git a/lib/elevator.ex b/lib/elevator.ex index 3ff73cf..b217aac 100644 --- a/lib/elevator.ex +++ b/lib/elevator.ex @@ -3,10 +3,6 @@ defmodule Elevator do @type button_type :: :cab | Elevator.HallOrders.hall_button_type() - @type combined_order_map :: %{ - floor() => MapSet.t(button_type()) - } - @num_floors 4 @door_open_duration_ms 1000 diff --git a/lib/elevator/decision.ex b/lib/elevator/decision.ex deleted file mode 100644 index 186e123..0000000 --- a/lib/elevator/decision.ex +++ /dev/null @@ -1,86 +0,0 @@ -defmodule Elevator.Decision do - @moduledoc """ - Pure functions for elevator request manipulation. - - These functions are intentionally pure to make them easy to unit test. - """ - - @spec requests_above?(Elevator.combined_order_map(), Elevator.floor()) :: boolean() - def requests_above?(reqs, floor) do - Enum.any?(reqs, fn {f, _} -> f > floor end) - end - - @spec requests_below?(Elevator.combined_order_map(), Elevator.floor()) :: boolean() - def requests_below?(reqs, floor) do - Enum.any?(reqs, fn {f, _} -> f < floor end) - end - - @spec combine_hall_and_cab( - Elevator.combined_order_map(), - MapSet.t(Elevator.floor()) - ) :: Elevator.combined_order_map() - def combine_hall_and_cab(hall_orders, cab_floors) do - Enum.reduce(cab_floors, hall_orders, fn floor, acc -> - Map.update(acc, floor, MapSet.new([:cab]), &MapSet.put(&1, :cab)) - end) - end - - @doc "Single decision function for elevator behavior. - Returns both direction and behavior for the current state and order snapshot." - @spec next_action(Elevator.combined_order_map(), Elevator.FSM.State.t()) :: - {:up | :down, :moving | :door_open | :idle} - def next_action( - orders, - %Elevator.FSM.State{ - direction: direction, - floor: floor, - between_floors: between_floors - } - ) do - btns_at_floor = Map.get(orders, floor, MapSet.new()) - - cond do - between_floors -> - {direction, :moving} - - map_size(orders) == 0 -> - {direction, :idle} - - direction == :up -> - cond do - MapSet.member?(btns_at_floor, :hall_up) or MapSet.member?(btns_at_floor, :cab) -> - {:up, :door_open} - - requests_above?(orders, floor) -> - {:up, :moving} - - MapSet.member?(btns_at_floor, :hall_down) -> - {:down, :door_open} - - requests_below?(orders, floor) -> - {:down, :moving} - - true -> - {:up, :idle} - end - - direction == :down -> - cond do - MapSet.member?(btns_at_floor, :hall_down) or MapSet.member?(btns_at_floor, :cab) -> - {:down, :door_open} - - requests_below?(orders, floor) -> - {:down, :moving} - - MapSet.member?(btns_at_floor, :hall_up) -> - {:up, :door_open} - - requests_above?(orders, floor) -> - {:up, :moving} - - true -> - {:down, :idle} - end - end - end -end diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 27acf22..71939c9 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -11,8 +11,8 @@ defmodule Elevator.FSM.Transition do alias Elevator.CabOrders alias Elevator.FSM.State alias Elevator.HallOrders - alias Elevator.Decision alias Elevator.Hardware.Outputs + alias Elevator.OrderUtils @motor_timeout_ms 3500 @transition_interval_ms 100 @@ -35,6 +35,69 @@ defmodule Elevator.FSM.Transition do } end + @doc """ + Computes the elevator's next `{direction, behavior}` pair + from current orders and elevator state. + + Tries to keep moving in the same direction. + """ + @spec next_action(Elevator.OrderUtils.combined_order_map(), Elevator.FSM.State.t()) :: + {:up | :down, :moving | :door_open | :idle} + def next_action( + orders, + %Elevator.FSM.State{ + direction: direction, + floor: floor, + between_floors: between_floors + } + ) do + btns_at_floor = Map.get(orders, floor, MapSet.new()) + + cond do + between_floors -> + {direction, :moving} + + map_size(orders) == 0 -> + {direction, :idle} + + direction == :up -> + cond do + MapSet.member?(btns_at_floor, :hall_up) or MapSet.member?(btns_at_floor, :cab) -> + {:up, :door_open} + + OrderUtils.orders_above?(orders, floor) -> + {:up, :moving} + + MapSet.member?(btns_at_floor, :hall_down) -> + {:down, :door_open} + + OrderUtils.orders_below?(orders, floor) -> + {:down, :moving} + + true -> + {:up, :idle} + end + + direction == :down -> + cond do + MapSet.member?(btns_at_floor, :hall_down) or MapSet.member?(btns_at_floor, :cab) -> + {:down, :door_open} + + OrderUtils.orders_below?(orders, floor) -> + {:down, :moving} + + MapSet.member?(btns_at_floor, :hall_up) -> + {:up, :door_open} + + OrderUtils.orders_above?(orders, floor) -> + {:up, :moving} + + true -> + {:down, :idle} + end + end + end + defp loop() do check_door_timer(State.get_state()) check_motor_timeout(State.get_state()) @@ -49,19 +112,20 @@ defmodule Elevator.FSM.Transition do defp get_my_orders() do hall_orders = HallOrders.get_my_orders() - pressed_cab_floors = CabOrders.get_my_orders() - Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) + cab_orders = CabOrders.get_my_orders() + OrderUtils.combine_hall_and_cab(hall_orders, cab_orders) end defp get_light_orders() do hall_orders = HallOrders.get_handling_orders() - pressed_cab_floors = CabOrders.get_my_orders() - Decision.combine_hall_and_cab(hall_orders, pressed_cab_floors) + cab_orders = CabOrders.get_my_orders() + OrderUtils.combine_hall_and_cab(hall_orders, cab_orders) end - @spec decide_and_update_state(Elevator.FSM.State.t(), Elevator.combined_order_map()) :: any() + @spec decide_and_update_state(Elevator.FSM.State.t(), Elevator.OrderUtils.combined_order_map()) :: + any() defp decide_and_update_state(state, orders) when not state.motor_timed_out do - {new_direction, new_behavior} = Decision.next_action(orders, state) + {new_direction, new_behavior} = next_action(orders, state) cond do state.behavior == :door_open -> diff --git a/lib/elevator/hall_orders/cost.ex b/lib/elevator/hall_orders/cost.ex index a1d1dac..4ef4722 100644 --- a/lib/elevator/hall_orders/cost.ex +++ b/lib/elevator/hall_orders/cost.ex @@ -2,12 +2,12 @@ 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. + Cost is estimated by simulating the local elevator with current orders plus the candidate hall order. """ alias Elevator.CabOrders - alias Elevator.Decision alias Elevator.FSM.State + alias Elevator.OrderUtils alias Elevator.HallOrders.Simulation require Logger @@ -16,7 +16,7 @@ defmodule Elevator.HallOrders.Cost do @type cost_map :: Elevator.HallOrders.hall_order_cost_map() @doc """ - Comupte the cost (time to serve) of a canditate hall order by simulating single elevator logic. + Comupte the cost (time to serve) of a candidate hall order by simulating single elevator logic. """ @spec compute_cost({floor(), hall_button_type()}, %{floor() => MapSet.t(hall_button_type())}) :: non_neg_integer() @@ -24,7 +24,7 @@ defmodule Elevator.HallOrders.Cost do state = State.get_state() cab_orders = CabOrders.get_my_orders() - # Include the candidate hall request in our local hall-order snapshot before simulating. + # Include the candidate hall order in our local hall-order snapshot before simulating. hall_orders_with_candidate = Map.update( my_hall_orders, @@ -33,16 +33,9 @@ defmodule Elevator.HallOrders.Cost do &MapSet.put(&1, hall_button_type) ) - combined_orders = Decision.combine_hall_and_cab(hall_orders_with_candidate, cab_orders) + combined_orders = OrderUtils.combine_hall_and_cab(hall_orders_with_candidate, cab_orders) - result = - Simulation.simulate_time_until_served(combined_orders, state, {floor, hall_button_type}) - - Logger.debug(fn -> - "hall_cost request=#{inspect({floor, hall_button_type})} state=#{state.behavior}@#{inspect(state.floor)} dir=#{state.direction} result=#{result}" - end) - - result + Simulation.simulate_time_until_served(combined_orders, state, {floor, hall_button_type}) end @doc """ diff --git a/lib/elevator/hall_orders/simulation.ex b/lib/elevator/hall_orders/simulation.ex index 5b77039..459b61e 100644 --- a/lib/elevator/hall_orders/simulation.ex +++ b/lib/elevator/hall_orders/simulation.ex @@ -3,8 +3,8 @@ defmodule Elevator.HallOrders.Simulation do Pure hall-order cost simulation. """ - alias Elevator.Decision alias Elevator.FSM.State + alias Elevator.FSM.Transition @travel_duration_ms 2500 @max_simulation_steps 256 @@ -12,7 +12,7 @@ defmodule Elevator.HallOrders.Simulation do @type floor :: Elevator.floor() @type hall_button_type :: Elevator.HallOrders.hall_button_type() - @type combined_order_map :: Elevator.combined_order_map() + @type combined_order_map :: Elevator.OrderUtils.combined_order_map() defmodule SimState do @enforce_keys [:orders, :elevator_state, :target, :time_ms, :steps_left] @@ -80,7 +80,7 @@ defmodule Elevator.HallOrders.Simulation do @spec do_simulate_step(simulation()) :: non_neg_integer() defp do_simulate_step(%SimState{orders: orders, elevator_state: elevator_state} = sim_state) do - {direction, behavior} = Decision.next_action(orders, elevator_state) + {direction, behavior} = Transition.next_action(orders, elevator_state) {next_orders, next_elevator_state, delta_ms} = case behavior do @@ -98,7 +98,7 @@ defmodule Elevator.HallOrders.Simulation do }, @travel_duration_ms} :door_open -> - {clear_requests_at_floor_in_direction(orders, elevator_state.floor, direction), + {clear_orders_at_floor_in_direction(orders, elevator_state.floor, direction), %{ elevator_state | behavior: :idle, @@ -125,7 +125,7 @@ defmodule Elevator.HallOrders.Simulation do |> Kernel.not() end - defp clear_requests_at_floor_in_direction(orders, floor, direction) do + defp clear_orders_at_floor_in_direction(orders, floor, direction) do hall_button_to_clear = [up: :hall_up, down: :hall_down][direction] Map.update(orders, floor, MapSet.new(), fn floor_orders -> diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index aeb9993..1bea33b 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -14,7 +14,7 @@ defmodule Elevator.Hardware.Outputs do Driver.set_motor_direction(:stop) end - @spec set_outputs(FSM.State.t(), Elevator.combined_order_map()) :: any() + @spec set_outputs(FSM.State.t(), Elevator.OrderUtils.combined_order_map()) :: any() def set_outputs(state, light_orders) do set_door_light(state) set_motors(state) diff --git a/lib/elevator/order_utils.ex b/lib/elevator/order_utils.ex new file mode 100644 index 0000000..98212c0 --- /dev/null +++ b/lib/elevator/order_utils.ex @@ -0,0 +1,31 @@ +defmodule Elevator.OrderUtils do + @moduledoc """ + Pure functions for elevator order manipulation. + + These functions are intentionally pure to make them easy to unit test. + """ + + @type combined_order_map :: %{ + Elevator.floor() => MapSet.t(Elevator.button_type()) + } + + @spec orders_above?(combined_order_map(), Elevator.floor()) :: boolean() + def orders_above?(reqs, floor) do + Enum.any?(reqs, fn {f, _} -> f > floor end) + end + + @spec orders_below?(combined_order_map(), Elevator.floor()) :: boolean() + def orders_below?(reqs, floor) do + Enum.any?(reqs, fn {f, _} -> f < floor end) + end + + @spec combine_hall_and_cab( + combined_order_map(), + MapSet.t(Elevator.floor()) + ) :: combined_order_map() + def combine_hall_and_cab(hall_orders, cab_floors) do + Enum.reduce(cab_floors, hall_orders, fn floor, acc -> + Map.update(acc, floor, MapSet.new([:cab]), &MapSet.put(&1, :cab)) + end) + end +end diff --git a/test/single/cost_test.exs b/test/single/cost_test.exs index ed46345..9e5beaa 100644 --- a/test/single/cost_test.exs +++ b/test/single/cost_test.exs @@ -16,26 +16,26 @@ defmodule Test.Single.CostTest do :ok end - test "same-floor request in current direction costs one door cycle" do + test "same-floor order 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 + test "same-floor opposite order is served immediately when no further orders" 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 + test "one floor away order costs travel plus door time" do set_state(floor: 0, direction: :up, behavior: :idle) assert Cost.compute_cost({1, :hall_up}, %{}) == Simulation.travel_duration_ms() + Elevator.door_open_duration_ms() end - test "one floor away request includes half current open-door delay" do + test "one floor away order includes half current open-door delay" do set_state(floor: 0, direction: :up, behavior: :door_open) state = State.get_state() diff --git a/test/single/decision_test.exs b/test/single/decision_test.exs index 179a7c2..a00135d 100644 --- a/test/single/decision_test.exs +++ b/test/single/decision_test.exs @@ -1,71 +1,71 @@ defmodule Test.Single.DecisionTest do use ExUnit.Case, async: true - alias Elevator.Decision + alias Elevator.FSM.Transition 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) == {:down, :idle} + assert Transition.next_action(orders, state) == {:down, :idle} end - test "no requests -> keep direction,idle" do + test "no orders -> keep direction,idle" do orders = %{} state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :idle} + assert Transition.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: :up, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :moving} + assert Transition.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: :down, between_floors: false} - assert Decision.next_action(orders, state) == {:down, :moving} + assert Transition.next_action(orders, state) == {:down, :moving} end test "cab above -> up,moving" do orders = %{2 => MapSet.new([:cab])} state = %Elevator.FSM.State{floor: 1, direction: :down, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :moving} + assert Transition.next_action(orders, state) == {:up, :moving} end test "same floor cab while moving up -> up,door_open" do orders = %{2 => MapSet.new([:cab])} state = %Elevator.FSM.State{floor: 2, direction: :up, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :door_open} + assert Transition.next_action(orders, state) == {:up, :door_open} end 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: :up, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :door_open} + assert Transition.next_action(orders, state) == {:up, :door_open} end 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: :down, between_floors: false} - assert Decision.next_action(orders, state) == {:down, :door_open} + assert Transition.next_action(orders, state) == {:down, :door_open} end test "same floor hall_up while moving up -> up,door_open" do orders = %{1 => MapSet.new([:hall_up])} state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :door_open} + assert Transition.next_action(orders, state) == {:up, :door_open} end test "same floor hall_down while moving down -> down,door_open" do orders = %{1 => MapSet.new([:hall_down])} state = %Elevator.FSM.State{floor: 1, direction: :down, between_floors: false} - assert Decision.next_action(orders, state) == {:down, :door_open} + assert Transition.next_action(orders, state) == {:down, :door_open} end - test "same floor cab with requests above while moving up -> up,door_open" do + test "same floor cab with orders above while moving up -> up,door_open" do orders = %{1 => MapSet.new([:cab]), 3 => MapSet.new([:hall_up])} state = %Elevator.FSM.State{floor: 1, direction: :up, between_floors: false} - assert Decision.next_action(orders, state) == {:up, :door_open} + assert Transition.next_action(orders, state) == {:up, :door_open} end end From a173668d0ebd6a2c55a9be5a58acd971954810b4 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 16:18:26 +0100 Subject: [PATCH 27/29] chore: cleanup driver --- lib/elevator/hardware/driver.ex | 121 ++++++-------------------------- 1 file changed, 21 insertions(+), 100 deletions(-) diff --git a/lib/elevator/hardware/driver.ex b/lib/elevator/hardware/driver.ex index ef44fa8..fcf5d70 100644 --- a/lib/elevator/hardware/driver.ex +++ b/lib/elevator/hardware/driver.ex @@ -1,16 +1,20 @@ defmodule Elevator.Hardware.Driver do + @moduledoc """ + Handout driver from TTK4145 GitHub with minor modifications. + + Modifications: + - Don't crash immediately if tcp connection fails. Retry connection periodically instead. + - Added @spec declarations to public functions. + - Changed magic numbers to atoms where applicable. + """ 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} - - def start_link([]) do - start_link([{127, 0, 0, 1}, 15657]) - end + @button_map %{hall_up: 0, hall_down: 1, cab: 2} + @state_map %{on: 1, off: 0} + @direction_map %{up: 1, down: 255, stop: 0} def start_link([address, port]) do GenServer.start_link(__MODULE__, [address, port], name: __MODULE__) @@ -43,121 +47,52 @@ defmodule Elevator.Hardware.Driver do end # User API ---------------------------------------------- - @doc """ - Sets the motor direction. - - ## Parameters - - direction: :up, :down, or :stop - """ + @spec set_motor_direction(:up | :down | :stop) :: :ok def set_motor_direction(direction) do GenServer.cast(__MODULE__, {:set_motor_direction, direction}) end - @doc """ - Sets the light for an order button. - - ## Parameters - - button_type: :hall_up, :hall_down, or :cab - - floor: floor number (integer) - - state: :on or :off - """ + @spec set_order_button_light(:hall_up | :hall_down | :cab, non_neg_integer(), :on | :off) :: :ok def set_order_button_light(button_type, floor, state) do GenServer.cast(__MODULE__, {:set_order_button_light, button_type, floor, state}) end - @doc """ - Sets the floor indicator display. - - ## Parameters - - floor: floor number to display (integer) - """ + @spec set_floor_indicator(non_neg_integer()) :: :ok def set_floor_indicator(floor) do GenServer.cast(__MODULE__, {:set_floor_indicator, floor}) end - @doc """ - Sets the stop button light. - - ## Parameters - - state: :on or :off - """ + @spec set_stop_button_light(:on | :off) :: :ok def set_stop_button_light(state) do GenServer.cast(__MODULE__, {:set_stop_button_light, state}) end - @doc """ - Sets the door open indicator light. - - ## Parameters - - state: :on or :off - """ + @spec set_door_open_light(:on | :off) :: :ok def set_door_open_light(state) do GenServer.cast(__MODULE__, {:set_door_open_light, state}) end - @doc """ - Gets the state of an order button. - - ## Parameters - - floor: floor number (integer) - - button_type: :hall_up, :hall_down, or :cab - - ## Returns - - :active if button is pressed - - :inactive if button is not pressed - """ + @spec get_order_button_state(non_neg_integer(), :hall_up | :hall_down | :cab) :: + :active | :inactive def get_order_button_state(floor, button_type) do GenServer.call(__MODULE__, {:get_order_button_state, floor, button_type}) end - @doc """ - Gets the current floor from the floor sensor. - - ## Returns - - floor number (integer) if elevator is at a floor - - :between_floors if elevator is between floors - """ + @spec get_floor_sensor_state() :: non_neg_integer() | :between_floors def get_floor_sensor_state do GenServer.call(__MODULE__, :get_floor_sensor_state) end - @doc """ - Gets the state of the stop button. - - ## Returns - - :active if stop button is pressed - - :inactive if stop button is not pressed - """ + @spec get_stop_button_state() :: :active | :inactive def get_stop_button_state do GenServer.call(__MODULE__, :get_stop_button_state) end - @doc """ - Gets the state of the obstruction switch (door sensor). - - ## Returns - - :active if obstruction is detected - - :inactive if no obstruction - """ + @spec get_obstruction_switch_state() :: :active | :inactive def get_obstruction_switch_state do GenServer.call(__MODULE__, :get_obstruction_switch_state) end - @doc """ - Checks if the driver GenServer is running. - - Note: This only checks if the GenServer process is alive, not if the TCP - connection is active. The driver will crash and restart automatically - if the connection fails during operation. - - ## Returns - - :ok if the GenServer is running - - {:error, :not_connected} if the state is not a valid socket (should rarely occur) - """ - def ping do - GenServer.call(__MODULE__, :ping) - end - # Casts ---------------------------------------------- @impl true def handle_cast({:set_motor_direction, direction}, {socket, addr, port}) do @@ -197,7 +132,6 @@ defmodule Elevator.Hardware.Driver do 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 @@ -208,7 +142,6 @@ defmodule Elevator.Hardware.Driver do 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 @@ -219,7 +152,6 @@ defmodule Elevator.Hardware.Driver do 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 @@ -230,17 +162,6 @@ defmodule Elevator.Hardware.Driver do 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, addr, port}) when is_port(socket) do - {:reply, :ok, {socket, addr, port}} - end - - @impl true - def handle_call(:ping, _from, state) do - {:reply, {:error, :not_connected}, state} - end end From cf820c8e653bf94e33848c892f42871534fd3ed8 Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 16:27:07 +0100 Subject: [PATCH 28/29] chore: smaating --- lib/elevator/fsm/transition.ex | 10 +++++----- lib/elevator/hardware/input_poller.ex | 12 ++++++------ lib/elevator/hardware/outputs.ex | 20 +++++++++----------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/lib/elevator/fsm/transition.ex b/lib/elevator/fsm/transition.ex index 71939c9..29dc200 100644 --- a/lib/elevator/fsm/transition.ex +++ b/lib/elevator/fsm/transition.ex @@ -51,7 +51,7 @@ defmodule Elevator.FSM.Transition do between_floors: between_floors } ) do - btns_at_floor = Map.get(orders, floor, MapSet.new()) + buttons_at_floor = Map.get(orders, floor, MapSet.new()) cond do between_floors -> @@ -62,13 +62,13 @@ defmodule Elevator.FSM.Transition do direction == :up -> cond do - MapSet.member?(btns_at_floor, :hall_up) or MapSet.member?(btns_at_floor, :cab) -> + MapSet.member?(buttons_at_floor, :hall_up) or MapSet.member?(buttons_at_floor, :cab) -> {:up, :door_open} OrderUtils.orders_above?(orders, floor) -> {:up, :moving} - MapSet.member?(btns_at_floor, :hall_down) -> + MapSet.member?(buttons_at_floor, :hall_down) -> {:down, :door_open} OrderUtils.orders_below?(orders, floor) -> @@ -80,13 +80,13 @@ defmodule Elevator.FSM.Transition do direction == :down -> cond do - MapSet.member?(btns_at_floor, :hall_down) or MapSet.member?(btns_at_floor, :cab) -> + MapSet.member?(buttons_at_floor, :hall_down) or MapSet.member?(buttons_at_floor, :cab) -> {:down, :door_open} OrderUtils.orders_below?(orders, floor) -> {:down, :moving} - MapSet.member?(btns_at_floor, :hall_up) -> + MapSet.member?(buttons_at_floor, :hall_up) -> {:up, :door_open} OrderUtils.orders_above?(orders, floor) -> diff --git a/lib/elevator/hardware/input_poller.ex b/lib/elevator/hardware/input_poller.ex index b8a892a..276aaae 100644 --- a/lib/elevator/hardware/input_poller.ex +++ b/lib/elevator/hardware/input_poller.ex @@ -53,13 +53,13 @@ defmodule Elevator.Hardware.InputPoller do schedule_button_poll() # Polls button and notifies Cab- and HallOrders if any are pressed - for floor <- 0..(Elevator.num_floors() - 1), btn <- get_pressed_buttons_at_floor(floor) do - case btn do + for floor <- 0..(Elevator.num_floors() - 1), button <- get_pressed_buttons_at_floor(floor) do + case button do :cab -> CabOrders.button_press(floor) - hall_btn -> - HallOrders.button_press(floor, hall_btn) + hall_button -> + HallOrders.button_press(floor, hall_button) end end @@ -70,8 +70,8 @@ defmodule Elevator.Hardware.InputPoller do defp get_pressed_buttons_at_floor(floor) do Elevator.button_types() - |> Enum.filter(fn btn -> - Driver.get_order_button_state(floor, btn) == :active + |> Enum.filter(fn button -> + Driver.get_order_button_state(floor, button) == :active end) end diff --git a/lib/elevator/hardware/outputs.ex b/lib/elevator/hardware/outputs.ex index 1bea33b..749bef1 100644 --- a/lib/elevator/hardware/outputs.ex +++ b/lib/elevator/hardware/outputs.ex @@ -1,6 +1,6 @@ defmodule Elevator.Hardware.Outputs do @moduledoc """ - Watches current state and controls the physical elevator. + Sets driver outputs given state and orders. """ require Logger @@ -22,10 +22,10 @@ defmodule Elevator.Hardware.Outputs do set_order_lights(light_orders) end - defp set_motors(elev_state) do - case elev_state.behavior do + defp set_motors(elevator_state) do + case elevator_state.behavior do :moving -> - Driver.set_motor_direction(elev_state.direction) + Driver.set_motor_direction(elevator_state.direction) _ -> Driver.set_motor_direction(:stop) @@ -39,16 +39,14 @@ defmodule Elevator.Hardware.Outputs do end defp set_order_lights(orders) do - for floor <- 0..(Elevator.num_floors() - 1), btn <- Elevator.button_types() do - lights = Map.get(orders, floor, MapSet.new()) - state = if MapSet.member?(lights, btn), do: :on, else: :off - Driver.set_order_button_light(btn, floor, state) + for floor <- 0..(Elevator.num_floors() - 1), button <- Elevator.button_types() do + orders_at_floor = Map.get(orders, floor, MapSet.new()) + state = if MapSet.member?(orders_at_floor, button), do: :on, else: :off + Driver.set_order_button_light(button, floor, state) end end - defp set_door_light(elev_state) do - behavior = elev_state.behavior - + defp set_door_light(%{behavior: behavior} = _elevator_state) do case behavior do :door_open -> Driver.set_door_open_light(:on) From 7e16167586ba1bf1348ef9f40bf543631fd8fe5e Mon Sep 17 00:00:00 2001 From: Magnus Date: Thu, 19 Mar 2026 16:33:53 +0100 Subject: [PATCH 29/29] feat: mix docs --- mix.exs | 3 ++- mix.lock | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 91e1781..1eb7cf8 100644 --- a/mix.exs +++ b/mix.exs @@ -28,7 +28,8 @@ 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} + {:pre_commit, "~> 0.3.4", only: :dev}, + {:ex_doc, "~> 0.34", only: :dev, runtime: false} ] end diff --git a/mix.lock b/mix.lock index be5b680..67adbc4 100644 --- a/mix.lock +++ b/mix.lock @@ -1,7 +1,13 @@ %{ "dotenvy": {:hex, :dotenvy, "1.0.1", "2d5204e22e44a640c6f43f3a90035fbb65af281e1a2967be1dab63eee87aeabc", [:mix], [], "hexpm", "0727f6c08636b6d6f935c5a0ccedfe0c05bc75e638683bd9fa0a26d7a9931d15"}, + "earmark_parser": {:hex, :earmark_parser, "1.4.44", "f20830dd6b5c77afe2b063777ddbbff09f9759396500cdbe7523efd58d7a339c", [:mix], [], "hexpm", "4778ac752b4701a5599215f7030989c989ffdc4f6df457c5f36938cc2d2a2750"}, + "ex_doc": {:hex, :ex_doc, "0.40.1", "67542e4b6dde74811cfd580e2c0149b78010fd13001fda7cfeb2b2c2ffb1344d", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "bcef0e2d360d93ac19f01a85d58f91752d930c0a30e2681145feea6bd3516e00"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "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"}, + "makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"}, + "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"}, + "makeup_erlang": {:hex, :makeup_erlang, "1.0.3", "4252d5d4098da7415c390e847c814bad3764c94a814a0b4245176215615e1035", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "953297c02582a33411ac6208f2c6e55f0e870df7f80da724ed613f10e6706afd"}, + "nimble_parsec": {:hex, :nimble_parsec, "1.4.2", "8efba0122db06df95bfaa78f791344a89352ba04baedd3849593bfce4d0dc1c6", [:mix], [], "hexpm", "4b21398942dda052b403bbe1da991ccd03a053668d147d53fb8c4e0efe09c973"}, "pre_commit": {:hex, :pre_commit, "0.3.4", "e2850f80be8090d50ad8019ef2426039307ff5dfbe70c736ad0d4d401facf304", [:mix], [], "hexpm", "16f684ba4f1fed1cba6b19e082b0f8d696e6f1c679285fedf442296617ba5f4e"}, "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"}, }