[asterisk-dev] [Code Review] 2877: CDRs: Improve handling of parked call message; resolve assertion failure from originating into a parking lot

[rmudgett]

> On Sept. 27, 2013, 8:49 p.m., rmudgett wrote:
> > ./branches/12/main/cdr.c, line 2543
> > <https://reviewboard.asterisk.org/r/2877/diff/1/?file=46217#file46217line2543>
> >
> >     You should test for NULL function in the table.
> 
> Matt Jordan wrote:
>     Nope - as the comment states:
>     
>     "/* This is guaranteed to succeed: the new CDR is created in the
>       * single state and will be able to handle the parkedcall message
>       */
>     
>     All new CDRs are created in the single state. The single state *must* handle the parkedcall message. It is invalid for it not to, and would be a programming error.
>     
>     (Note that most handlers for messages do test for whether or not the CDR's state is NULL because they don't know what state the CDR is in - here, we know.)

I think that comment should be reworded.  The "This is guaranteed to succeed" comment looks more like it is describing the call will always succeed not that the function pointer must exist.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2877/#review9842
-----------------------------------------------------------


On Sept. 24, 2013, 9:37 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2877/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2013, 9:37 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22842
>     https://issues.asterisk.org/jira/browse/ASTERISK-22842
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch covers two problems:
> 
> (1) Currently, when a call is transferred into a parking lot from a bridge (using either the blind transfer or one touch parking mechanisms), the application fails to be set a "Park" in the resulting CDR record for the parked channel. This is due to the ParkedCall message arriving before the BridgeEnter into the parking bridge. The ParkedCall message isn't handled as the CDR for the channel has already been finalized (due to the channel having left its two party bridge), and the BridgeEnter - which creates the new CDR - doesn't have the parking information. This patch modifies the behavior so that reception of a ParkedCall message will - if not handled by a CDR chain - cause a new CDR to be created and put into the Parking state.
> 
> Handy table to show the problem:
> 
> OLD BEHAVIOR:
>    Msg                           State
> BridgeEnter (two party)      (???)->Bridge
> BridgeLeave (two party)      Bridge->Finalized
> ParkedCall                      Finalized (doesn't handle the message)
> BridgeEnter (parking)        Single->Parked   (new CDR)
> BridgeLeave (parking)        Parked->Finalized
> 
> NEW:
>    Msg                         State
> BridgeEnter (two party)      (???)->Bridge
> BridgeLeave (two party)      Bridge->Finalized
> ParkedCall                   Single->Parked   (new CDR)
> BridgeEnter (parking)          Parked
> BridgeLeave (parking)        Parked->Finalized
> 
> 
> (2) It fixes a FRACK that occurred when a channel is originated into a parking space. The DialedPending state - which occurs for both Dialed and Originated channels - assumed that it couldn't handle the parking transitions due to it having a Party B; however, Originated channels don't have a Party B. As such, the existing CDR needs to transition into the parking state - this patch does that.
> 
> States:
>    Msg             State
> DialBegin       Single->Dial
> DialEnd         Dial->DialPending
> ParkedCall      DialPending->Parked
> 
> 
> Diffs
> -----
> 
>   ./branches/12/main/cdr.c 399746 
> 
> Diff: https://reviewboard.asterisk.org/r/2877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130927/f35cfb1e/attachment.html>