----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3154/#review10686 -----------------------------------------------------------
/branches/12/apps/app_dial.c <https://reviewboard.asterisk.org/r/3154/#comment20177> I agree with the removal of ast_channel_publish_dial() here. As you pointed out, the status can actually be altered due to events that occur before the parties are bridged. However, I disagree with the removal of publish_dial_end_event() from here. No matter what ends up happening to the peer, we can accurately publish that the other channels that did not answer have a status of "CANCEL". /branches/12/main/cdr.c <https://reviewboard.asterisk.org/r/3154/#comment20178> Couple of nitpicks: 1) Why do you only examine the first four characters of snapshot->appl? If someone has installed a custom app called "dial_bathsoap" then that app would get locked in. I think you should require a whole string match. 2) Since the snapshot application comes from a constant from within app_dial, you could drop the case insensitivity and compare case-sensitively to "Dial". - Mark Michelson On Jan. 24, 2014, 6:08 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3154/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2014, 6:08 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23164 > https://issues.asterisk.org/jira/browse/ASTERISK-23164 > > > Repository: Asterisk > > > Description > ------- > > This patch fixes a number of small-ish problems that were noticed when > witnessing the records that the FreePBX dialplan produces. > (1) Mid-call events (as well as privacy options) have the ability to change > the overall state of the Dial operation after the called party answers. This > means that publishing the DialEnd event when the called party is premature; > we have to wait for the execution of these subroutines to complete before we > can signal the overall status of the DialEnd. This patch moves that > publication and adds handlers for the mid-call events. > (2) This patch fixes a bug with the F option, where specifying only a > priority would be skipped as no '^' character is needed or present. This bug > fix will be merged in 1.8+. > (3) The AST_FLAG_OUTGOING channel flag is cleared if an after bridge goto > datastore is detected. This flag was preventing CDRs from being recorded for > all outbound channels that had a 'continue' option enabled on them by the > Dial application. > (4) The CDR engine now locks the 'Dial' application as being the CDR > application if it detects that the current CDR has entered that app. This is > similar to the logic that is done for Parking. In general, if we entered into > Dial, then we want that CDR to record the application as such - this prevent > pre-dial handlers, mid-call handlers, and other shenaniganry from changing > the application value. > (5) The CDR engine now checks for both the AST_FLAG_DEAD and the > AST_SOFTHANGUP_HANGUP_EXEC to determine if the channel is in hangup logic or > dead. In either case, we don't want to record changes in the channel. > (6) Finally, because we now have the ability to synchronize on the messages > published to the CDR topic, on shutdown the CDR engine will now synchronize > to the messages currently in flight. This helps to ensure that all in-flight > CDRs are written before shutting down. > > > Diffs > ----- > > /branches/12/main/pbx.c 406293 > /branches/12/main/manager_channels.c 406293 > /branches/12/main/cdr.c 406293 > /branches/12/main/bridge_after.c 406293 > /branches/12/apps/app_dial.c 406293 > > Diff: https://reviewboard.asterisk.org/r/3154/diff/ > > > Testing > ------- > > See review https://reviewboard.asterisk.org/r/3153/ > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
