[asterisk-dev] [Code Review] 2959: pjsip: AMI commands and events

[Matt Jordan]

> On Oct. 24, 2013, 10:50 p.m., Paul Belanger wrote:
> > My only comment is about the naming conventions, we should try and keep AMI events in sync.  Right now we have FooBegin / FooEnd, and FooStart / FooStop, and Foo / FooComplete.  I'd like us see about pick one format and sticking with it when possible.
> > 
> > I believe Begin / End has been the most recent addition, do we want to default to that?

There is no proposed convention for that in AMI 1.4. I really don't want to go through and refactor all of those events again, particularly since that specification was put up for review nearly a year ago.

That being said, the Begin/End and Start/Stop nomenclature could be considered to be inconsistent; in general, people have gone with what they thought made sense based on the semantics the event was conveying. If someone want to propose a consistent scheme for that I'd be fine with that, but I'd rather see that on the -dev list and - once someone has made a decision - a patch provided that does the change.

As far as Foo/FooComplete is concerned, however, those are used to indicate when a list of events has finished that was initiated by an action. Those are not inconsistent with the others, and changing them to something else feels incredibly nit picky.

As far as the substatus field goes, the specification for AMI went with explicit event names to indicate the beginning and ending of a pair of events. That approach was chosen as it allows libraries to key off of the begin or end events if that is the only event that they care about; this reduces the amount of work implementers have to do as they don't have to inspect a sub type field to determine whether or not they care about the event.


- Matt


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


On Oct. 24, 2013, 5:43 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2959/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2013, 5:43 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22609
>     https://issues.asterisk.org/jira/browse/ASTERISK-22609
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Created the following AMI commands and corresponding events for res_pjsip:
> 
> PJSIPShowEndpoints - Provides a listing of all pjsip endpoints and a few select attributes on each.
>   Events:
>     EndpointList - for each endpoint a few attributes
>     EndpointlistComplete - after all endpoints have been listed
> 
> PJSIPShowEndpoint - Provides a detail list of attributes for a specified endpoint
>   Events:
>     EndpointDetail - attributes on an endpoint
>     AorDetail - raised for each AOR on an endpoint
>     AuthDetail - raised for each associated inbound and outbound auth
>     TransportDetail - transport attributes
>     IdentifyDetail - attributes for the identify object associated with the endpoint
>     EndpointDetailComplete - last event raised after all detail events
> 
> PJSIPShowRegistrationsInbound - Provides a detail listing of all inbound registrations
>   Events:
>     InboundRegistrationDetail - inbound registration attributes for each registration
>     InboundRegistrationDetailComplete - raised after all detail records have been listed
> 
> PJSIPShowRegistrationsOutbound  - Provides a detail listing of all outbound registrations
>   Events:
>     OutboundRegistrationDetail - outbound registration attributes for each registration
>     OutboundRegistrationDetailComplete - raised after all detail records have been listed
> 
> PJSIPShowSubscriptions - A detail listing of all subscriptions and their attributes
>   Events:
>     SubscriptionDetail - on each subscription detailed attributes
>     SubscriptionDetailComplete - raised after all detail records have been listed
> 
> 
> Diffs
> -----
> 
>   branches/12/include/asterisk/acl.h 401659 
>   branches/12/include/asterisk/manager.h 401659 
>   branches/12/include/asterisk/res_pjsip.h 401659 
>   branches/12/include/asterisk/res_pjsip_pubsub.h 401659 
>   branches/12/include/asterisk/sorcery.h 401659 
>   branches/12/include/asterisk/strings.h 401659 
>   branches/12/main/acl.c 401659 
>   branches/12/main/manager.c 401659 
>   branches/12/main/sorcery.c 401659 
>   branches/12/main/utils.c 401659 
>   branches/12/res/res_pjsip.c 401659 
>   branches/12/res/res_pjsip.exports.in 401659 
>   branches/12/res/res_pjsip/config_auth.c 401659 
>   branches/12/res/res_pjsip/config_transport.c 401659 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 401659 
>   branches/12/res/res_pjsip/location.c 401659 
>   branches/12/res/res_pjsip/pjsip_configuration.c 401659 
>   branches/12/res/res_pjsip_endpoint_identifier_ip.c 401659 
>   branches/12/res/res_pjsip_exten_state.c 401659 
>   branches/12/res/res_pjsip_mwi.c 401659 
>   branches/12/res/res_pjsip_outbound_registration.c 401659 
>   branches/12/res/res_pjsip_pubsub.c 401659 
>   branches/12/res/res_pjsip_registrar.c 401659 
> 
> Diff: https://reviewboard.asterisk.org/r/2959/diff/
> 
> 
> Testing
> -------
> 
> Ran a few manual tests over AMI and observed the output to make sure things look correct.  Also wrote some testsuite tests: https://reviewboard.asterisk.org/r/2958/
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131025/635c215e/attachment.html>