[asterisk-dev] [Code Review] 2492: Initial support for endpoints.

Matt Jordan reviewboard at asterisk.org
Tue May 7 08:57:37 CDT 2013



> On May 6, 2013, 1:48 p.m., Matt Jordan wrote:
> > /trunk/main/stasis_endpoints.c, line 123
> > <https://reviewboard.asterisk.org/r/2492/diff/1/?file=37116#file37116line123>
> >
> >     I don't care for variables that are a single character that are not used as a counter in a loop - and it doesn't appear like 'v' is really needed either, as the return value of ast_json_object_set/ast_json_array_append could be checked directly.
> 
> David Lee wrote:
>     I agree that v was a poor choice in naming, but I dislike both multi-line if clauses and lines extending too far past 80 columns. Introducing the extra variable alleviates both.
> 
> Matt Jordan wrote:
>     Our line length is 90 columns :-)
>     
>     https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines#CodingGuidelines-Codeformatting
> 
> David Lee wrote:
>     And none of my lines exceed 90 columns :-P

No, but it does mean you don't have to limit yourself to a single character variable in order to not exceed 80 columns. You have another 10 characters to use!


- Matt


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


On May 6, 2013, 9:48 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2492/
> -----------------------------------------------------------
> 
> (Updated May 6, 2013, 9:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21421
>     https://issues.asterisk.org/jira/browse/ASTERISK-21421
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> An endpoint is an external device/system that may offer/accept
> channels to/from Asterisk. While this is a very useful concept for end
> users, it is surprisingly not a core concept within Asterisk itself.
> 
> This patch defines ast_endpoint as a separate object, which channel
> drivers may use to expose their concept of an endpoint. As the channel
> driver creates channels, it can use ast_endpoint_add_channel() to
> associate channels to the endpoint. This updated the endpoint
> appropriately, and forwards all of the channel's events to the
> endpoint's topic.
> 
> In order to avoid excessive locking on the endpoint object itself, the
> mutable state is not accessible via getters. Instead, you can create a
> snapshot using ast_endpoint_snapshot_create() to get a consistent
> snapshot of the internal state.
> 
> This patch also includes a set of topics and messages associated with
> endpoints, and implementations of the endpoint-related RESTful
> API. chan_sip was updated to create endpoints with SIP peers, but the
> state of the endpoints is not updated with the state of the peer.
> 
> Along for the ride in this patch is a Stasis test API. This is a
> stasis_message_sink object, which can be subscribed to a Stasis
> topic. It has functions for blocking while waiting for conditions in
> the message sink to be fulfilled.
> 
> (closes issue ASTERISK-21421)
> Review: https://reviewboard.asterisk.org/r/2492/
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 387290 
>   /trunk/channels/sip/include/sip.h 387290 
>   /trunk/include/asterisk/astobj2.h 387290 
>   /trunk/include/asterisk/endpoints.h PRE-CREATION 
>   /trunk/include/asterisk/stasis.h 387290 
>   /trunk/include/asterisk/stasis_endpoints.h PRE-CREATION 
>   /trunk/include/asterisk/stasis_test.h PRE-CREATION 
>   /trunk/main/asterisk.c 387290 
>   /trunk/main/astobj2.c 387290 
>   /trunk/main/channel_internal_api.c 387290 
>   /trunk/main/endpoints.c PRE-CREATION 
>   /trunk/main/stasis_cache.c 387290 
>   /trunk/main/stasis_endpoints.c PRE-CREATION 
>   /trunk/res/res_stasis_http_endpoints.c 387290 
>   /trunk/res/res_stasis_test.c PRE-CREATION 
>   /trunk/res/res_stasis_test.exports.in PRE-CREATION 
>   /trunk/res/stasis_http/resource_endpoints.h 387290 
>   /trunk/res/stasis_http/resource_endpoints.c 387290 
>   /trunk/rest-api/api-docs/endpoints.json 387290 
>   /trunk/tests/test_endpoints.c PRE-CREATION 
>   /trunk/tests/test_stasis_endpoints.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2492/diff/
> 
> 
> Testing
> -------
> 
> Unit tests!
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130507/654d47cc/attachment.htm>


More information about the asterisk-dev mailing list