Adjustable T1, T2 and T4 timeouts

[a12n]

These are constants now and can't be changed.

Probably, the values should be per client/server or per transaction. In the simplest case, they just can be made var. This would allow to change the values at startup. Something like this:

diff --git a/transaction/transaction.go b/transaction/transaction.go
index 3f446fb..cef8198 100644
--- a/transaction/transaction.go
+++ b/transaction/transaction.go
@@ -12,23 +12,27 @@ import (
 	"github.com/emiago/sipgo/sip"
 )
 
+var (
+	T1 = 500 * time.Millisecond
+	T2 = 4 * time.Second
+	T4 = 5 * time.Second
+)
+
+func Timer_A() time.Duration { return T1 }
+func Timer_B() time.Duration { return 64 * T1 }
+func Timer_E() time.Duration { return T1 }
+func Timer_F() time.Duration { return 64 * T1 }
+func Timer_G() time.Duration { return T1 }
+func Timer_H() time.Duration { return 64 * T1 }
+func Timer_I() time.Duration { return T4 }
+func Timer_J() time.Duration { return 64 * T1 }
+func Timer_K() time.Duration { return T4 }
+func Timer_L() time.Duration { return 64 * T1 }
+func Timer_M() time.Duration { return 64 * T1 }
+
 const (
-	T1        = 500 * time.Millisecond
-	T2        = 4 * time.Second
-	T4        = 5 * time.Second
-	Timer_A   = T1
-	Timer_B   = 64 * T1
 	Timer_D   = 32 * time.Second
-	Timer_E   = T1
-	Timer_F   = 64 * T1
-	Timer_G   = T1
-	Timer_H   = 64 * T1
-	Timer_I   = T4
-	Timer_J   = 64 * T1
-	Timer_K   = T4
 	Timer_1xx = 200 * time.Millisecond
-	Timer_L   = 64 * T1
-	Timer_M   = 64 * T1
 
 	TxSeperator = "__"
 )
diff --git a/transaction/client_tx.go b/transaction/client_tx.go
index 110b83b..37d428c 100644
--- a/transaction/client_tx.go
+++ b/transaction/client_tx.go
@@ -58,10 +58,10 @@ func (tx *ClientTx) Init() error {
 		// If a reliable transport is being used, the client transaction SHOULD NOT
 		// start timer A (Timer A controls request retransmissions).
 		// Timer A - retransmission
-		// tx.log.Tracef("timer_a set to %v", Timer_A)
+		// tx.log.Tracef("timer_a set to %v", Timer_A())
 
 		tx.mu.Lock()
-		tx.timer_a_time = Timer_A
+		tx.timer_a_time = Timer_A()
 
 		tx.timer_a = time.AfterFunc(tx.timer_a_time, func() {
 			tx.spinFsm(client_input_timer_a)
@@ -73,7 +73,7 @@ func (tx *ClientTx) Init() error {
 
 	// Timer B - timeout
 	tx.mu.Lock()
-	tx.timer_b = time.AfterFunc(Timer_B, func() {
+	tx.timer_b = time.AfterFunc(Timer_B(), func() {
 		tx.mu.Lock()
 		tx.lastErr = fmt.Errorf("Timer_B timed out. %w", ErrTimeout)
 		tx.mu.Unlock()
diff --git a/transaction/client_tx_fsm.go b/transaction/client_tx_fsm.go
index ac3d318..5d1b7be 100644
--- a/transaction/client_tx_fsm.go
+++ b/transaction/client_tx_fsm.go
@@ -320,13 +320,13 @@ func (tx *ClientTx) actCancelTimeout() FsmInput {
 
 	tx.cancel()
 
-	// tx.Log().Tracef("timer_b set to %v", Timer_B)
+	// tx.Log().Tracef("timer_b set to %v", Timer_B())
 
 	tx.mu.Lock()
 	if tx.timer_b != nil {
 		tx.timer_b.Stop()
 	}
-	tx.timer_b = time.AfterFunc(Timer_B, func() {
+	tx.timer_b = time.AfterFunc(Timer_B(), func() {
 		tx.spinFsm(client_input_timer_b)
 	})
 	tx.mu.Unlock()
@@ -401,9 +401,9 @@ func (tx *ClientTx) actPassupAccept() FsmInput {
 		tx.timer_b = nil
 	}
 
-	// tx.Log().Tracef("timer_m set to %v", Timer_M)
+	// tx.Log().Tracef("timer_m set to %v", Timer_M())
 
-	tx.timer_m = time.AfterFunc(Timer_M, func() {
+	tx.timer_m = time.AfterFunc(Timer_M(), func() {
 		select {
 		case <-tx.done:
 			return

The drawback here is the changed type of Timer_*, the values have to be functions on T1 and T4. Although the values are used only inside transaction package in sipgo they're still exported and available for external code.

Maybe, you have a better idea on making the timers configurable?

[emiago]

I would not go with this func. I would still like to know some use case of changing this values from default one?
Exposing just with var for me is enough, if someone needs to change one timer, he should look others as well.

[a12n]

I would still like to know some use case of changing this values from default one?

T1 is an estimate of RTT and it should be quite different when you connect to a node in the same data center or to a node on another continent. In the former case it should retry (and eventually fail and do failover) much earlier.

It's not that important for my current project, where I'm trying to go with sipgo. 500 ms is OK in this case. But there might be cases when it's a big deal.

I would not go with this func.

Yeah, it doesn't look nice. I just can't come up with a better way now. Maybe, introducing more ClientRequestOption for timers will be better.

[emiago]

thnx for sharing. Sure T1 is most important. So it is more I want to change T1 and let others be populated.

Maybe we can have this to be populated on package level
SetT1 SetT2 etc..

Let me know would this be enough. We can always export all timers, but maybe for now keeping package API small?

[emiago]

This waits too long. Adding priority

[emiago]

@a12n was delayed, but done.