[Asterisk-code-review] res pjsip: Match Asterisk 1.8 BLF behavior of chan sip (asterisk[13])

[Zach R]

Zach R has posted comments on this change. ( https://gerrit.asterisk.org/5484 )

Change subject: res_pjsip: Match Asterisk 1.8 BLF behavior of chan_sip
......................................................................


Patch Set 3:

> > Could this change break other endpoints that rely on the
 > > statestring being "confirmed" vs "early" for this case?
 > >
 > > In chan_sip there was an option, notifyringing, that depending
 > how
 > > set is used to switch between the two. Is a similar option needed
 > > here?
 > 
 > Bump.
 > 
 > Zach, were you planning on answering Kevin's question?

 > > Could this change break other endpoints that rely on the
 > > statestring being "confirmed" vs "early" for this case?
 > >
 > > In chan_sip there was an option, notifyringing, that depending
 > how
 > > set is used to switch between the two. Is a similar option needed
 > > here?
 > 
 > Bump.
 > 
 > Zach, were you planning on answering Kevin's question?

Sorry, I did not see the question as I have been a bit busy at work with things that had came up.

I don't believe PJSIP currently has any sort of yes or no flag that could be used to simulate the notifyringing option of chan_sip. I read up on that option since it wasn't too familiar to me so correct me if I am wrong on my understanding of the option. Instead of it controlling if the phone is sent a notify on ringing, it instead controls what the XML dialog has in the state information, early or confirmed.

So it is quite possible that if a specific phone does something on the confirmed vs early, such as how the Cisco SPA phones 

 > > Could this change break other endpoints that rely on the
 > > statestring being "confirmed" vs "early" for this case?
 > >
 > > In chan_sip there was an option, notifyringing, that depending
 > how
 > > set is used to switch between the two. Is a similar option needed
 > > here?
 > 
 > Bump.
 > 
 > Zach, were you planning on answering Kevin's question?

 > > Could this change break other endpoints that rely on the
 > > statestring being "confirmed" vs "early" for this case?
 > >
 > > In chan_sip there was an option, notifyringing, that depending
 > how
 > > set is used to switch between the two. Is a similar option needed
 > > here?
 > 
 > Bump.
 > 
 > Zach, were you planning on answering Kevin's question?

Sorry, I had not seen the question between the last time I had checked this for any new posts.

 

 > > Could this change break other endpoints that rely on the
 > > statestring being "confirmed" vs "early" for this case?
 > >
 > > In chan_sip there was an option, notifyringing, that depending
 > how
 > > set is used to switch between the two. Is a similar option needed
 > > here?
 > 
 > Bump.
 > 
 > Zach, were you planning on answering Kevin's question?

I read up on the notifyringing option as I wasn't fully sure I understood it, and from what I read it basically controlled whether a notify included confirmed or early as the state tag rather than controlling if it sent something on the ringing state. The thing I am not sure about related to this option, if the phone was idle and had this option set to no would it send confirmed or early for the first.

Currently I don't believe PJSIP has an option that is similar to the one from chan_sip. Such a new option would be necessary if you wanted a flag to switch the behavior on an endpoint between the two like chan_sip. 

Though I don't believe my patch will break any endpoints, since the default behavior in chan_sip was set to yes, so it would send early on ringing. 

The changes I proposed flip this from it currently replicating notifyringing=no to the default of chan_sip notifyringing=yes. 

Though it may be worth having the switch setting in pjsip endpoints in the off chance there is a phone or sip object out there that relies on confirmed vs early in the ringing && in use situation. As well it would be useful such that PJSIP would replicate how chan_sip truly behaved instead of just one or the other option.

-- 
To view, visit https://gerrit.asterisk.org/5484
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4326228f83a327a7510fecae0fec43c2945a3f25
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Zach R <zrothy at monmouth.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Zach R <zrothy at monmouth.com>
Gerrit-HasComments: No