Improve Ta timer

Author: mickel8

lib/conn_check_handler.ex

@@ -4,11 +4,6 @@ defmodule ExICE.ConnCheckHandler do
   alias ExICE.CandidatePair
   alias ExICE.Attribute.UseCandidate
 
-  @doc """
-  Called when timer Ta fires and a new checklist transaction can be performed.
-  """
-  @callback handle_checklist(ice_agent :: map()) :: map()
-
   @doc """
   Called when conn check request arrives.
   """

lib/controlled_handler.ex

@@ -8,50 +8,6 @@ defmodule ExICE.ControlledHandler do
   alias ExICE.{CandidatePair, Checklist, ICEAgent}
   alias ExICE.Attribute.UseCandidate
 
-  @impl true
-  def handle_checklist(state) do
-    case Checklist.get_next_pair(state.checklist) do
-      %CandidatePair{} = pair ->
-        Logger.debug("Sending conn check on pair: #{inspect(pair.id)}")
-        {pair, state} = ICEAgent.send_conn_check(pair, state)
-        put_in(state, [:checklist, pair.id], pair)
-
-      nil ->
-        cond do
-          # if we knew, the other side uses regular nomination
-          # we would move to the completed state as soon as we 
-          # received nomination request but because we don't
-          # know wheter the other side uses regular or aggressive
-          # nomination we have to be prepared for the case
-          # where there is selected pair and we are not in the completed 
-          Checklist.finished?(state.checklist) and state.gathering_state == :complete and
-            state.selected_pair != nil and state.eoc == true ->
-            Logger.debug("""
-            Finished all conn checks, there won't be any further local or remote candidates
-            and we have selected pair. Changing connection state to completed.
-            """)
-
-            ICEAgent.change_connection_state(:completed, state)
-
-          # if we know, there are won't be any remote (eoc==true) and 
-          # local (gathering_state==complete) candidates, we finished 
-          # performing all conn checks and there is no selected or valid pair
-          # (state.state==checking), move to the failed state
-          Checklist.finished?(state.checklist) and state.gathering_state == :complete and
-            state.eoc == true and state.state == :checking ->
-            Logger.debug("""
-            Finished all conn checks, there won't be any further local or remote candidates
-            and we don't have any valid or selected pair. Changing connection state to failed.
-            """)
-
-            ICEAgent.change_connection_state(:failed, state)
-
-          true ->
-            state
-        end
-    end
-  end
-
   @impl true
   def handle_conn_check_request(state, pair, msg, nil, key) do
     ICEAgent.send_binding_success_response(pair, msg, key)
@@ -94,6 +50,10 @@ defmodule ExICE.ControlledHandler do
 
       %CandidatePair{} = pair ->
         if pair.state == :succeeded do
+          # FIXME we should check against valid? flag
+          # it's possible to have to pairs: one with state succeeded
+          # and flag valid? set to false, and the other one with state
+          # succeeded and flag valid? set to true
           # TODO should we call this selected or nominated pair
           Logger.debug("Nomination request on valid pair: #{pair.id}.")
           update_nominated_flag(state, pair.id, true)
@@ -139,27 +99,6 @@ defmodule ExICE.ControlledHandler do
           state
       end
 
-    checklist_finished? =
-      not (Checklist.in_progress?(state.checklist) or Checklist.waiting?(state.checklist))
-
-    state =
-      if state.eoc and checklist_finished? and state.gathering_state == :complete and
-           state.state != :completed do
-        # Assuming the controlling side uses regulard nomination, 
-        # the controlled side could move to the completed
-        # state as soon as it receives nomination request (or after 
-        # successful triggered check caused by nomination request).
-        # However, to be compatible with the older RFC's aggresive
-        # nomination, we wait for the end-of-candidates indication
-        # and checklist to be finished.
-        # This also means, that if the other side never sets eoc,
-        # we will never move to the completed state.
-        # This seems to be compliant with libwebrtc.
-        ICEAgent.change_connection_state(:completed, state)
-      else
-        state
-      end
-
     checklist =
       Map.update!(state.checklist, pair_id, fn pair ->
         %CandidatePair{pair | nominate?: false, nominated?: true}

lib/controlling_handler.ex

@@ -7,40 +7,6 @@ defmodule ExICE.ControllingHandler do
   alias ExICE.{CandidatePair, Checklist, ICEAgent}
   alias ExICE.Attribute.UseCandidate
 
-  @impl true
-  def handle_checklist(%{nominating?: {true, pair_id}} = state) do
-    case Map.fetch!(state.checklist, pair_id) do
-      %CandidatePair{valid?: false, state: :failed} ->
-        # pair that we tried to nominate timed out
-        Logger.debug("""
-        Pair we tried to nominate failed. Changing connection state to failed. Pair id: #{pair_id}.\
-        """)
-
-        ICEAgent.change_connection_state(:failed, state)
-
-      _ ->
-        state
-    end
-  end
-
-  @impl true
-  def handle_checklist(state) do
-    case Checklist.get_next_pair(state.checklist) do
-      %CandidatePair{} = pair ->
-        Logger.debug("Sending conn check on pair: #{inspect(pair.id)}")
-        {pair, state} = ICEAgent.send_conn_check(pair, state)
-        put_in(state, [:checklist, pair.id], pair)
-
-      nil ->
-        if ICEAgent.time_to_nominate?(state) do
-          Logger.debug("Time to nominate a pair! Looking for a best valid pair...")
-          ICEAgent.try_nominate(state)
-        else
-          state
-        end
-    end
-  end
-
   @impl true
   def handle_conn_check_request(state, pair, msg, %UseCandidate{}, _key) do
     Logger.debug("""
@@ -96,7 +62,6 @@ defmodule ExICE.ControllingHandler do
       state
       | checklist: checklist,
         nominating?: {false, nil},
-        state: :completed,
         selected_pair: Map.fetch!(checklist, pair_id)
     }
   end