[Home]

Summary:ASTERISK-02831: [patch] Support MGCP distinctive ring
Reporter:florian (florian)Labels:
Date Opened:2004-11-16 09:05:48.000-0600Date Closed:2008-01-15 15:20:12.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) patch_mgcp_distinctivering
( 1) patch_mgcp_distring_head_fix.txt
( 2) patch_mgcp_distring_head.txt
Description:This patch provides MGCP distinctive ring support (part of RFC3660) to endpoints based on the ALERT_INFO variable, just like chan_sip does.

****** ADDITIONAL INFORMATION ******

I think I also fixed a potential buffer overflow with the tone variable :-)

Also note, I don't think this method of distinctive ring signalling is the most elegant, but the chan_zap method is not either since that is channel specific. I would much rather move to support the 'r2' type parameter as a valid parameter to app_dial... However, I didn't have time to approach that yet :-)

A disclaimer for items posted by me on this bugtracker was faxed just now, but validity has not yet been confirmed by Digium. Mark knows how to get in touch with me :-)
Comments:By: twisted (twisted) 2004-12-01 00:55:10.000-0600

Somehow, this slipped through the cracks.  For future reference, please prepend [patch] to the summary line of any patches you submit.  Thanks!
By: florian (florian) 2004-12-01 01:46:56.000-0600

Yes, I realised that myself just after I submitted it, but I can't change it afterwards, bit silly :)
By: twisted (twisted) 2004-12-15 20:42:27.000-0600

Has anyone tested this?  florian, can you re-post your patch to fit easier-to-view standards?  (ie, filename.txt)

Also, does this still apply to current cvs-HEAD?

--Housekeeping
By: florian (florian) 2004-12-20 14:52:30.000-0600

I will try to update the patch for cvs head and see where I end up.
By: florian (florian) 2004-12-23 20:11:30.000-0600

Patch was updated slightly to work with CVS-HEAD of today. See the attached file (patch_mgcp_distring_head.txt). Please evaluate and test it.
By: khb (khb) 2004-12-24 00:38:19.000-0600

Please check your syntax/spelling again.
+            dinctive_ring = ast_var_value(current);  
????????????????????????????

Also, I would expand the string length to accommodate complete specification
of the line package ( ringing tone,  repetition and length)
rather than only a single character,
eg.  L/r3(rep=4,to=8000)


perhaps along these lines:



       char tone[80] = "L/rX";
       char* ring = "";

       headp = &ast->varshead;
       AST_LIST_TRAVERSE(headp, cur, entries) {
               if (!strcasecmp(ast_var_name(cur),"ALERT_INFO")) {
                       ring = ast_var_value(cur);
                       if (!ring) ring = "";
       }       }
       ast_mutex_lock(&sub->lock);

       switch (p->hookstate) {
       case MGCP_OFFHOOK:
               strcpy(tone, "L/wt");   /* call waiting tone */
               break;
       case MGCP_ONHOOK:
       default:
               if (strlen(ring) <= 1) {
                       if (*ring >= 0x30 && *ring < 0x38) {
                               tone[3] = *ring;      /* distinctive ringing */
                       } else {
                               tone[3] = 'g';        /* normal ringing */
                       }
               } else {
                       snprintf(tone, sizeof tone, "L/%s", ring);      /* send any Line package command */
               }
               break;
       }

edited on: 12-24-04 00:45
By: florian (florian) 2004-12-24 02:45:30.000-0600

You are right about the variable naming, that was clearly a typo, thanks. I've added a new patch.

As for your other comment: I've evaluated what can be added in RFC3660, and come to the conclusion that both 'rep' and 'to' actually don't add much value to the support we are trying to achieve. The Dial app already supports timing-out, therefore the MGCP endpoint doesn't have to. Additionally, the 'to' is completely ignored if 'rep' is specified. In brief, I just don't see the point in adding that support, especially since it makes the ALERT_INFO variable non-standard where it can be multi-device right now..

My goal is to have one method signalling of distinctive ring to a channel that works with most devices that support it. That way it becomes easier to manage and maintain a mixed environment.
By: khb (khb) 2004-12-26 11:54:40.000-0600

Well, there is nothing very standard about ALERT_INFO anyhow, different devices and manufactures use different schemes (within SIP mostly)

> My goal is to have one method signalling of distinctive ring to a channel that
> works with most devices that support it. That way it becomes easier to manage
> and maintain a mixed environment.

My example works with any MGCP device that supports the L/ package. The user can enter any parameter string and thus this supports any device equally.

Nothing harder to manage either, since my given example doesn't prevent you from just specifying a single digit, it only verifies that it indeed is a digit and is in the proper range according to RFC.
By: florian (florian) 2004-12-27 08:19:18.000-0600

hmkay. can you implement that change and see if it works for you ? I will verify with my phones as well, but it will be some time next week since I'm on tight schedules these days.
By: Mark Spencer (markster) 2005-01-05 21:57:43.000-0600

Added to CVS head, with minor mods.
By: Russell Bryant (russell) 2005-01-05 22:42:25.000-0600

not included in 1.0
By: Digium Subversion (svnbot) 2008-01-15 15:20:12.000-0600

Repository: asterisk
Revision: 4690

U   trunk/channels/chan_mgcp.c

------------------------------------------------------------------------
r4690 | markster | 2008-01-15 15:20:12 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge distinctive ring for MGCP (bug ASTERISK-2831, with mods)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=4690