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



branches/12/include/asterisk/stasis_app.h
<https://reviewboard.asterisk.org/r/3025/#comment19612>

    This was declared above.



branches/12/main/devicestate.c
<https://reviewboard.asterisk.org/r/3025/#comment19614>

    Unused.



branches/12/res/ari.make
<https://reviewboard.asterisk.org/r/3025/#comment19617>

    device_states should be snake_case instead of camelCase. That's my bad in 
the generator.
    
    Fixed on 12 in r402981. Merge from the latest 12, run make ari-stubs, and 
rename your files to snake_case.
    
    Sorry about that.



branches/12/res/ari/resource_deviceStates.c
<https://reviewboard.asterisk.org/r/3025/#comment19618>

    Isn't a 404 Not Found also possible here?



branches/12/res/ari/resource_deviceStates.c
<https://reviewboard.asterisk.org/r/3025/#comment19619>

    This should be a 409 Conflict, since it's a user error (wrong sort of 
device state) and not a server side error.



branches/12/res/ari/resource_deviceStates.c
<https://reviewboard.asterisk.org/r/3025/#comment19621>

    1. I would have thought that missing would be a 404, and unknown a 500
    2. You have a bad fall through to the default case.
    3. You shouldn't be using default anyways. Be specific, so when new 
response codes are added, GCC will tell us about missing cases in dev mode.



branches/12/res/ari/resource_deviceStates.c
<https://reviewboard.asterisk.org/r/3025/#comment19622>

    409



branches/12/res/ari/resource_deviceStates.c
<https://reviewboard.asterisk.org/r/3025/#comment19623>

    Similar comment as above.



branches/12/res/res_stasis.c
<https://reviewboard.asterisk.org/r/3025/#comment19615>

    No sense having both app_name and stasis_app_name. Just 
s/app_name/stasis_app_name/ and make it public.



branches/12/res/res_stasis.c
<https://reviewboard.asterisk.org/r/3025/#comment19616>

    Since res_stasis registers its own event sources, you might have a module 
reference problem here. It will bump its own reference count during module 
load. Since the unload won't run until the reference count goes back down to 
zero, and the unregister happens during the unload process, much sadness.


Mostly good. Just a few problems.

- David Lee


On Nov. 21, 2013, 1:12 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3025/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2013, 1:12 p.m.)
> 
> 
> Review request for Asterisk Developers, David Lee and Matt Jordan.
> 
> 
> Bugs: ASTERISK-22838
>     https://issues.asterisk.org/jira/browse/ASTERISK-22838
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Created a data model and implemented functionality for an ARI device state 
> resource.  The following operations have been added that allow a user to 
> manipulate an ARI controlled device:
> 
> PUT    /device-states/{deviceName}&{deviceState} - Create/Change the state of 
> an ARI controlled device
> GET    /device-states/{deviceName} - Retrieve the current state of a device
> DELETE /device-states/{deviceName} - Destroy a device-state controlled by ARI
> 
> The ARI controlled device must begin with 'Stasis:'.  An example controlled 
> device name would be Stasis:Example.
> 
> A 'DeviceStateChanged' event has also been added so that an application can 
> subscribe and receive device change events.  Any device state, ARI controlled 
> or not, can be subscribed to.
> 
> While adding the event, the underlying subscription control mechanism was 
> refactored so that all current and future resource subscriptions would be the 
> same.  Each event resource must now register itself in order to be able to 
> properly handle [un]subscribes.
> 
> 
> Diffs
> -----
> 
>   branches/12/rest-api/resources.json 402955 
>   branches/12/rest-api/api-docs/events.json 402955 
>   branches/12/rest-api/api-docs/deviceStates.json PRE-CREATION 
>   branches/12/rest-api/api-docs/applications.json 402955 
>   branches/12/res/stasis/app.c 402955 
>   branches/12/res/res_stasis_device_state.exports.in PRE-CREATION 
>   branches/12/res/res_stasis_device_state.c PRE-CREATION 
>   branches/12/res/res_stasis.c 402955 
>   branches/12/res/res_ari_deviceStates.c PRE-CREATION 
>   branches/12/res/ari/resource_deviceStates.c PRE-CREATION 
>   branches/12/res/ari/resource_deviceStates.h PRE-CREATION 
>   branches/12/res/ari/resource_applications.h 402955 
>   branches/12/res/ari/ari_model_validators.c 402955 
>   branches/12/res/ari/ari_model_validators.h 402955 
>   branches/12/res/ari.make 402955 
>   branches/12/main/devicestate.c 402955 
>   branches/12/include/asterisk/stasis_app_device_state.h PRE-CREATION 
>   branches/12/include/asterisk/stasis_app.h 402955 
>   branches/12/include/asterisk/devicestate.h 402955 
> 
> Diff: https://reviewboard.asterisk.org/r/3025/diff/
> 
> 
> Testing
> -------
> 
> Some basic testing done using the swagger framework and a websocket 
> connection.  Testing adding, changing, and removing device state with regards 
> to an ARI controlled device were successful.  Also tested [un]subscribing to 
> the events from device with success.  Planning on writing some testsuite test 
> cases as well.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to