[svn-commits] mmichelson: branch group/CCSS r227050 - in /team/group/CCSS: apps/ include/as...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Mon Nov 2 17:01:11 CST 2009


Author: mmichelson
Date: Mon Nov  2 17:01:08 2009
New Revision: 227050

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=227050
Log:
Change CC complete timing.

One situation that rmudgett and I discussed is one in which
CC is declared to be complete before a downstream endpoint has received
the SETUP/INVITE/whatever for the recall. It is possible, though
not likely, that the CC structures could be destroyed prior to
the recall arriving on a downstream server, thus resulting in
the callee not being properly dialed. If that didn't make sense,
sorry.

Anyway, the remedy for this is to not declare CC to be completed
when the first frame arrives, but rather to wait until the end of
the wait_for_answer function in Dial, so we have a final verdict on
the call.

At this point, there are still some compilation errors in ccss.c, all
of which are due to the fact that the cc_recall datastore definition
is further down in the file than it should be.

The next steps are to get those errors fixed, do another run-through
to get all doxygen up-to-date and accurate, and then re-run all my
tests to be sure that I have not introduced any new bugs while fixing
the problems being addressed the last few days.


Modified:
    team/group/CCSS/apps/app_dial.c
    team/group/CCSS/include/asterisk/ccss.h
    team/group/CCSS/main/ccss.c

Modified: team/group/CCSS/apps/app_dial.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/apps/app_dial.c?view=diff&rev=227050&r1=227049&r2=227050
==============================================================================
--- team/group/CCSS/apps/app_dial.c (original)
+++ team/group/CCSS/apps/app_dial.c Mon Nov  2 17:01:08 2009
@@ -887,7 +887,7 @@
 	struct chanlist *outgoing, int *to, struct ast_flags64 *peerflags,
 	struct privacy_args *pa,
 	const struct cause_args *num_in, int *result, char *dtmf_progress,
-	const int ignore_cc, int cc_recall_core_id)
+	const int ignore_cc)
 {
 	struct cause_args num = *num_in;
 	int prestart = num.busy + num.congestion + num.nochan;
@@ -900,6 +900,8 @@
 #endif
 	struct ast_party_connected_line connected_caller;
 	struct ast_str *featurecode = ast_str_alloca(FEATURE_MAX_LEN + 1);
+	int cc_recall_core_id;
+	int is_cc_recall;
 
 	ast_party_connected_line_init(&connected_caller);
 	if (single) {
@@ -919,6 +921,8 @@
 			ast_party_connected_line_free(&connected_caller);
 		}
 	}
+
+	is_cc_recall = ast_cc_is_recall(in, &cc_recall_core_id);
 
 #ifdef HAVE_EPOLL
 	for (epollo = outgoing; epollo; epollo = epollo->next)
@@ -952,6 +956,9 @@
 				ast_verb(3, "No one is available to answer at this time (%d:%d/%d/%d)\n", numlines, num.busy, num.congestion, num.nochan);
 			}
 			*to = 0;
+			if (is_cc_recall) {
+				ast_cc_failed(cc_recall_core_id, "No channels available to try recalling. How sad");
+			}
 			return NULL;
 		}
 		winner = ast_waitfor_n(watchers, pos, to);
@@ -1019,22 +1026,6 @@
 				ast_clear_flag64(o, DIAL_STILLGOING);
 				handle_cause(in->hangupcause, &num);
 				continue;
-			}
-			/* We successfully read a frame from one of the outbound channels.
-			 * If this is a cc recall, then that means we have succeeded in the
-			 * recall process. It really doesn't matter what type of frame we received,
-			 * just that we received something.
-			 */
-			if (cc_recall_core_id != -1) {
-				ast_cc_completed(cc_recall_core_id, "Call completed successfully!");
-				/* Setting this to -1 will make sure that no further frames cause superfluous
-				 * state changes.
-				 */
-				/* XXX While this certainly will quiet a single dial to multiple channels, a dial to a local
-				 * channel will still cause superfluous state changes to be queued potentially. It's not a
-				 * showstopper, but it does mean some wasted cycles.
-				 */
-				cc_recall_core_id = -1;
 			}
 			if (f->frametype == AST_FRAME_CONTROL) {
 				switch(f->subclass) {
@@ -1248,6 +1239,9 @@
 					}
 					ast_frfree(f);
 				}
+				if (is_cc_recall) {
+					ast_cc_completed(in, "CC completed, although the caller hung up (cancelled)");
+				}
 				return NULL;
 			}
 
@@ -1265,6 +1259,9 @@
 						strcpy(pa->status, "CANCEL");
 						ast_frfree(f);
 						ast_channel_unlock(in);
+						if (is_cc_recall) {
+							ast_cc_completed(in, "CC completed, but the caller used DTMF to exit");
+						}
 						return NULL;
 					}
 					ast_channel_unlock(in);
@@ -1277,6 +1274,9 @@
 					strcpy(pa->status, "CANCEL");
 					ast_cdr_noanswer(in->cdr);
 					ast_frfree(f);
+					if (is_cc_recall) {
+						ast_cc_completed(in, "CC completed, but the caller hung up with DTMF");
+					}
 					return NULL;
 				}
 			}
@@ -1319,6 +1319,9 @@
 	}
 #endif
 
+	if (is_cc_recall) {
+		ast_cc_completed(in, "Recall completed!");
+	}
 	return peer;
 }
 
@@ -2025,7 +2028,7 @@
 		}
 	}
 
-	peer = wait_for_answer(chan, outgoing, &to, peerflags, &pa, &num, &result, dtmf_progress, ignore_cc, ast_cc_get_recall_core_id(chan));
+	peer = wait_for_answer(chan, outgoing, &to, peerflags, &pa, &num, &result, dtmf_progress, ignore_cc);
 
 	/* The ast_channel_datastore_remove() function could fail here if the
 	 * datastore was moved to another channel during a masquerade. If this is

Modified: team/group/CCSS/include/asterisk/ccss.h
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/include/asterisk/ccss.h?view=diff&rev=227050&r1=227049&r2=227050
==============================================================================
--- team/group/CCSS/include/asterisk/ccss.h (original)
+++ team/group/CCSS/include/asterisk/ccss.h Mon Nov  2 17:01:08 2009
@@ -954,12 +954,12 @@
  * When we receive confirmation that an endpoint has responded to our
  * CC recall, we call this function.
  *
- * \param core_id core_id of the CC transaction
+ * \param chan The inbound channel making the CC recall
  * \param debug optional string to print for debugging purposes
  * \retval 0 Success
  * \retval -1 Failure
  */
-int ast_cc_completed(int core_id, const char * const debug);
+int ast_cc_completed(struct ast_channel *chan, const char * const debug);
 
 /*!
  * \since 1.6.4

Modified: team/group/CCSS/main/ccss.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/main/ccss.c?view=diff&rev=227050&r1=227049&r2=227050
==============================================================================
--- team/group/CCSS/main/ccss.c (original)
+++ team/group/CCSS/main/ccss.c Mon Nov  2 17:01:08 2009
@@ -1394,7 +1394,7 @@
 		cc_interfaces->done = 1;
 	}
 
-	if ((cc_recall_datastore = ast_channel_datastore_find(chan, &cc_recall_ds_info, NULL))) {
+	if ((cc_recall_datastore = ast_channel_datastore_find(chan, &recall_ds_info, NULL))) {
 		recall_cc_data = cc_recall_datastore->data;
 		recall_cc_data->done = 1;
 	}
@@ -2654,8 +2654,36 @@
 	return cc_request_state_change(CC_RECALLING, core_id, debug);
 }
 
-int ast_cc_completed(int core_id, const char * const debug)
-{
+int ast_cc_completed(struct ast_channel *chan, const char * const debug)
+{
+	struct ast_datastore *recall_datastore;
+	struct cc_recall_ds_data *recall_data;
+	int core_id;
+
+	ast_channel_lock(chan);
+	if (!(recall_datastore = ast_channel_datastore_find(chan, &recall_ds_info, NULL))) {
+		/* Silly! Why did you call this function if there's no recall DS? */
+		ast_channel_unlock(chan);
+		return -1;
+	}
+	recall_data = recall_datastore->data;
+	if (recall_data->nested || recall_data->done) {
+		/* If this is being called from a nested Dial, it is too
+		 * early to determine if the recall has actually completed.
+		 * The outermost dial is the only one with the authority to
+		 * declare the recall to be complete.
+		 *
+		 * Similarly, if this function has been called when the 
+		 * recall has progressed beyond the first dial, this is not
+		 * a legitimate time to declare the recall to be done. In fact,
+		 * that should have been done already.
+		 */
+		ast_channel_unlock(chan);
+		return -1;
+	}
+
+	core_id = recall_data->core_id;
+	ast_channel_unlock(chan);
 	return cc_request_state_change(CC_COMPLETE, core_id, debug);
 }
 
@@ -2797,6 +2825,7 @@
 struct cc_recall_ds_data {
 	int core_id;
 	char done;
+	char nested;
 	struct ast_cc_interface_tree *interface_tree;
 };
 
@@ -2810,6 +2839,7 @@
 	}
 	new_data->interface_tree = cc_ref(old_data->interface_tree, "Bump refcount of interface tree for recall");
 	new_data->core_id = old_data->core_id;
+	new_data->nested = 1;
 	return new_data;
 }
 




More information about the svn-commits mailing list