chan_sip: Clarify 'Forcerport' Setting Displayed When Running "sip show peers"
authorMichael L. Young <elgueromexicano@gmail.com>
Mon, 28 Oct 2013 14:59:16 +0000 (14:59 +0000)
committerMichael L. Young <elgueromexicano@gmail.com>
Mon, 28 Oct 2013 14:59:16 +0000 (14:59 +0000)
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

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

UPGRADE-12.txt
channels/chan_sip.c

index 426b1a9..8652320 100644 (file)
@@ -25,6 +25,16 @@ There are many significant architectural changes in Asterisk 12. It is
 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)
index 791d693..52e7cdd 100644 (file)
@@ -19457,7 +19457,7 @@ int peercomparefunc(const void *a, const void *b)
 }
 
 /* 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 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
 
        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 @@ static struct sip_peer *_sip_show_peers_one(int fd, struct mansession *s, struct
                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 @@ static struct sip_peer *_sip_show_peers_one(int fd, struct mansession *s, struct
                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? */