Skip to content

Improve Ta timer #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions lib/conn_check_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
69 changes: 4 additions & 65 deletions lib/controlled_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}
Expand Down
35 changes: 0 additions & 35 deletions lib/controlling_handler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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("""
Expand Down Expand Up @@ -96,7 +62,6 @@ defmodule ExICE.ControllingHandler do
state
| checklist: checklist,
nominating?: {false, nil},
state: :completed,
selected_pair: Map.fetch!(checklist, pair_id)
}
end
Expand Down
Loading