[svn-commits] elguero: trunk r402113 - in /trunk: ./ UPGRADE-12.txt channels/chan_sip.c

Mon Oct 28 09:59:25 CDT 2013

Author: elguero
Date: Mon Oct 28 09:59:16 2013
New Revision: 402113

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=402113
Log:
chan_sip: Clarify 'Forcerport' Setting Displayed When Running "sip show peers"

While looking at ASTERISK-22236, Walter Doekes pointed out that when running
"sip show peers", the setting being displayed can be confusing.  The display of
"N" used to mean NAT (i.e. yes).  The NAT setting has gone through many
different changes resulting in the display of different characters to try and
convey what the current setting is for 'Forcerport' (A for Auto and Forcerport
is currently on, a for Auto but Forcerport is off, Y for yes, and N for no).
During the initial code review to try and clarify these settings (especially
since "N" no longer meant what it used to mean in prior versions of Asterisk),
Mark Michelson suggested using the full space available to display the settings
which helped to make the settings very clear.  That was a great suggestion.

Therefore, this patch does the following:

* The column for 'Forcerport' now will show: Auto (Yes), Auto (No), Yes, or No.

* A column for the 'Comedia' setting has been added.  It too will display the
  setting in a non-cryptic way: Auto (Yes), Auto (No), Yes, or No.

* UPGRADE.txt has been updated to document this change.

(closes issue ASTERISK-22728)
Reported by: Walter Doekes
Tested by: Michael L. Young
Patches:
    asterisk-forcerport-display-clarification_v3.diff
                                     uploaded by Michael L. Young (license 5026)

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

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

Merged revisions 402112 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/UPGRADE-12.txt
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/UPGRADE-12.txt
URL: http://svnview.digium.com/svn/asterisk/trunk/UPGRADE-12.txt?view=diff&rev=402113&r1=402112&r2=402113
==============================================================================

--- trunk/UPGRADE-12.txt (original)
+++ trunk/UPGRADE-12.txt Mon Oct 28 09:59:16 2013
@@ -25,6 +25,16 @@
 recommended that you not only read through this document for important
 changes that affect an upgrade, but that you also read through the CHANGES
 document in depth to better understand the new options available to you.
+chan_sip - Clarify The "sip show peers" Forcerport Column And Add Comedia
+ - Under the "Forcerport" column, the "N" used to mean NAT (i.e. Yes).  With
+   the additon of auto_* NAT settings, the meaning changed and there was a
+   certain combination of letters added to indicate the current setting. The
+   combination of using "Y", "N", "A" or "a", can be confusing.  Therefore, we
+   now display clearly what the current Forcerport setting is: "Yes", "No",
+   "Auto (Yes)", "Auto (No)".
+ - Since we are clarifying the Forcerport column, we have added a column to
+   display the Comedia setting since this is useful information as well.  We
+   no longer have a simple "NAT" setting like other versions before 11.
 
 Additional information on the architectural changes made in Asterisk can be
 found on the Asterisk wiki (https://wiki.asterisk.org)

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=402113&r1=402112&r2=402113
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Mon Oct 28 09:59:16 2013
@@ -19457,7 +19457,7 @@
 }
 
 /* the last argument is left-aligned, so we don't need a size anyways */
-#define PEERS_FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-3.3s %-8s %-11s %-32.32s %s\n"
+#define PEERS_FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-10.10s %-3.3s %-8s %-11s %-32.32s %s\n"
 
 /*! \brief Used in the sip_show_peers functions to pass parameters */
 struct show_peers_context {
@@ -19519,7 +19519,7 @@
 
 	if (!s) {
 		/* Normal list */
-		ast_cli(fd, PEERS_FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "ACL", "Port", "Status", "Description", (cont.realtimepeers ? "Realtime" : ""));
+		ast_cli(fd, PEERS_FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "Comedia", "ACL", "Port", "Status", "Description", (cont.realtimepeers ? "Realtime" : ""));
 	}
 
 	ao2_lock(peers);
@@ -19635,9 +19635,8 @@
 		ast_cli(fd, PEERS_FORMAT2, name,
 		tmp_host,
 		peer->host_dynamic ? " D " : "   ",	/* Dynamic or not? */
-		ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_RPORT) ?
-			ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? " A " : " a " :
-			ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? " N " : "   ",	/* NAT=yes? */
+		force_rport_string(peer->flags),
+		comedia_string(peer->flags),
 		(!ast_acl_list_is_empty(peer->acl)) ? " A " : "   ",       /* permit/deny */
 		tmp_port, status,
 		peer->description ? peer->description : "",
@@ -19668,7 +19667,7 @@
 		ast_sockaddr_isnull(&peer->addr) ? "0" : tmp_port,
 		peer->host_dynamic ? "yes" : "no",	/* Dynamic or not? */
 		ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_RPORT) ? "yes" : "no",
-		ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? "yes" : "no",	/* NAT=yes? */
+		ast_test_flag(&peer->flags[0], SIP_NAT_FORCE_RPORT) ? "yes" : "no",
 		ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_COMEDIA) ? "yes" : "no",
 		ast_test_flag(&peer->flags[1], SIP_PAGE2_SYMMETRICRTP) ? "yes" : "no",
 		ast_test_flag(&peer->flags[1], SIP_PAGE2_VIDEOSUPPORT) ? "yes" : "no",	/* VIDEOSUPPORT=yes? */


