[asterisk-dev] [Code Review] 2941: Improve Understanding Of 'Forcerport' When Running "sip show peers"

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2941/#review9957
-----------------------------------------------------------


I have a suggestion here. The Forcerport column has enough room in the column to hold up to ten characters. Why not go ahead and expand the words out entirely. You'd have:

Yes - force rport is enabled
No - force rport is disabled
Auto(Yes) - Set to auto, and has been enabled due to auto-detection
Auto(No) - Set to auto, and has not been enabled

You'd also need the Comedia column to hold ten characters to accommodate the expanded names. This way there's no cryptic 'a' vs. 'A' or 'N' meaning "NAT" vs. 'N' meaning "No".

- Mark Michelson


On Oct. 18, 2013, 9:17 p.m., Michael Young wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2941/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2013, 9:17 p.m.)
> 
> 
> Review request for Asterisk Developers and wdoekes.
> 
> 
> Bugs: ASTERISK-22728
>     https://issues.asterisk.org/jira/browse/ASTERISK-22728
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Walter Doekes pointed out in ASTERISK-22236, that when running "sip show peers", there can be confusion about what "N" means under the column "Forcerport". The "N" used to stand for NAT (yes). Now, that we use this same column for other settings involving the force_rport setting, someone could get confused as to what is meant by the N.
> 
> If "A" is displayed under it, it means auto_force_rport is turned on and force_rport is set to on (yes) for the peer. If "a" (lowercase) is displayed, it means auto_force_rport is turned on but force_rport is off (no) for the peer.
> 
> The proposed patch makes a minor change. If force_rport is off, it will use "N" to mean no instead of NAT. If force_rport is on, it will use "Y" to mean yes, the force_rport setting is on for the peer.
> 
> This mirrors what is displayed through the manager as well. The manager uses yes and no for these settings.
> 
> Also, this patch adds a column for the Comedia setting since that would be helpful to see if auto_force_rport/force_rport AND comedia/auto_comedia are turned on or not.
> 
> *** We are not sure whether we should expand the width of the display by adding the Comedia column.  So, I am putting this up for review to get feedback on not only on the changes described above but also we would be really interested in everyone's thoughts in regards to adding the ability to see the Comedia setting when "sip show settings" is run.
> 
> 
> Diffs
> -----
> 
>   /branches/11/UPGRADE.txt 401181 
>   /branches/11/channels/chan_sip.c 401182 
> 
> Diff: https://reviewboard.asterisk.org/r/2941/diff/
> 
> 
> Testing
> -------
> 
> Tested on local dev box.
> 
> 
> Thanks,
> 
> Michael Young
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131021/43424dcf/attachment.html>