[asterisk-dev] [svn-commits] russell: branch russell/fxo_mwi r90795 - in /team/russell/fxo_mwi: channels/ ...
Tzafrir Cohen
tzafrir.cohen at xorcom.com
Tue Dec 4 05:30:13 CST 2007
Hi
Oh, good to see some progress. So I decided to add some pesky comments:
On Tue, Dec 04, 2007 at 12:15:48AM -0000, SVN commits to the Digium repositories wrote:
> Author: russell
> Date: Mon Dec 3 18:15:48 2007
> New Revision: 90795
>
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=90795
> Log:
> Add 1.4 based patch for adding MWI monitoring support for FXO lines. Some of
> this will have to be re-worked to work with how MWI works in trunk.
> (original patch from markster)
>
> Modified:
> team/russell/fxo_mwi/channels/chan_zap.c
> team/russell/fxo_mwi/configs/zapata.conf.sample
> team/russell/fxo_mwi/include/asterisk/callerid.h
> team/russell/fxo_mwi/main/callerid.c
>
> Modified: team/russell/fxo_mwi/channels/chan_zap.c
> URL: http://svn.digium.com/view/asterisk/team/russell/fxo_mwi/channels/chan_zap.c?view=diff&rev=90795&r1=90794&r2=90795
> ==============================================================================
> --- team/russell/fxo_mwi/channels/chan_zap.c (original)
> +++ team/russell/fxo_mwi/channels/chan_zap.c Mon Dec 3 18:15:48 2007
> @@ -224,6 +224,7 @@
>
> static char defaultcic[64] = "";
> static char defaultozz[64] = "";
> +static char externnotify[160] = "";
Do we really want to make the externnotify option a global?
When there is "externnotify" in users.conf, it may set both this and the
voicemail.conf option, right?
Also: is 160 long enough for something that includes a bit more than a
full path?
>
> static char progzone[10] = "";
>
> @@ -552,6 +553,7 @@
> unsigned int usedistinctiveringdetection:1;
> unsigned int zaptrcallerid:1; /*!< should we use the callerid from incoming call on zap transfer or not */
> unsigned int transfertobusy:1; /*!< allow flash-transfers to busy channels */
> + unsigned int mwimonitor:1;
> /* Channel state or unavilability flags */
> unsigned int inservice:1;
> unsigned int locallyblocked:1;
> @@ -608,6 +610,7 @@
> int callwaitingrepeat; /*!< How many samples to wait before repeating call waiting */
> int cidcwexpire; /*!< When to expire our muting for CID/CW */
> unsigned char *cidspill;
> + struct callerid_state *mwi;
Simply "mwi"? Maybe mwi_str?
> int cidpos;
> int cidlen;
> int ringt;
> @@ -737,6 +740,7 @@
> .mohinterpret = "default",
> .mohsuggest = "",
> .transfertobusy = 1,
> + .mwimonitor = 0,
This is unnecessary. The struct is initialized to zeros by default. A
value of 0 can simply be ommited.
>
> .cid_signalling = CID_SIG_BELL,
> .cid_start = CID_START_RING,
> @@ -1842,6 +1846,15 @@
> }
> ast_debug(1, "Disabled conferencing\n");
> return 0;
> +}
> +
/*! Run an external script to notify a voicemail message status change
@mailbox name of the mailbox
@thereornot New status of voicemail box
*/
> +static void notify_message(char *mailbox, int thereornot)
> +{
> + char s[256];
Why 256?
> + if (!ast_strlen_zero(mailbox) && !ast_strlen_zero(externnotify)) {
> + snprintf(s, sizeof(s), "%s %s %d", externnotify, mailbox, thereornot);
> + ast_safe_system(s);
> + }
> @@ -12505,6 +12561,8 @@
> confp->chan.immediate = ast_true(v->value);
> } else if (!strcasecmp(v->name, "transfertobusy")) {
> confp->chan.transfertobusy = ast_true(v->value);
> + } else if (!strcasecmp(v->name, "mwimonitor")) {
> + confp->chan.mwimonitor = ast_true(v->value);
> } else if (!strcasecmp(v->name, "cid_rxgain")) {
> if (sscanf(v->value, "%f", &confp->chan.cid_rxgain) != 1) {
> ast_log(LOG_WARNING, "Invalid cid_rxgain: %s\n", v->value);
> @@ -13067,7 +13125,9 @@
> ast_copy_string(defaultcic, v->value, sizeof(defaultcic));
> } else if (!strcasecmp(v->name, "defaultozz")) {
> ast_copy_string(defaultozz, v->value, sizeof(defaultozz));
> - }
> + } else if (!strcasecmp(v->name, "externnotify")) {
> + ast_copy_string(externnotify, v->value, sizeof(externnotify));
> + }
Any reason why this value should not be allowed to be set on reload?
> } else if (!skipchannels)
> ast_log(LOG_WARNING, "Ignoring %s\n", v->name);
> }
>
> Modified: team/russell/fxo_mwi/configs/zapata.conf.sample
> URL: http://svn.digium.com/view/asterisk/team/russell/fxo_mwi/configs/zapata.conf.sample?view=diff&rev=90795&r1=90794&r2=90795
> ==============================================================================
> --- team/russell/fxo_mwi/configs/zapata.conf.sample (original)
> +++ team/russell/fxo_mwi/configs/zapata.conf.sample Mon Dec 3 18:15:48 2007
> @@ -337,6 +337,20 @@
> ;
> ;hidecallerid=yes
> ;
> +; The following option enables receiving MWI on FXO lines. The default
> +; value is no. When this is enabled, and MWI notification indicates on or off,
> +; the script specified by the externnotify option is executed.
> +;
> +;mwimonitor=no
;mwimonitor=yes
The default is clearly stated above: "no". If this is to be useful as a
sample, make it just useful to unrem and get the second-best value.
> +;
> +; This option is used in conjunction with mwimonitor. This will get executed
> +; when incoming MWI state changes. The script is passed 2 arguments. The
> +; first is the corresponding mailbox, and the second is 1 or 0, indicating if
> +; there are messages waiting or not. An example script is provided
> +; in contrib/scripts/zapnotify.sh.
;
; This value is global and affects all channels, regardless of where it
; is placed in the configuration file.
Or, if indeed not allowed to be set on reload:
;
; This value is global and affects all channels, regardless of where it
; is placed in the configuration file. Furthermore, it may only be set
; on initial channel load, and ignored when reloading configuration.
(Yes, this is also true if you use users.conf where sections are used
and configuration should not "leak").
Also note that the script is still not there :-)
> +;
> +;externnotify=/usr/local/bin/zapnotify.sh
> +;
Cheers,
--
Tzafrir Cohen
icq#16849755 jabber:tzafrir.cohen at xorcom.com
+972-50-7952406 mailto:tzafrir.cohen at xorcom.com
http://www.xorcom.com iax:guest at local.xorcom.com/tzafrir
More information about the asterisk-dev
mailing list