No subject

e chan_sip.c portion of the diff has a lot of unrelated changes. I looked t=
hrough and found the changes you made to extensionstate_update(). Did you m=
ake any other changes in the latest version of the diff?

- Mark


On July 30, 2012, 4:43 a.m., Guenther Kelleter wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2048/
> -----------------------------------------------------------
> =

> (Updated July 30, 2012, 4:43 a.m.)
> =

> =

> Review request for Asterisk Developers, Mark Michelson, Matt Jordan, and =
Thomas Arimont.
> =

> =

> Summary
> -------
> =

> (For Digium: this is the implementation of the feature described in chapt=
er 2 of the document SIP_NOTIFY_dialog_event_infos_DATUS_v1_4.odt)
> =

> This patch extends the extension state callbacks so that monitoring chann=
els (as chan_sip) get more information of the devices which are responsible=
 for an extension state change. The additional information is needed by cha=
n_sip to present names/numbers of the caller and callee in an early-state S=
IP notification. Users of extenstion state callback not interested in the a=
dditional information are not affected by the changes.
> =

> Motivation: to present the involved party's name/number in an early-state=
 nofification (used by the notified device as a pickup offer) one after ano=
ther so that a user can see which call he will pick up in an undirected pic=
kup.
> Such a pickup offer to a user shall indicate the same call (number/name-A=
 calls number/name-B) as the call which would be picked up when an undirect=
ed pickup is executed.
> =

> =

> Users interested in additional state info must use the new functions ast_=
extension_state_add_extended() resp. ast_extension_state_add_destroy_extend=
ed() to register an extended state callback. When the callback is registere=
d this way, an extra member device_state_info of struct ast_state_cb_info i=
s passed to the callback in addition to the aggregated extension state. Thi=
s container holds an object for every device of the monitored extension hin=
t consisting of the device name, the device state and a channel reference t=
o the channel which (presumably) caused the device state.
> =

> The information is used by chan_sip for early-state notifications. When t=
he state of a device changes and the new state contains AST_EVENT_RINGING, =
an early-state notification is sent to the subscribed devices with the call=
er/callee names/numbers of the oldest ringing channel of the monitored exte=
nsion.
> The notified user may then invoke a direct pickup, which will pickup exac=
tly this channel.
> =

> Users of the old non-extended callbacks will only be called when the aggr=
egated state did change (same behavior as before). Users of the extended ca=
llback will also be called when the state is unchanged but does contain AST=
_EVENT_RINGING. That could be the case if two channels are ringing at one d=
evice and one of them hangs up, so the aggregated state does not change. Th=
is way the monitoring channel can create a new early-state notification wit=
h the now ringing party-ids.
> =

> This patch requires the channel creation time member of ast_channel and t=
he changes for the direct pickup to pick up the oldest ringing channel, whi=
ch are both introduced in review https://reviewboard.asterisk.org/r/2043/.
> =

> =

> Diffs
> -----
> =

>   /trunk/channels/chan_sip.c 370418 =

>   /trunk/channels/sip/include/sip.h 370418 =

>   /trunk/include/asterisk/pbx.h 370418 =

>   /trunk/main/pbx.c 370418 =

> =

> Diff: https://reviewboard.asterisk.org/r/2048/diff
> =

> =

> Testing
> -------
> =

> Checked the name/number of caller/callee displayed on the notified phone.
> Checked that a second call at an already ringing device does not invoke a=
 notification as long as the first call is ringing, instead it is sent when=
 the first ringing channel is picked up by some other party or hung up.
> Checked that the picked up caller is the same as advertized in the early-=
state notification when more than one calls are ringing on the subscribed e=
xtension.
> Ref counting for the containers and channels checked.
> Checked that a notification is sent to newly subscribed watchers.
> =

> =

> Thanks,
> =

> Guenther
> =

>


--===============0455946111505323350==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable




<html>
 <body>
  <div style=3D"font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor=3D"#f9f3c9" width=3D"100%" cellpadding=3D"8" style=3D"bor=
der: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href=3D"https://reviewboard.asterisk.org/r/2048/">https://reviewbo=
ard.asterisk.org/r/2048/</a>
     </td>
    </tr>
   </table>
   <br />





 <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-spa=
ce: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">From what =
I can tell, all the changes here are good. The problem is that the chan_sip=
.c portion of the diff has a lot of unrelated changes. I looked through and=
 found the changes you made to extensionstate_update(). Did you make any ot=
her changes in the latest version of the diff?</pre>
 <br />







<p>- Mark</p>


<br />
<p>On July 30th, 2012, 4:43 a.m., Guenther Kelleter wrote:</p>






<table bgcolor=3D"#fefadf" width=3D"100%" cellspacing=3D"0" cellpadding=3D"=
8" style=3D"background-image: url('https://reviewboard.asterisk.org/media/r=
b/images/review_request_box_top_bg.png'); background-position: left top; ba=
ckground-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Asterisk Developers, Mark Michelson, Matt Jordan, a=
nd Thomas Arimont.</div>
<div>By Guenther Kelleter.</div>


<p style=3D"color: grey;"><i>Updated July 30, 2012, 4:43 a.m.</i></p>




<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Descripti=
on </h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"margin: 0; padding: 0; white-space: pre-wrap; white-space:=
 -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap=
: break-word;">(For Digium: this is the implementation of the feature descr=
ibed in chapter 2 of the document SIP_NOTIFY_dialog_event_infos_DATUS_v1_4.=
odt)

This patch extends the extension state callbacks so that monitoring channel=
s (as chan_sip) get more information of the devices which are responsible f=
or an extension state change. The additional information is needed by chan_=
sip to present names/numbers of the caller and callee in an early-state SIP=
 notification. Users of extenstion state callback not interested in the add=
itional information are not affected by the changes.

Motivation: to present the involved party&#39;s name/number in an early-sta=
te nofification (used by the notified device as a pickup offer) one after a=
nother so that a user can see which call he will pick up in an undirected p=
ickup.
Such a pickup offer to a user shall indicate the same call (number/name-A c=
alls number/name-B) as the call which would be picked up when an undirected=
 pickup is executed.


Users interested in additional state info must use the new functions ast_ex=
tension_state_add_extended() resp. ast_extension_state_add_destroy_extended=
() to register an extended state callback. When the callback is registered =
this way, an extra member device_state_info of struct ast_state_cb_info is =
passed to the callback in addition to the aggregated extension state. This =
container holds an object for every device of the monitored extension hint =
consisting of the device name, the device state and a channel reference to =
the channel which (presumably) caused the device state.

The information is used by chan_sip for early-state notifications. When the=
 state of a device changes and the new state contains AST_EVENT_RINGING, an=
 early-state notification is sent to the subscribed devices with the caller=
/callee names/numbers of the oldest ringing channel of the monitored extens=
ion.
The notified user may then invoke a direct pickup, which will pickup exactl=
y this channel.

Users of the old non-extended callbacks will only be called when the aggreg=
ated state did change (same behavior as before). Users of the extended call=
back will also be called when the state is unchanged but does contain AST_E=
VENT_RINGING. That could be the case if two channels are ringing at one dev=
ice and one of them hangs up, so the aggregated state does not change. This=
 way the monitoring channel can create a new early-state notification with =
the now ringing party-ids.

This patch requires the channel creation time member of ast_channel and the=
 changes for the direct pickup to pick up the oldest ringing channel, which=
 are both introduced in review https://reviewboard.asterisk.org/r/2043/.
</pre>
  </td>
 </tr>
</table>


<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing <=
/h1>
<table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D"=
10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"margin: 0; padding: 0; white-space: pre-wrap; white-space:=
 -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap=
: break-word;">Checked the name/number of caller/callee displayed on the no=
tified phone.
Checked that a second call at an already ringing device does not invoke a n=
otification as long as the first call is ringing, instead it is sent when t=
he first ringing channel is picked up by some other party or hung up.
Checked that the picked up caller is the same as advertized in the early-st=
ate notification when more than one calls are ringing on the subscribed ext=
ension.
Ref counting for the containers and channels checked.
Checked that a notification is sent to newly subscribed watchers.</pre>
  </td>
 </tr>
</table>




<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b>=
 </h1>
<ul style=3D"margin-left: 3em; padding-left: 0;">

 <li>/trunk/channels/chan_sip.c <span style=3D"color: grey">(370418)</span>=
</li>

 <li>/trunk/channels/sip/include/sip.h <span style=3D"color: grey">(370418)=
</span></li>

 <li>/trunk/include/asterisk/pbx.h <span style=3D"color: grey">(370418)</sp=
an></li>

 <li>/trunk/main/pbx.c <span style=3D"color: grey">(370418)</span></li>

</ul>

<p><a href=3D"https://reviewboard.asterisk.org/r/2048/diff/" style=3D"margi=
n-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>


--===============0455946111505323350==--