| Summary: | ASTERISK-11358: [patch] deprecate some obsolete MOH constructs | ||
| Reporter: | Dmitry Andrianov (dimas) | Labels: | |
| Date Opened: | 2008-02-01 13:57:19.000-0600 | Date Closed: | 2008-02-14 18:23:20.000-0600 |
| Priority: | Trivial | Regression? | No |
| Status: | Closed/Complete | Components: | Resources/res_musiconhold |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) v1-moh.patch ( 1) v2-moh.patch | |
| Description: | Attached patch adds duration parameter to MusicOnHold, deprecates WaitMusicOnHold in favor of enchanced MusicOnHold and also deprecates SetMusicOnHold in favor of Set(CHANNEL(musicclass)=...). Also MusicOnHold / StartMusicOnHold patched to use standard Asterisk macros for parsing application arguments. ****** ADDITIONAL INFORMATION ****** Made against trunk but I think the patch is applicable to 1.6 | ||
| Comments: | By: Mark Michelson (mmichelson) 2008-02-11 13:26:10.000-0600 Functionally, this patch seems fine, but there are a few minor things that can be changed. 1. Typically deprecation warnings are printed only once. With this patch, the deprecation warning will be printed with each execution of SetMusicOnHold and WaitMusicOnHold. 2. I'm a bit uncomfortable with treating a 0 second duration as a literal 0 second timeout. My opinion is that 0 should be treated as an indefinite duration. 3. Optional change: 'ast_strlen_zero(args.class) ? "default" : args.class' can be changed to 'S_OR(args.class, "default)' 4. When deprecating an application, please make a note in the UPGRADE.txt file. Otherwise the patch seems fine. I always enjoy reviewing your patches, dimas! Keep up the good work! By: Dmitry Andrianov (dimas) 2008-02-11 14:43:09.000-0600 Thanks. 1. I fixed warnings to behave like others. However I did not put that check intentionally - I believe that regular warnings on the console is the only way to attract user's attention to deprecated constructs. When such a warning appears only once, there is a hight risk it won't be noticed. 2. fixed 3. fixed. However I did S_OR(args.class, NULL) instead. This is because when args.class is empty, ast_moh_start not necesarrily uses "default" - it can use something set with CHANNEL(musicclass)=... instead so saying we could not play "default" was a bit misleading. Now it says "<null>". The ast_moh_start itself displays more detailed information anyway. 4. fixed By: Mark Michelson (mmichelson) 2008-02-13 09:08:12.000-0600 Excellent! Thanks for the quick update. By: Digium Subversion (svnbot) 2008-02-13 09:44:10.000-0600 Repository: asterisk Revision: 103658 U trunk/UPGRADE.txt U trunk/res/res_musiconhold.c ------------------------------------------------------------------------ r103658 | mmichelson | 2008-02-13 09:44:08 -0600 (Wed, 13 Feb 2008) | 10 lines 1. Deprecate SetMusicOnHold and WaitMusicOnHold. 2. Add a duration parameter to MusicOnHold (closes issue ASTERISK-11358) Reported by: dimas Patches: v2-moh.patch uploaded by dimas (license 88) Tested by: dimas ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=103658 By: Digium Subversion (svnbot) 2008-02-14 18:23:20.000-0600 Repository: asterisk Revision: 103708 _U team/murf/bug11210/ U team/murf/bug11210/UPGRADE.txt U team/murf/bug11210/apps/app_externalivr.c U team/murf/bug11210/apps/app_queue.c U team/murf/bug11210/apps/app_voicemail.c U team/murf/bug11210/channels/chan_iax2.c U team/murf/bug11210/configs/queues.conf.sample U team/murf/bug11210/configure U team/murf/bug11210/configure.ac U team/murf/bug11210/doc/tex/imapstorage.tex U team/murf/bug11210/funcs/func_cdr.c U team/murf/bug11210/include/asterisk/autoconfig.h.in U team/murf/bug11210/res/res_agi.c U team/murf/bug11210/res/res_musiconhold.c ------------------------------------------------------------------------ r103708 | murf | 2008-02-14 18:23:17 -0600 (Thu, 14 Feb 2008) | 187 lines Merged revisions 103658,103662,103668,103677,103682,103685,103687,103689,103691,103694,103699-103700,103702,103704-103705 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r103658 | mmichelson | 2008-02-13 08:47:25 -0700 (Wed, 13 Feb 2008) | 10 lines 1. Deprecate SetMusicOnHold and WaitMusicOnHold. 2. Add a duration parameter to MusicOnHold (closes issue ASTERISK-11358) Reported by: dimas Patches: v2-moh.patch uploaded by dimas (license 88) Tested by: dimas ................ r103662 | jpeeler | 2008-02-13 14:04:31 -0700 (Wed, 13 Feb 2008) | 6 lines (closes issue ASTERISK-11286) Reported by: ctooley Patches: additional_eivr_commands.patch uploaded by ctooley (license 136) Tested by: ctooley ................ r103668 | oej | 2008-02-14 03:19:09 -0700 (Thu, 14 Feb 2008) | 2 lines Formatting fixes ................ r103677 | qwell | 2008-02-14 11:39:51 -0700 (Thu, 14 Feb 2008) | 11 lines Add periodic jitter stats to CLI and manager. (closes issue ASTERISK-7968) Reported by: stevedavies Patches: jblogging-trunk.patch uploaded by stevedavies jblogging-trunk_wmgrevent.patch uploaded by johann8384 updated_jbloggin-trunk_mgrevent.patch uploaded by johann8384 (license 190) (with additional changes by me) Tested by: stevedavies, johann8384 ................ r103682 | jpeeler | 2008-02-14 12:47:39 -0700 (Thu, 14 Feb 2008) | 1 line a few syntax changes and safer code ................ r103685 | qwell | 2008-02-14 12:52:21 -0700 (Thu, 14 Feb 2008) | 13 lines Merged revisions 103683 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r103683 | qwell | 2008-02-14 13:51:10 -0600 (Thu, 14 Feb 2008) | 5 lines Document the 'l' option to the CDR() function. (Thanks voipgate for pointing out the option, and Leif for providing text for it.) Closes issue ASTERISK-11166. ........ ................ r103687 | mmichelson | 2008-02-14 13:46:00 -0700 (Thu, 14 Feb 2008) | 10 lines Change the queue holdtime announcement to happen at any interval (not just greater than two minutes). Remove the saying of less-than for holdtime announcements since it can lead to awkward holdtime announcements. Using '1' as a queue-round-seconds value is no longer valid. (closes issue ASTERISK-9451) Reported by: caio1982 Patches: queue_announce5.diff uploaded by caio1982 (license 22) Tested by: caio1982, putnopvut ................ r103689 | mmichelson | 2008-02-14 13:58:30 -0700 (Thu, 14 Feb 2008) | 17 lines Merged revisions 103688 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r103688 | mmichelson | 2008-02-14 14:55:48 -0600 (Thu, 14 Feb 2008) | 9 lines Fix the new message count if delete=yes when using IMAP storage. (closes issue ASTERISK-10917) Reported by: jaroth Patches: deleteflag_v2.patch uploaded by jaroth (license 50) Tested by: jaroth ........ ................ r103691 | mmichelson | 2008-02-14 14:04:37 -0700 (Thu, 14 Feb 2008) | 11 lines Merged revisions 103690 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r103690 | mmichelson | 2008-02-14 15:03:02 -0600 (Thu, 14 Feb 2008) | 3 lines Fix build for non-IMAP builds ........ ................ r103694 | qwell | 2008-02-14 14:21:53 -0700 (Thu, 14 Feb 2008) | 8 lines Modify ldap autoconf function, so that an incorrect ldap library is not found on Solaris (it is incompatible). Also removes second check for awk, which causes Solaris to find an incompatible version of awk. (closes issue ASTERISK-11290) Reported by: snuffy Patches: bug-11829.diff uploaded by snuffy (license 35) ................ r103699 | mmichelson | 2008-02-14 16:35:21 -0700 (Thu, 14 Feb 2008) | 20 lines Blocked revisions 103698 via svnmerge ........ r103698 | mmichelson | 2008-02-14 17:30:17 -0600 (Thu, 14 Feb 2008) | 13 lines Change to the configure logic regarding IMAP. Prior to this commit, if you wished to configure Asterisk with IMAP support, you would use the --with-imap configure switch in one of the following two ways: --with-imap=/some/directory would look in the directory specified for a UW IMAP source installation --with-imap would assume that you had imap-2004g installed in .. relative to the Asterisk source With this set of changes the two above options still work the same, but there are two new behaviors, too. --with-imap=system will assume that you have -libc-client.so where you store your shared objects and will attempt to find c-client headers in your include path either in the imap or c-client directory. If either of the two original methods of specifying the imap option should fail, then the check for --with-imap =system will be performed in addition. It is only after this "system" check that failure can happen. ........ ................ r103700 | mmichelson | 2008-02-14 16:39:47 -0700 (Thu, 14 Feb 2008) | 5 lines See commit message for svn revision 103698. This behavior is the same as what is described there. The difference is that trunk already had the --with-imap=system option, but it only checked the include path for headers in the imap directory and not also the c-client directory. ................ r103702 | mmichelson | 2008-02-14 16:48:12 -0700 (Thu, 14 Feb 2008) | 10 lines Blocked revisions 103701 via svnmerge ........ r103701 | mmichelson | 2008-02-14 17:44:17 -0600 (Thu, 14 Feb 2008) | 3 lines Update documentation regarding configuration of IMAP ........ ................ r103704 | mmichelson | 2008-02-14 16:49:54 -0700 (Thu, 14 Feb 2008) | 10 lines Blocked revisions 103703 via svnmerge ........ r103703 | mmichelson | 2008-02-14 17:49:24 -0600 (Thu, 14 Feb 2008) | 3 lines Make a small clarification in the documentation ........ ................ r103705 | mmichelson | 2008-02-14 16:51:49 -0700 (Thu, 14 Feb 2008) | 3 lines Trunk version of 1.4's imap documentation updates ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=103708 | ||