chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.

The distinctive ring feature interferes with detecting Caller ID and
appears to have been broken for years.  What happens is if you have a
ring-ring cadence as used in the UK you get too many DAHDI events for the
distinctive ring pattern array and Caller ID detection is aborted.  I
think when Zapata/DAHDI added the ring begin event it broke distinctive
ring.  More events happen than before and the code does no filtering of
which event times are recorded in the pattern array.

* Made distinctive ring only record the ringt count when the ring ends
instead of on just any DAHDI event.  Distinctive ring can be ring,
ring-ring, ring-ring-ring, or different ring durations for the up to three
rings.

* Fixed the distinctive ring detection enable (chan_dahdi.conf option
usedistinctiveringdetection) to be per port instead of somewhat per port
and somewhat global.  This has been broken since v1.8.

* Fixed using the default distinctive ring context when the detected
pattern does not match any configured dringX patterns.  The default
context did not get set when the previous call was a matched distinctive
ring pattern and the current call is not matched.  This has been broken
since v1.8.

* Made distinctive ring have no effect on Caller ID detection when it is
disabled.  Caller ID detection just monitors for 10 seconds before giving
up.

* Fixed leak of struct callerid_state memory when a polarity reversal
during Caller ID detection causes the incoming call to be aborted.

DAHDI-1143
AST-1545
ASTERISK-24825 #close
Reported by: Richard Mudgett

ASTERISK-17588
Reported by: Daniel Flounders

Review: https://reviewboard.asterisk.org/r/4444/
........

Merged revisions 432530 from http://svn.asterisk.org/svn/asterisk/branches/11


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/13@432534 65c4cc65-6c06-0410-ace0-fbb531ad65f3

5 files changed, 157 insertions, 130 deletions

diff --git a/UPGRADE.txt b/UPGRADE.txt
index 8f005e939..e18d646ec 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -21,6 +21,15 @@
 === UPGRADE-12.txt  -- Upgrade info for 11 to 12
 ===========================================================
 
+From 13.2.0 to 13.3.0:
+
+chan_dahdi:
+ - For users using the FXO port (FXS signaling) distinctive ring detection
+   feature, you will need to adjust the dringX count values.  The count
+   values now only record ring end events instead of any DAHDI event.  A
+   ring-ring-ring pattern would exceed the pattern limits and stop
+   Caller-ID detection.
+
 From 13.1.0 to 13.2.0:
 
 ARI:
diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c
index 72c8497f0..53899d869 100644
--- a/channels/chan_dahdi.c
+++ b/channels/chan_dahdi.c
@@ -1301,13 +1301,21 @@ static int my_start_cid_detect(void *pvt, int cid_signalling)
 	return 0;
 }
 
+static int restore_gains(struct dahdi_pvt *p);
+
 static int my_stop_cid_detect(void *pvt)
 {
 	struct dahdi_pvt *p = pvt;
 	int index = SUB_REAL;
-	if (p->cs)
+
+	if (p->cs) {
 		callerid_free(p->cs);
+	}
+
+	/* Restore linear mode after Caller*ID processing */
 	dahdi_setlinear(p->subs[index].dfd, p->subs[index].linear);
+	restore_gains(p);
+
 	return 0;
 }
 
@@ -1383,7 +1391,6 @@ static int my_get_callerid(void *pvt, char *namebuf, char *numbuf, enum analog_e
 }
 
 static const char *event2str(int event);
-static int restore_gains(struct dahdi_pvt *p);
 
 static int my_distinctive_ring(struct ast_channel *chan, void *pvt, int idx, int *ringdata)
 {
@@ -1396,7 +1403,7 @@ static int my_distinctive_ring(struct ast_channel *chan, void *pvt, int idx, int
 	int i;
 	int res;
 	int checkaftercid = 0;
-
+	const char *matched_context;
 	struct dahdi_pvt *p = pvt;
 	struct analog_pvt *analog_p = p->sig_pvt;
 
@@ -1406,19 +1413,15 @@ static int my_distinctive_ring(struct ast_channel *chan, void *pvt, int idx, int
 		checkaftercid = 1;
 	}
 
-	/* We must have a ring by now, so, if configured, lets try to listen for
-	 * distinctive ringing */
+	/* We must have a ring by now so lets try to listen for distinctive ringing */
 	if ((checkaftercid && distinctiveringaftercid) || !checkaftercid) {
 		/* Clear the current ring data array so we don't have old data in it. */
 		for (receivedRingT = 0; receivedRingT < RING_PATTERNS; receivedRingT++)
 			ringdata[receivedRingT] = 0;
 		receivedRingT = 0;
-		if (checkaftercid && distinctiveringaftercid)
+
+		if (checkaftercid && distinctiveringaftercid) {
 			ast_verb(3, "Detecting post-CID distinctive ring\n");
-		/* Check to see if context is what it should be, if not set to be. */
-		else if (strcmp(p->context,p->defcontext) != 0) {
-			ast_copy_string(p->context, p->defcontext, sizeof(p->context));
-			ast_channel_context_set(chan, p->defcontext);
 		}
 
 		for (;;) {
@@ -1431,22 +1434,23 @@ static int my_distinctive_ring(struct ast_channel *chan, void *pvt, int idx, int
 			}
 			if (i & DAHDI_IOMUX_SIGEVENT) {
 				res = dahdi_get_event(p->subs[idx].dfd);
+				ast_debug(3, "Got event %d (%s)...\n", res, event2str(res));
 				if (res == DAHDI_EVENT_NOALARM) {
 					p->inalarm = 0;
 					analog_p->inalarm = 0;
-				}
-				ast_log(LOG_NOTICE, "Got event %d (%s)...\n", res, event2str(res));
-				res = 0;
-				/* Let us detect distinctive ring */
-
-				ringdata[receivedRingT] = analog_p->ringt;
+				} else if (res == DAHDI_EVENT_RINGOFFHOOK) {
+					/* Let us detect distinctive ring */
+					ringdata[receivedRingT] = analog_p->ringt;
 
-				if (analog_p->ringt < analog_p->ringt_base/2)
-					break;
-				/* Increment the ringT counter so we can match it against
-				   values in chan_dahdi.conf for distinctive ring */
-				if (++receivedRingT == RING_PATTERNS)
-					break;
+					if (analog_p->ringt < analog_p->ringt_base / 2) {
+						break;
+					}
+					/* Increment the ringT counter so we can match it against
+					   values in chan_dahdi.conf for distinctive ring */
+					if (++receivedRingT == RING_PATTERNS) {
+						break;
+					}
+				}
 			} else if (i & DAHDI_IOMUX_READ) {
 				res = read(p->subs[idx].dfd, buf, sizeof(buf));
 				if (res < 0) {
@@ -1465,44 +1469,49 @@ static int my_distinctive_ring(struct ast_channel *chan, void *pvt, int idx, int
 			}
 		}
 	}
-	if ((checkaftercid && usedistinctiveringdetection) || !checkaftercid) {
-		/* this only shows up if you have n of the dring patterns filled in */
-		ast_verb(3, "Detected ring pattern: %d,%d,%d\n",ringdata[0],ringdata[1],ringdata[2]);
-		for (counter = 0; counter < 3; counter++) {
-		/* Check to see if the rings we received match any of the ones in chan_dahdi.conf for this channel */
-			distMatches = 0;
-			/* this only shows up if you have n of the dring patterns filled in */
-			ast_verb(3, "Checking %d,%d,%d\n",
-					p->drings.ringnum[counter].ring[0],
-					p->drings.ringnum[counter].ring[1],
-					p->drings.ringnum[counter].ring[2]);
-			for (counter1 = 0; counter1 < 3; counter1++) {
-				ast_verb(3, "Ring pattern check range: %d\n", p->drings.ringnum[counter].range);
-				if (p->drings.ringnum[counter].ring[counter1] == -1) {
-					ast_verb(3, "Pattern ignore (-1) detected, so matching pattern %d regardless.\n",
-					ringdata[counter1]);
-					distMatches++;
-				} else if (ringdata[counter1] <= (p->drings.ringnum[counter].ring[counter1] + p->drings.ringnum[counter].range) &&
-										ringdata[counter1] >= (p->drings.ringnum[counter].ring[counter1] - p->drings.ringnum[counter].range)) {
-					ast_verb(3, "Ring pattern matched in range: %d to %d\n",
-					(p->drings.ringnum[counter].ring[counter1] - p->drings.ringnum[counter].range),
-					(p->drings.ringnum[counter].ring[counter1] + p->drings.ringnum[counter].range));
-					distMatches++;
-				}
-			}
 
-			if (distMatches == 3) {
-				/* The ring matches, set the context to whatever is for distinctive ring.. */
-				ast_copy_string(p->context, S_OR(p->drings.ringContext[counter].contextData, p->defcontext), sizeof(p->context));
-				ast_channel_context_set(chan, S_OR(p->drings.ringContext[counter].contextData, p->defcontext));
-				ast_verb(3, "Distinctive Ring matched context %s\n",p->context);
+	/* Check to see if the rings we received match any of the ones in chan_dahdi.conf for this channel */
+	ast_verb(3, "Detected ring pattern: %d,%d,%d\n", ringdata[0], ringdata[1], ringdata[2]);
+	matched_context = p->defcontext;
+	for (counter = 0; counter < 3; counter++) {
+		int range = p->drings.ringnum[counter].range;
+
+		distMatches = 0;
+		ast_verb(3, "Checking %d,%d,%d with +/- %d range\n",
+			p->drings.ringnum[counter].ring[0],
+			p->drings.ringnum[counter].ring[1],
+			p->drings.ringnum[counter].ring[2],
+			range);
+		for (counter1 = 0; counter1 < 3; counter1++) {
+			int ring = p->drings.ringnum[counter].ring[counter1];
+
+			if (ring == -1) {
+				ast_verb(3, "Pattern ignore (-1) detected, so matching pattern %d regardless.\n",
+					ringdata[counter1]);
+				distMatches++;
+			} else if (ring - range <= ringdata[counter1] && ringdata[counter1] <= ring + range) {
+				ast_verb(3, "Ring pattern %d is in range: %d to %d\n",
+					ringdata[counter1], ring - range, ring + range);
+				distMatches++;
+			} else {
+				/* The current dring pattern cannot match. */
 				break;
 			}
 		}
+
+		if (distMatches == 3) {
+			/* The ring matches, set the context to whatever is for distinctive ring.. */
+			matched_context = S_OR(p->drings.ringContext[counter].contextData, p->defcontext);
+			ast_verb(3, "Matched Distinctive Ring context %s\n", matched_context);
+			break;
+		}
+	}
+
+	/* Set selected distinctive ring context if not already set. */
+	if (strcmp(p->context, matched_context) != 0) {
+		ast_copy_string(p->context, matched_context, sizeof(p->context));
+		ast_channel_context_set(chan, matched_context);
 	}
-	/* Restore linear mode (if appropriate) for Caller*ID processing */
-	dahdi_setlinear(p->subs[idx].dfd, p->subs[idx].linear);
-	restore_gains(p);
 
 	return 0;
 }
@@ -12788,6 +12797,7 @@ static struct dahdi_pvt *mkintf(int channel, const struct dahdi_chan_conf *conf,
 				analog_p->transfer = conf->chan.transfer;
 				analog_p->transfertobusy = conf->chan.transfertobusy;
 				analog_p->use_callerid = tmp->use_callerid;
+				analog_p->usedistinctiveringdetection = tmp->usedistinctiveringdetection;
 				analog_p->use_smdi = tmp->use_smdi;
 				analog_p->smdi_iface = tmp->smdi_iface;
 				analog_p->outsigmod = ANALOG_SIG_NONE;
diff --git a/channels/chan_dahdi.h b/channels/chan_dahdi.h
index e4a689a1a..4bb5d19a2 100644
--- a/channels/chan_dahdi.h
+++ b/channels/chan_dahdi.h
@@ -447,7 +447,7 @@ struct dahdi_pvt {
 	 */
 	char description[32];
 	/*!
-	 * \brief Saved context string.
+	 * \brief Default distinctive ring context.
 	 */
 	char defcontext[AST_MAX_CONTEXT];
 	/*! \brief Extension to use in the dialplan. */
diff --git a/channels/sig_analog.c b/channels/sig_analog.c
index 0c7701772..1d44a29b9 100644
--- a/channels/sig_analog.c
+++ b/channels/sig_analog.c
@@ -1636,6 +1636,9 @@ static void analog_decrease_ss_count(void)
 
 static int analog_distinctive_ring(struct ast_channel *chan, struct analog_pvt *p, int idx, int *ringdata)
 {
+	if (!p->usedistinctiveringdetection) {
+		return 0;
+	}
 	if (analog_callbacks.distinctive_ring) {
 		return analog_callbacks.distinctive_ring(chan, p->chan_pvt, idx, ringdata);
 	}
@@ -2453,50 +2456,53 @@ static void *__analog_ss_thread(void *data)
 
 			/* If set to use V23 Signalling, launch our FSK gubbins and listen for it */
 			} else if ((p->cid_signalling == CID_SIG_V23) || (p->cid_signalling == CID_SIG_V23_JP)) {
-				int timeout = 10000;  /* Ten seconds */
-				struct timeval start = ast_tvnow();
-				enum analog_event ev;
-
 				namebuf[0] = 0;
 				numbuf[0] = 0;
 
 				if (!analog_start_cid_detect(p, p->cid_signalling)) {
+					int timeout = 10000;  /* Ten seconds */
+					struct timeval start = ast_tvnow();
+					enum analog_event ev;
 					int off_ms;
 					int ms;
 					struct timeval off_start;
-					while (1) {
-						res = analog_get_callerid(p, namebuf, numbuf, &ev, timeout - ast_tvdiff_ms(ast_tvnow(), start));
 
+					if (!p->usedistinctiveringdetection) {
+						/* Disable distinctive ring timeout count */
+						analog_set_ringtimeout(p, 0);
+					}
+					while ((ms = ast_remaining_ms(start, timeout))) {
+						res = analog_get_callerid(p, namebuf, numbuf, &ev, ms);
+						if (res < 0) {
+							ast_log(LOG_WARNING,
+								"CallerID returned with error on channel '%s'\n",
+								ast_channel_name(chan));
+							break;
+						}
 						if (res == 0) {
 							break;
 						}
-
-						if (res == 1) {
-							if (ev == ANALOG_EVENT_NOALARM) {
-								analog_set_alarm(p, 0);
-							}
-							if (p->cid_signalling == CID_SIG_V23_JP) {
-								if (ev == ANALOG_EVENT_RINGBEGIN) {
-									analog_off_hook(p);
-									usleep(1);
-								}
-							} else {
-								ev = ANALOG_EVENT_NONE;
-								break;
-							}
+						if (res != 1) {
+							continue;
 						}
-
-						if (ast_tvdiff_ms(ast_tvnow(), start) > timeout)
+						if (ev == ANALOG_EVENT_NOALARM) {
+							analog_set_alarm(p, 0);
+						}
+						if (p->cid_signalling == CID_SIG_V23_JP) {
+							if (ev == ANALOG_EVENT_RINGBEGIN) {
+								analog_off_hook(p);
+								usleep(1);
+							}
+						} else {
 							break;
-
+						}
 					}
+
 					name = namebuf;
 					number = numbuf;
 
-					analog_stop_cid_detect(p);
-
 					if (p->cid_signalling == CID_SIG_V23_JP) {
-						res = analog_on_hook(p);
+						analog_on_hook(p);
 						usleep(1);
 					}
 
@@ -2510,11 +2516,13 @@ static void *__analog_ss_thread(void *data)
 						if (res <= 0) {
 							ast_log(LOG_WARNING,
 								"CID timed out waiting for ring. Exiting simple switch\n");
+							analog_stop_cid_detect(p);
 							ast_hangup(chan);
 							goto quit;
 						}
 						if (!(f = ast_read(chan))) {
 							ast_log(LOG_WARNING, "Hangup received waiting for ring. Exiting simple switch\n");
+							analog_stop_cid_detect(p);
 							ast_hangup(chan);
 							goto quit;
 						}
@@ -2524,13 +2532,11 @@ static void *__analog_ss_thread(void *data)
 							break; /* Got ring */
 					}
 
-					if (analog_distinctive_ring(chan, p, idx, NULL)) {
+					res = analog_distinctive_ring(chan, p, idx, NULL);
+					analog_stop_cid_detect(p);
+					if (res) {
 						goto quit;
 					}
-
-					if (res < 0) {
-						ast_log(LOG_WARNING, "CallerID returned with error on channel '%s'\n", ast_channel_name(chan));
-					}
 				} else {
 					ast_log(LOG_WARNING, "Unable to get caller ID space\n");
 				}
@@ -2542,66 +2548,67 @@ static void *__analog_ss_thread(void *data)
 				goto quit;
 			}
 		} else if (p->use_callerid && p->cid_start == ANALOG_CID_START_RING) {
-			int timeout = 10000;  /* Ten seconds */
-			struct timeval start = ast_tvnow();
-			enum analog_event ev;
-			int curRingData[RING_PATTERNS] = { 0 };
-			int receivedRingT = 0;
-
 			namebuf[0] = 0;
 			numbuf[0] = 0;
 
 			if (!analog_start_cid_detect(p, p->cid_signalling)) {
-				while (1) {
-					res = analog_get_callerid(p, namebuf, numbuf, &ev, timeout - ast_tvdiff_ms(ast_tvnow(), start));
+				int timeout = 10000;  /* Ten seconds */
+				struct timeval start = ast_tvnow();
+				enum analog_event ev;
+				int ring_data[RING_PATTERNS] = { 0 };
+				int ring_data_idx = 0;
+				int ms;
 
+				if (!p->usedistinctiveringdetection) {
+					/* Disable distinctive ring timeout count */
+					analog_set_ringtimeout(p, 0);
+				}
+				while ((ms = ast_remaining_ms(start, timeout))) {
+					res = analog_get_callerid(p, namebuf, numbuf, &ev, ms);
+					if (res < 0) {
+						ast_log(LOG_WARNING,
+							"CallerID returned with error on channel '%s'\n",
+							ast_channel_name(chan));
+						break;
+					}
 					if (res == 0) {
 						break;
 					}
-
-					if (res == 1 || res == 2) {
-						if (ev == ANALOG_EVENT_NOALARM) {
-							analog_set_alarm(p, 0);
-						} else if (ev == ANALOG_EVENT_POLARITY && p->hanguponpolarityswitch && p->polarity == POLARITY_REV) {
-							ast_debug(1, "Hanging up due to polarity reversal on channel %d while detecting callerid\n", p->channel);
-							p->polarity = POLARITY_IDLE;
-							ast_hangup(chan);
-							goto quit;
-						} else if (ev != ANALOG_EVENT_NONE && ev != ANALOG_EVENT_RINGBEGIN && ev != ANALOG_EVENT_RINGOFFHOOK) {
-							break;
-						}
-						if (res != 2) {
-							/* Let us detect callerid when the telco uses distinctive ring */
-							curRingData[receivedRingT] = p->ringt;
-
-							if (p->ringt < p->ringt_base/2) {
-								break;
-							}
-							/* Increment the ringT counter so we can match it against
-							   values in chan_dahdi.conf for distinctive ring */
-							if (++receivedRingT == RING_PATTERNS) {
-								break;
-							}
-						}
+					if (res != 1) {
+						continue;
 					}
-
-					if (ast_tvdiff_ms(ast_tvnow(), start) > timeout) {
-						break;
+					if (ev == ANALOG_EVENT_NOALARM) {
+						analog_set_alarm(p, 0);
+					} else if (ev == ANALOG_EVENT_POLARITY
+						&& p->hanguponpolarityswitch
+						&& p->polarity == POLARITY_REV) {
+						ast_debug(1,
+							"Hanging up due to polarity reversal on channel %d while detecting callerid\n",
+							p->channel);
+						p->polarity = POLARITY_IDLE;
+						analog_stop_cid_detect(p);
+						ast_hangup(chan);
+						goto quit;
+					} else if (ev == ANALOG_EVENT_RINGOFFHOOK
+						&& p->usedistinctiveringdetection
+						&& ring_data_idx < RING_PATTERNS) {
+						/*
+						 * Detect callerid while collecting possible
+						 * distinctive ring pattern.
+						 */
+						ring_data[ring_data_idx] = p->ringt;
+						++ring_data_idx;
 					}
-
 				}
+
 				name = namebuf;
 				number = numbuf;
 
+				res = analog_distinctive_ring(chan, p, idx, ring_data);
 				analog_stop_cid_detect(p);
-
-				if (analog_distinctive_ring(chan, p, idx, curRingData)) {
+				if (res) {
 					goto quit;
 				}
-
-				if (res < 0) {
-					ast_log(LOG_WARNING, "CallerID returned with error on channel '%s'\n", ast_channel_name(chan));
-				}
 			} else {
 				ast_log(LOG_WARNING, "Unable to get caller ID space\n");
 			}
diff --git a/channels/sig_analog.h b/channels/sig_analog.h
index 13c92c657..6415b6eb8 100644
--- a/channels/sig_analog.h
+++ b/channels/sig_analog.h
@@ -281,6 +281,7 @@ struct analog_pvt {
 	unsigned int transfer:1;
 	unsigned int transfertobusy:1;			/*!< allow flash-transfers to busy channels */
 	unsigned int use_callerid:1;			/*!< Whether or not to use caller id on this channel */
+	unsigned int usedistinctiveringdetection:1;
 	unsigned int callwaitingcallerid:1;		/*!< TRUE if send caller ID for Call Waiting */
 	/*!
 	 * \brief TRUE if SMDI (Simplified Message Desk Interface) is enabled