[asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

[rmudgett]

> On Jan. 6, 2015, 10:13 a.m., Kevin Harwell wrote:
> > Since we are bumping the API should this be mentioned in the UPGRADE file?

I'll add a note in UPGRADE.


> On Jan. 6, 2015, 10:13 a.m., Kevin Harwell wrote:
> > /branches/13/include/asterisk/manager.h, lines 308-315
> > <https://reviewboard.asterisk.org/r/4315/diff/1/?file=70150#file70150line308>
> >
> >     If listflag is no longer used do we want to deprecate this function and have a new one without the parameter.

I can remove the parameter on the trunk version of the patch.


> On Jan. 6, 2015, 10:13 a.m., Kevin Harwell wrote:
> > /branches/13/include/asterisk/manager.h, lines 317-352
> > <https://reviewboard.asterisk.org/r/4315/diff/1/?file=70150#file70150line317>
> >
> >     Just a suggestion, but using a variable argument list you could combine these two functions.

True.  However, this way allows more flexibility in generating any additional AMI headers for the event.


- rmudgett


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


On Jan. 2, 2015, 6:42 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4315/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2015, 6:42 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24049
>     https://issues.asterisk.org/jira/browse/ASTERISK-24049
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> * Made the following AMI actions use list API calls for consistency:
> Agents
> BridgeInfo
> BridgeList
> BridgeTechnologyList
> ConfbridgeLIst
> ConfbridgeLIstRooms
> CoreShowChannels
> DAHDIShowChannels
> DBGet
> DeviceStateList
> ExtensionStateList
> FAXSessions
> Hangup
> IAXpeerlist
> IAXpeers
> IAXregistry
> MeetmeList
> MeetmeListRooms
> MWIGet
> ParkedCalls
> Parkinglots
> PJSIPShowEndpoint
> PJSIPShowEndpoints
> PJSIPShowRegistrationsInbound
> PJSIPShowRegistrationsOutbound
> PJSIPShowResourceLists
> PJSIPShowSubscriptionsInbound
> PJSIPShowSubscriptionsOutbound
> PresenceStateList
> PRIShowSpans
> QueueStatus
> QueueSummary
> ShowDialPlan
> SIPpeers
> SIPpeerstatus
> SIPshowregistry
> SKINNYdevices
> SKINNYlines
> Status
> VoicemailUsersList
> 
> * Incremented the AMI version to 2.7.0.
> 
> * Changed astman_send_listack() to not use the listflag parameter and
> always set the value to "Start" so the start capitalization is consistent.
> i.e., The FAXSessions used "Start" while the rest of the system used
> "start".  The corresponding complete event always used "Complete".
> 
> * Fixed ami_show_resource_lists() "PJSIPShowResourceLists" to output the
> AMI ActionID for all of its list events.
> 
> * Fixed off-nominal AMI protocol error in manager_bridge_info(),
> manager_parking_status_single_lot(), and
> manager_parking_status_all_lots().  Use of astman_send_error() after
> responding to the original AMI action request violates the action response
> pattern by sending two responses.
> 
> * Fixed minor protocol error in action_getconfig() when no requested
> categories are found.  Each line needs to be formatted as "Header: text".
> 
> * Fixed off-nominal memory leak in manager_build_parked_call_string().
> 
> * Eliminated unnecessary use of RAII_VAR() in ami_subscription_detail().
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip_registrar.c 430178 
>   /branches/13/res/res_pjsip_pubsub.c 430178 
>   /branches/13/res/res_pjsip_outbound_registration.c 430178 
>   /branches/13/res/res_pjsip/pjsip_configuration.c 430178 
>   /branches/13/res/res_mwi_external_ami.c 430178 
>   /branches/13/res/res_manager_presencestate.c 430178 
>   /branches/13/res/res_manager_devicestate.c 430178 
>   /branches/13/res/res_fax.c 430178 
>   /branches/13/res/parking/parking_manager.c 430178 
>   /branches/13/main/pbx.c 430178 
>   /branches/13/main/manager_bridges.c 430178 
>   /branches/13/main/manager.c 430178 
>   /branches/13/main/db.c 430178 
>   /branches/13/main/bridge.c 430178 
>   /branches/13/include/asterisk/manager.h 430178 
>   /branches/13/channels/chan_skinny.c 430178 
>   /branches/13/channels/chan_sip.c 430178 
>   /branches/13/channels/chan_iax2.c 430178 
>   /branches/13/channels/chan_dahdi.c 430178 
>   /branches/13/apps/app_voicemail.c 430178 
>   /branches/13/apps/app_queue.c 430178 
>   /branches/13/apps/app_meetme.c 430178 
>   /branches/13/apps/app_confbridge.c 430178 
>   /branches/13/apps/app_agent_pool.c 430178 
> 
> Diff: https://reviewboard.asterisk.org/r/4315/diff/
> 
> 
> Testing
> -------
> 
> Issued all of the AMI actions listed above to verify that the output was consistent.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150107/1dc8820c/attachment-0001.html>