Updates per code reviews

Use ICECandidateType instead of string
Combine two methods to one SetNAT1To1IPs
Resolves #835

ice_go.go

@@ -24,7 +24,7 @@ func (api *API) NewICEGatherer(opts ICEGatherOptions) (*ICEGatherer, error) {
 		api.settingEngine.candidates.ICENetworkTypes,
 		api.settingEngine.candidates.InterfaceFilter,
 		api.settingEngine.candidates.NAT1To1IPs,
-		api.settingEngine.candidates.NAT1To1IPCandidate,
+		api.settingEngine.candidates.NAT1To1IPCandidateType,
 		opts,
 	)
 }

icegatherer.go

@@ -62,7 +62,7 @@ func NewICEGatherer(
 	networkTypes []NetworkType,
 	interfaceFilter func(string) bool,
 	nat1To1IPs []string,
-	nat1To1IPCandidate string,
+	nat1To1IPCandidateType ICECandidateType,
 	opts ICEGatherOptions,
 ) (*ICEGatherer, error) {
 	var validatedServers []*ice.URL
@@ -83,14 +83,14 @@ func NewICEGatherer(
 		candidateTypes = append(candidateTypes, ice.CandidateTypeRelay)
 	}
 
-	var nat1To1IPCandidateType ice.CandidateType
+	var nat1To1CandiTyp ice.CandidateType
-	switch nat1To1IPCandidate {
+	switch nat1To1IPCandidateType {
-	case "host":
+	case ICECandidateTypeHost:
-		nat1To1IPCandidateType = ice.CandidateTypeHost
+		nat1To1CandiTyp = ice.CandidateTypeHost
-	case "srflx":
+	case ICECandidateTypeSrflx:
-		nat1To1IPCandidateType = ice.CandidateTypeServerReflexive
+		nat1To1CandiTyp = ice.CandidateTypeServerReflexive
 	default:
-		nat1To1IPCandidateType = ice.CandidateTypeUnspecified
+		nat1To1CandiTyp = ice.CandidateTypeUnspecified
 	}
 
 	return &ICEGatherer{
@@ -113,7 +113,7 @@ func NewICEGatherer(
 		relayAcceptanceMinWait:    relayAcceptanceMinWait,
 		interfaceFilter:           interfaceFilter,
 		nat1To1IPs:                nat1To1IPs,
-		nat1To1IPCandidateType:    nat1To1IPCandidateType,
+		nat1To1IPCandidateType:    nat1To1CandiTyp,
 	}, nil
 }
 

icegatherer_test.go

@@ -23,7 +23,7 @@ func TestNewICEGatherer_Success(t *testing.T) {
 		ICEServers: []ICEServer{{URLs: []string{"stun:stun.l.google.com:19302"}}},
 	}
 
-	gatherer, err := NewICEGatherer(0, 0, nil, nil, nil, nil, nil, nil, nil, logging.NewDefaultLoggerFactory(), false, false, nil, func(string) bool { return true }, nil, "", opts)
+	gatherer, err := NewICEGatherer(0, 0, nil, nil, nil, nil, nil, nil, nil, logging.NewDefaultLoggerFactory(), false, false, nil, func(string) bool { return true }, nil, 0, opts)
 	if err != nil {
 		t.Error(err)
 	}
@@ -72,7 +72,7 @@ func TestICEGather_LocalCandidateOrder(t *testing.T) {
 	}
 
 	to := time.Second
-	gatherer, err := NewICEGatherer(10000, 10010, &to, &to, &to, &to, &to, &to, &to, logging.NewDefaultLoggerFactory(), false, false, []NetworkType{NetworkTypeUDP4}, func(string) bool { return true }, nil, "", opts)
+	gatherer, err := NewICEGatherer(10000, 10010, &to, &to, &to, &to, &to, &to, &to, logging.NewDefaultLoggerFactory(), false, false, []NetworkType{NetworkTypeUDP4}, func(string) bool { return true }, nil, 0, opts)
 	if err != nil {
 		t.Error(err)
 	}
@@ -126,7 +126,7 @@ func TestNewICEGatherer_NAT1To1IP(t *testing.T) {
 			0, 0, nil, nil, nil, nil, nil, nil, nil,
 			logging.NewDefaultLoggerFactory(),
 			false, false, nil, func(string) bool { return true },
-			[]string{"1.2.3.4"}, "host", // <---- testing here
+			[]string{"1.2.3.4"}, ICECandidateTypeHost, // <---- testing here
 			opts)
 		if err != nil {
 			t.Error(err)
@@ -157,7 +157,7 @@ func TestNewICEGatherer_NAT1To1IP(t *testing.T) {
 			0, 0, nil, nil, nil, nil, nil, nil, nil,
 			logging.NewDefaultLoggerFactory(),
 			false, false, nil, func(string) bool { return true },
-			[]string{"4.5.6.7"}, "srflx", // <---- testing here
+			[]string{"4.5.6.7"}, ICECandidateTypeSrflx, // <---- testing here
 			opts)
 		if err != nil {
 			t.Error(err)
@@ -188,7 +188,7 @@ func TestNewICEGatherer_NAT1To1IP(t *testing.T) {
 			0, 0, nil, nil, nil, nil, nil, nil, nil,
 			logging.NewDefaultLoggerFactory(),
 			false, false, nil, func(string) bool { return true },
-			[]string{"6.6.6.6"}, "prflx", // <---- testing here
+			[]string{"6.6.6.6"}, ICECandidateTypePrflx, // <---- testing here
 			opts)
 		if err != nil {
 			t.Error(err)

settingengine.go

@@ -30,12 +30,12 @@ type SettingEngine struct {
 		ICERelayAcceptanceMinWait    *time.Duration
 	}
 	candidates struct {
-		ICELite            bool
+		ICELite                bool
-		ICETrickle         bool
+		ICETrickle             bool
-		ICENetworkTypes    []NetworkType
+		ICENetworkTypes        []NetworkType
-		InterfaceFilter    func(string) bool
+		InterfaceFilter        func(string) bool
-		NAT1To1IPs         []string
+		NAT1To1IPs             []string
-		NAT1To1IPCandidate string
+		NAT1To1IPCandidateType ICECandidateType
 	}
 	LoggerFactory logging.LoggerFactory
 }
@@ -117,27 +117,30 @@ func (e *SettingEngine) SetInterfaceFilter(filter func(string) bool) {
 	e.candidates.InterfaceFilter = filter
 }
 
-// SetNAT1To1IPs has a list of external IP addresses of 1:1 (D)NAT.
+// SetNAT1To1IPs sets a list of external IP addresses of 1:1 (D)NAT
+// and a candidate type for which the external IP address is used.
 // This is useful when you are host a server using Pion on an AWS EC2 instance
 // which has a private address, behind a 1:1 DNAT with a public IP (e.g.
 // Elastic IP). In this case, you can give the public IP address so that
-// Pion will use the public IP address in its candidate instead of the private IP
+// Pion will use the public IP address in its candidate instead of the private
-// address.
+// IP address. The second argument, candidateType, is used to tell Pion which
-func (e *SettingEngine) SetNAT1To1IPs(ips []string) {
-	e.candidates.NAT1To1IPs = ips
-}
-
-// SetNAT1To1IPCandidate is used along with SetNAT1To1IPs, to tell Pion which
 // type of candidate should use the given public IP address.
 // Two types of candidates are supported:
-// - "host": The public IP address will be used for the host candidate in the SDP.
+//
-// - "srflx": A server reflexive candidate with the given public IP address will be added
+// ICECandidateTypeHost:
-// to the SDP. If you choose "host", then the private IP address won't be advertised with
+//		The public IP address will be used for the host candidate in the SDP.
-// the peer. Also, this option cannot be used along with mDNS.
+// ICECandidateTypeSrflx:
-// If you choose "srflx", it simply adds a server reflexive candidate with the public IP.
+//		A server reflexive candidate with the given public IP address will be added
-// The host candidate is still available along with mDNS capabilities unaffected.
+// to the SDP.
-// Please note that you cannot give STUN server URL at the same time. It will result in
+//
-// an error otherwise.
+// Please note that if you choose ICECandidateTypeHost, then the private IP address
-func (e *SettingEngine) SetNAT1To1IPCandidate(candidate string) {
+// won't be advertised with the peer. Also, this option cannot be used along with mDNS.
-	e.candidates.NAT1To1IPCandidate = candidate
+//
+// If you choose ICECandidateTypeSrflx, it simply adds a server reflexive candidate
+// with the public IP. The host candidate is still available along with mDNS
+// capabilities unaffected. Also, you cannot give STUN server URL at the same time.
+// It will result in an error otherwise.
+func (e *SettingEngine) SetNAT1To1IPs(ips []string, candidateType ICECandidateType) {
+	e.candidates.NAT1To1IPs = ips
+	e.candidates.NAT1To1IPCandidateType = candidateType
 }

settingengine_test.go

@@ -61,3 +61,23 @@ func TestDetachDataChannels(t *testing.T) {
 		t.Fatalf("Failed to enable detached data channels.")
 	}
 }
+
+func TestSetNAT1To1IPs(t *testing.T) {
+	s := SettingEngine{}
+	if s.candidates.NAT1To1IPs != nil {
+		t.Errorf("Invalid default value")
+	}
+	if s.candidates.NAT1To1IPCandidateType != 0 {
+		t.Errorf("Invalid default value")
+	}
+
+	ips := []string{"1.2.3.4"}
+	typ := ICECandidateTypeHost
+	s.SetNAT1To1IPs(ips, typ)
+	if len(s.candidates.NAT1To1IPs) != 1 || s.candidates.NAT1To1IPs[0] != "1.2.3.4" {
+		t.Fatalf("Failed to set NAT1To1IPs")
+	}
+	if s.candidates.NAT1To1IPCandidateType != typ {
+		t.Fatalf("Failed to set NAT1To1IPCandidateType")
+	}
+}