[asterisk-dev] [Code Review] 2971: app_confbridge: Allow dynamically created menus via CONFBRIDGE function

[jrose]

> On Oct. 30, 2013, 6:36 p.m., rmudgett wrote:
> > /trunk/apps/confbridge/conf_config_parser.c, lines 1883-1884
> > <https://reviewboard.asterisk.org/r/2971/diff/1/?file=47672#file47672line1883>
> >
> >     This looks like a pre-existing bug.
> >     
> >     The order of the DTMF actions is reversed by this copy function.

Switched to INSERT_TAIL


> On Oct. 30, 2013, 6:36 p.m., rmudgett wrote:
> > /trunk/apps/confbridge/conf_config_parser.c, line 2246
> > <https://reviewboard.asterisk.org/r/2971/diff/1/?file=47672#file47672line2246>
> >
> >     You don't need to copy the menu into a new struct.  The purpose of this function is to install the DTMF hooks of the selected menu profile into the bridge features structure.

So in retrospect, I guess it is safe enough to just use a reference for menus created through configuration given how the config framework works. The referenced menus will persist without being editable and they'll simply be replaced in the list.

However, I probably do still need to have a copy of the menu stored in the channel datastore for the reasons I mentioned before. If the menu in the datastore can be manipulated by external AMI setting dialplan functions, that might lead to awkward behavior.


- jrose


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


On Oct. 29, 2013, 7:10 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2971/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2013, 7:10 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan, Mark Michelson, and rmudgett.
> 
> 
> Bugs: ASTERISK-22760
>     https://issues.asterisk.org/jira/browse/ASTERISK-22760
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Adds the ability to dynamically create menus using the CONFBRIDGE dialplan function. This includes the ability to use the existing options as well as to specify templates with the 'template' option.
> 
> I suppose it's worth mentioning that there is one minor bit of awkwardness. Both the channel datastore and the conference user will end up with an allocation of the menu. This is probably for the best for a couple reasons though. For instance, the CONFBRIDGE function can modify the contents of the datastore allocated menu while the user is still in the conference if it is set via AMI. We probably don't want the menu to be modified in place during this time (even if it would be cool to be able to edit a menu while the conference is in progress).
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/confbridge/include/confbridge.h 402126 
>   /trunk/apps/confbridge/conf_config_parser.c 402126 
>   /trunk/apps/app_confbridge.c 402126 
>   /trunk/CHANGES 402126 
> 
> Diff: https://reviewboard.asterisk.org/r/2971/diff/
> 
> 
> Testing
> -------
> 
> The following extensions were tested for successful entry with the expected options as well as not having any obvious memory leaks:
> 
> exten => 6060,1,NoOp
> exten => 6060,n,Set(CONFBRIDGE(menu,*)=playback_and_continue(conf-usermenu))
> exten => 6060,n,Set(CONFBRIDGE(menu,0)=leave_conference)
> exten => 6060,n,Set(CONFBRIDGE(menu,*9)=increase_volume)
> exten => 6060,n,Set(CONFBRIDGE(menu,*7)=decrease_volume)
> exten => 6060,n,Answer()
> exten => 6060,n,Confbridge(this_conference)
> 
> exten => 6061,1,Answer()
> exten => 6061,n,Confbridge(this_conference)
> 
> exten => 6062,1,Answer()
> exten => 6062,n,Confbridge(this_conference,,,sample_user_menu)
> 
> exten => 6063,1,NoOp
> exten => 6063,n,Set(CONFBRIDGE(menu,template)=sample_user_menu)
> exten => 6063,n,Set(CONFBRIDGE(menu,0)=leave_conference)
> exten => 6063,n,Answer()
> exten => 6063,n,Confbridge(this_conference)
> 
> With the following sample_user_menu configuration in confbridge.conf
> 
> [sample_user_menu]
> type=menu
> *=playback_and_continue(conf-usermenu)
> *1=toggle_mute
> 1=toggle_mute
> *4=decrease_listening_volume
> 4=decrease_listening_volume
> *6=increase_listening_volume
> 6=increase_listening_volume
> *7=decrease_talking_volume
> 7=decrease_talking_volume
> *8=leave_conference
> 8=leave_conference
> *9=increase_talking_volume
> 9=increase_talking_volume
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131030/32e953a7/attachment.html>