Re: [asterisk-dev] [Code Review] 2959: pjsip: AMI commands and events
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2959/#review10237 --- Ship it! branches/12/res/res_pjsip_pubsub.c https://reviewboard.asterisk.org/r/2959/#comment19578 Introduced a couple of red blobs here. - Mark Michelson On Nov. 15, 2013, 7:58 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2959/ --- (Updated Nov. 15, 2013, 7:58 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/res/res_pjsip_registrar.c 402377 branches/12/res/res_pjsip_pubsub.c 402377 branches/12/res/res_pjsip_outbound_registration.c 402377 branches/12/res/res_pjsip_mwi.c 402377 branches/12/res/res_pjsip_exten_state.c 402377 branches/12/res/res_pjsip_endpoint_identifier_ip.c 402377 branches/12/res/res_pjsip/pjsip_configuration.c 402377 branches/12/res/res_pjsip/location.c 402377 branches/12/res/res_pjsip/include/res_pjsip_private.h 402377 branches/12/res/res_pjsip/config_transport.c 402377 branches/12/res/res_pjsip/config_auth.c 402377 branches/12/res/res_pjsip.exports.in 402377 branches/12/res/res_pjsip.c 402377 branches/12/main/utils.c 402377 branches/12/main/sorcery.c 402377 branches/12/main/acl.c 402377 branches/12/include/asterisk/utils.h 402377 branches/12/include/asterisk/strings.h 402377 branches/12/include/asterisk/sorcery.h 402377 branches/12/include/asterisk/res_pjsip_pubsub.h 402377 branches/12/include/asterisk/res_pjsip.h 402377 branches/12/include/asterisk/acl.h 402377 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 -- _ -- 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
Re: [asterisk-dev] [Code Review] 2987: ARI: Don't leak information about implementation details
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2987/#review10232 --- Ship it! branches/12/include/asterisk/stasis.h https://reviewboard.asterisk.org/r/2987/#comment19576 sanitize branches/12/include/asterisk/stasis.h https://reviewboard.asterisk.org/r/2987/#comment19577 sanitize - Joshua Colp On Nov. 11, 2013, 3:12 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2987/ --- (Updated Nov. 11, 2013, 3:12 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22744 https://issues.asterisk.org/jira/browse/ASTERISK-22744 Repository: Asterisk Description --- This change prevents channels used as implementation details from leaking out to ARI. It does this by preventing creation of JSON blobs of channel snapshots created from those channels and sanitizing JSON blobs of bridge snapshots as they are created. This introduces a framework for excluding information from output targeted at Stasis applications on a consumer-by-consumer basis using channel sanitization callbacks which could be extended to bridges or endpoints if necessary. This results in NULL inputs to ast_json_pack calls which generate unhelpful error messages, so that has been dealt with as well. This also corrects a bug I noticed while investigating the issue where BridgeCreated events would not be created. Diffs - branches/12/res/stasis/app.c 402347 branches/12/res/res_stasis.c 402347 branches/12/res/ari/resource_endpoints.c 402347 branches/12/res/ari/resource_channels.c 402347 branches/12/res/ari/resource_bridges.c 402347 branches/12/main/stasis_message.c 402347 branches/12/main/stasis_endpoints.c 402347 branches/12/main/stasis_channels.c 402347 branches/12/main/stasis_bridges.c 402347 branches/12/main/rtp_engine.c 402347 branches/12/main/json.c 402347 branches/12/include/asterisk/stasis_endpoints.h 402347 branches/12/include/asterisk/stasis_channels.h 402347 branches/12/include/asterisk/stasis_bridges.h 402347 branches/12/include/asterisk/stasis_app.h 402347 branches/12/include/asterisk/stasis.h 402347 Diff: https://reviewboard.asterisk.org/r/2987/diff/ Testing --- Manual testing with bridges and channels. Thanks, opticron -- _ -- 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
Re: [asterisk-dev] [Code Review] 3016: app_directory: Set DIRECTORY_RESULT variable to indicate why Directory application finished
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3016/ --- (Updated Nov. 21, 2013, 4:38 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers, Mark Michelson and rmudgett. Changes --- Committed in revision 402995 Repository: Asterisk Description --- Directory is a bit of a mess stylistically... so I tried to manage this as best as I could. The premise is simple enough. Directory should set a reason for exit when it is finished. The reasons include: OPERATORuser requested operator by pressing '0' for operator ASSISTANT user requested assistant by pressing '*' for assistant TIMEOUT user pressed nothing and Directory stopped waiting HANGUP user's channel hung up SELECTEDuser selected a user from the directory and is routed USEREXITuser pressed '#' from the selection prompt to exit FAILED directory failed in a way that wasn't accounted for. Diffs - /trunk/apps/app_directory.c 402851 /trunk/CHANGES 402851 Diff: https://reviewboard.asterisk.org/r/3016/diff/ Testing --- I tested that each of the values would be received from their respective actions with the exception of FAILED... which I put in there to account for exit conditions that I wasn't aware of anyway. I'm currently writing a testsuite test to verify the behavior of this feature. Thanks, Jonathan Rose -- _ -- 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
Re: [asterisk-dev] [Code Review] 2994: ari:Add application/json parameter support
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2994/ --- (Updated Nov. 21, 2013, 2:47 p.m.) Review request for Asterisk Developers and Paul Belanger. Changes --- Properly handle allocations when both query params and JSON body params are offered. Bugs: ASTERISK-22685 https://issues.asterisk.org/jira/browse/ASTERISK-22685 Repository: Asterisk Description --- The patch allows ARI to parse request parameters from an incoming JSON request body, instead of requiring the request to come in as query parameters (which is just weird for POST and DELETE) or form parameters (which is okay, but a bit asymmetric given that all of our responses are JSON). For any operation that does _not_ have a parameter defined of type body (i.e. paramType: body in the API declaration), if a request provides a request body with a Content type of application/json, the provided JSON document is parsed and searched for parameters. The expected fields in the provided JSON document should match the query parameters defined for the operation. If the parameter has 'allowMultiple' set, then the field in the JSON document may optionally be an array of values. Diffs (updated) - /branches/12/tests/test_ari.c 402455 /branches/12/rest-api-templates/swagger_model.py 402455 /branches/12/rest-api-templates/res_ari_resource.c.mustache 402455 /branches/12/rest-api-templates/param_parsing.mustache 402455 /branches/12/rest-api-templates/asterisk_processor.py 402455 /branches/12/res/res_ari_sounds.c 402455 /branches/12/res/res_ari_recordings.c 402455 /branches/12/res/res_ari_playback.c 402455 /branches/12/res/res_ari_endpoints.c 402455 /branches/12/res/res_ari_channels.c 402455 /branches/12/res/res_ari_bridges.c 402455 /branches/12/res/res_ari_asterisk.c 402455 /branches/12/res/res_ari_applications.c 402455 /branches/12/res/res_ari.c 402455 /branches/12/main/http.c 402455 /branches/12/include/asterisk/http.h 402455 /branches/12/include/asterisk/ari.h 402455 Diff: https://reviewboard.asterisk.org/r/2994/diff/ Testing --- Testsuite test. See https://reviewboard.asterisk.org/r/2993/ Thanks, David Lee -- _ -- 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
On Nov. 21, 2013, 12:59 a.m., Paul Belanger wrote:
branches/12/rest-api/api-docs/device_states.json, line 11
https://reviewboard.asterisk.org/r/3025/diff/1/?file=48553#file48553line11
Hmm, not a fan of this URL schema. At first glance, what about just
/states.
Additionally, I know within asterisk it is a device state however from
ARI we don't actually have a device, we have endpoints. So, that in itself
is confusing.
I think the snake_case here is the issue, since most everything else
has moved to camelCase. Consistency FTW!
I completely disagree with changing this to /states. That's not descriptive
enough and is completely generic, which the API itself is not. If I'm a
developer looking at that the first question I'd ask would be states for what?
channels?. Having something else with it (even if not device) gives immediate
information and makes it much clearer.
- Joshua
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/#review10226
---
On Nov. 20, 2013, 10:25 p.m., Kevin Harwell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/
---
(Updated Nov. 20, 2013, 10:25 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 402915
branches/12/rest-api/api-docs/events.json 402915
branches/12/rest-api/api-docs/device_states.json PRE-CREATION
branches/12/rest-api/api-docs/applications.json 402915
branches/12/res/stasis/app.c 402915
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 402915
branches/12/res/res_ari_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.h PRE-CREATION
branches/12/res/ari/resource_applications.h 402915
branches/12/res/ari/ari_model_validators.c 402915
branches/12/res/ari/ari_model_validators.h 402915
branches/12/res/ari.make 402915
branches/12/main/devicestate.c 402915
branches/12/include/asterisk/stasis_app_device_state.h PRE-CREATION
branches/12/include/asterisk/stasis_app.h 402915
branches/12/include/asterisk/devicestate.h 402915
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
Re: [asterisk-dev] [Code Review] 2947: ARI: Adding a channel to a bridge while a live recording is active blocks
On Nov. 21, 2013, 12:53 p.m., opticron wrote: branches/12/include/asterisk/stasis_app.h, line 180 https://reviewboard.asterisk.org/r/2947/diff/5/?file=48327#file48327line180 This should return the enum instead of int. Kevin Harwell wrote: The problem with making this an enum is currently it is assumed different enum types could be returned. So instead of having a single master enum for all result types, resources could define and use their expected result types specific to their modules. If this could return multiple different enum types, collisions between values in the enums could produce incorrect error messages. As an example, a rule from module B could cause a bridge entry attempted by module A to fail. The enum that currently exists is global and not module-specific even though the only error value it contains is from a single module. - opticron --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2947/#review10246 --- On Nov. 21, 2013, 1:50 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2947/ --- (Updated Nov. 21, 2013, 1:50 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22624 https://issues.asterisk.org/jira/browse/ASTERISK-22624 Repository: Asterisk Description --- Added a channel recording flag that is checked before adding a channel to a bridge. If the the channel is currently being recorded a 409 Conflict is returned and the channel is not added to the bridge. Diffs - branches/12/rest-api/api-docs/bridges.json 402969 branches/12/res/stasis/control.c 402969 branches/12/res/stasis/command.c 402969 branches/12/res/stasis/command.h 402969 branches/12/res/res_stasis_recording.c 402969 branches/12/res/res_stasis_playback.c 402969 branches/12/res/res_stasis_answer.c 402969 branches/12/res/res_ari_bridges.c 402969 branches/12/res/ari/resource_bridges.c 402969 branches/12/include/asterisk/stasis_app_impl.h 402969 branches/12/include/asterisk/stasis_app.h 402969 Diff: https://reviewboard.asterisk.org/r/2947/diff/ Testing --- Ran a manual test and also wrote a testsuite test: https://reviewboard.asterisk.org/r/2951/ 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
Re: [asterisk-dev] [Code Review] 3024: Testsuite: Test the MixMonitor 'f' option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3024/#review10240 --- Ship it! Ship It! - Joshua Colp On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3024/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This adds a simple test that ensures the 'f' option to MixMonitor performs as documented. Diffs - /asterisk/trunk/tests/apps/tests.yaml 4331 /asterisk/trunk/tests/apps/mixmonitor_f_option/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/mixmonitor_f_option/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3024/diff/ Testing --- Test consistently passes. Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 2959: pjsip: AMI commands and events
George Joseph wrote: Just FYI... I've got a whole bunch of pjsip cli stuff waiting on this patch to be committed. Any chance of this happening this week? It'll get reviewed today and pending any other findings arising it'll go in shortly. If possible you could just throw your stuff up for review too and mark it as dependent on the AMI commands/events review (you can do that in Reviewboard now! yay!). Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: www.digium.com www.asterisk.org -- _ -- 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
Re: [asterisk-dev] [Code Review] 3023: Add MixMonitor() option to specify channel variable into which to store the recording filename
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/#review10242 --- Shouldn't something like this be a channel function from which you retrieve the value, instead of specifying a variable into which the name is placed? Seems like a significant regression in spitting things out to channel variables, instead of using dialplan functions to retrieve values when needed. - Tilghman Lesher On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This change introduces an 'f' option for the MixMonitor() application. The argument for this option specifies a channel variable into which the application should store the filename (as in, the full path) instead of the standard MIXMONITOR_FILENAME. The rationale for this option is that MixMonitor is structured in such a way that a single channel may have multiple simultaneous MixMonitors running at any given time. Thus, the MixMonitor application should not have any assumptions that the channel has only a single MixMonitor being run on it. Setting a single MIXMONITOR_FILENAME channel variable violates such assumptions by overwriting the value set from previous invocations of MixMonitor. By using the 'f' and 'i' options for MixMonitor, a user can easily manage multiple recordings on a single channel. Diffs - /trunk/apps/app_mixmonitor.c 402883 Diff: https://reviewboard.asterisk.org/r/3023/diff/ Testing --- See https://reviewboard.asterisk.org/r/3024/ Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/#review10254
---
branches/12/include/asterisk/stasis_app.h
https://reviewboard.asterisk.org/r/3025/#comment19605
If this is going to be exposed globally, it needs to be renamed.
branches/12/include/asterisk/stasis_app.h
https://reviewboard.asterisk.org/r/3025/#comment19603
There's no need to forward declare this a second time.
branches/12/res/res_stasis_device_state.c
https://reviewboard.asterisk.org/r/3025/#comment19613
Why not search via OBJ_SEARCH_KEY? This seems like a pretty expensive
operation when it doesn't have to be.
- opticron
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
Re: [asterisk-dev] [Code Review] 2947: ARI: Adding a channel to a bridge while a live recording is active blocks
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2947/ --- (Updated Nov. 21, 2013, 1:50 p.m.) Review request for Asterisk Developers. Changes --- Update per review items Bugs: ASTERISK-22624 https://issues.asterisk.org/jira/browse/ASTERISK-22624 Repository: Asterisk Description --- Added a channel recording flag that is checked before adding a channel to a bridge. If the the channel is currently being recorded a 409 Conflict is returned and the channel is not added to the bridge. Diffs (updated) - branches/12/rest-api/api-docs/bridges.json 402969 branches/12/res/stasis/control.c 402969 branches/12/res/stasis/command.c 402969 branches/12/res/stasis/command.h 402969 branches/12/res/res_stasis_recording.c 402969 branches/12/res/res_stasis_playback.c 402969 branches/12/res/res_stasis_answer.c 402969 branches/12/res/res_ari_bridges.c 402969 branches/12/res/ari/resource_bridges.c 402969 branches/12/include/asterisk/stasis_app_impl.h 402969 branches/12/include/asterisk/stasis_app.h 402969 Diff: https://reviewboard.asterisk.org/r/2947/diff/ Testing --- Ran a manual test and also wrote a testsuite test: https://reviewboard.asterisk.org/r/2951/ 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
---
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
Re: [asterisk-dev] [Code Review] 3023: Add MixMonitor() option to specify channel variable into which to store the recording filename
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/#review10235 --- Ship it! Ship It! - Joshua Colp On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This change introduces an 'f' option for the MixMonitor() application. The argument for this option specifies a channel variable into which the application should store the filename (as in, the full path) instead of the standard MIXMONITOR_FILENAME. The rationale for this option is that MixMonitor is structured in such a way that a single channel may have multiple simultaneous MixMonitors running at any given time. Thus, the MixMonitor application should not have any assumptions that the channel has only a single MixMonitor being run on it. Setting a single MIXMONITOR_FILENAME channel variable violates such assumptions by overwriting the value set from previous invocations of MixMonitor. By using the 'f' and 'i' options for MixMonitor, a user can easily manage multiple recordings on a single channel. Diffs - /trunk/apps/app_mixmonitor.c 402883 Diff: https://reviewboard.asterisk.org/r/3023/diff/ Testing --- See https://reviewboard.asterisk.org/r/3024/ Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/#review10260
---
branches/12/res/res_stasis_device_state.c
https://reviewboard.asterisk.org/r/3025/#comment19629
This could probably stand to be a lot smaller (like 64 maybe)
branches/12/res/res_stasis_device_state.c
https://reviewboard.asterisk.org/r/3025/#comment19627
Instead of constructing a device_state_subscription and searching by
OBJ_SEARCH_OBJECT, just use the name that was passed in and search by
OBJ_SEARCH_KEY. That way, you save the overhead of the creation and destruction
of an ao2_object whenever this function is called.
branches/12/res/res_stasis_device_state.c
https://reviewboard.asterisk.org/r/3025/#comment19628
ao2_unlink_flags() ignores any OBJ_SEARCH_* flags that are passed into it.
You can just use ao2_unlink() here instead.
branches/12/res/res_stasis_device_state.c
https://reviewboard.asterisk.org/r/3025/#comment19630
You could save some cycles here by just calling
ao2_find(device_state_subscriptions, name, OBJ_SEARCH_KEY | OBJ_UNLINK |
OBJ_NODATA);
- Mark Michelson
On Nov. 21, 2013, 7: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, 7: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
Re: [asterisk-dev] [Code Review] 3024: Testsuite: Test the MixMonitor 'f' option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3024/#review10236 --- I don't see where the user event is actually checked... shouldn't you have specified the requirement of a Yay event in your test-config.yaml? - Joshua Colp On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3024/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This adds a simple test that ensures the 'f' option to MixMonitor performs as documented. Diffs - /asterisk/trunk/tests/apps/tests.yaml 4331 /asterisk/trunk/tests/apps/mixmonitor_f_option/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/mixmonitor_f_option/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3024/diff/ Testing --- Test consistently passes. Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3003: ARI: Add the ability to snoop (spy/whisper) on channels
On Nov. 21, 2013, 8:05 p.m., Matt Jordan wrote: /branches/12/res/res_stasis_snoop.c, lines 215-228 https://reviewboard.asterisk.org/r/3003/diff/3/?file=48153#file48153line215 Going out on a limb here, but if this ever occurs, something has gone horribly wrong. We should never masquerade a Snoop channel: that means someone has performed an ast_channel_yank and is now doing something godawful. It may be worthwhile having an assert in here, or something that indicates that this should not have occurred - even if we do try to 'handle it gracefully'. It's perfectly legal for this to happen, an example would be redirecting the Snoop channel to the dialplan. It will continue to work and nothing funky will happen. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3003/#review10253 --- On Nov. 12, 2013, 6:43 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3003/ --- (Updated Nov. 12, 2013, 6:43 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22780 https://issues.asterisk.org/jira/browse/ASTERISK-22780 Repository: Asterisk Description --- This patch adds a snoop operation to channels which allows spying and whispering to occur. The operation creates a Snoop channel, sends it into a provided Stasis application, and returns a snapshot of it. Once in the Stasis application any normal operation can be done with it (such as recording or placing into a bridge). Diffs - /branches/12/rest-api/api-docs/channels.json 402707 /branches/12/res/res_stasis_snoop.exports.in PRE-CREATION /branches/12/res/res_stasis_snoop.c PRE-CREATION /branches/12/res/res_ari_channels.c 402707 /branches/12/res/ari/resource_channels.c 402707 /branches/12/res/ari/resource_channels.h 402707 /branches/12/main/audiohook.c 402707 /branches/12/include/asterisk/stasis_app_snoop.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3003/diff/ Testing --- Used my softphone to call in and then swagger-ui to create snoop channels to do various things. Testsuite test coming soon! Thanks, Joshua Colp -- _ -- 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
Re: [asterisk-dev] [Code Review] 3003: ARI: Add the ability to snoop (spy/whisper) on channels
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3003/#review10253 --- /branches/12/res/res_stasis_snoop.c https://reviewboard.asterisk.org/r/3003/#comment19601 I'm not sure this needs to be dynamically expanded. Unique ids of channels have a well defined upper bound, AST_MAX_UNIQUEID defined in channel.h. You could just as well make this a char array and save yourself the trouble. /branches/12/res/res_stasis_snoop.c https://reviewboard.asterisk.org/r/3003/#comment19602 Going out on a limb here, but if this ever occurs, something has gone horribly wrong. We should never masquerade a Snoop channel: that means someone has performed an ast_channel_yank and is now doing something godawful. It may be worthwhile having an assert in here, or something that indicates that this should not have occurred - even if we do try to 'handle it gracefully'. - Matt Jordan On Nov. 12, 2013, 6:43 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3003/ --- (Updated Nov. 12, 2013, 6:43 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22780 https://issues.asterisk.org/jira/browse/ASTERISK-22780 Repository: Asterisk Description --- This patch adds a snoop operation to channels which allows spying and whispering to occur. The operation creates a Snoop channel, sends it into a provided Stasis application, and returns a snapshot of it. Once in the Stasis application any normal operation can be done with it (such as recording or placing into a bridge). Diffs - /branches/12/rest-api/api-docs/channels.json 402707 /branches/12/res/res_stasis_snoop.exports.in PRE-CREATION /branches/12/res/res_stasis_snoop.c PRE-CREATION /branches/12/res/res_ari_channels.c 402707 /branches/12/res/ari/resource_channels.c 402707 /branches/12/res/ari/resource_channels.h 402707 /branches/12/main/audiohook.c 402707 /branches/12/include/asterisk/stasis_app_snoop.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3003/diff/ Testing --- Used my softphone to call in and then swagger-ui to create snoop channels to do various things. Testsuite test coming soon! Thanks, Joshua Colp -- _ -- 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
On Nov. 21, 2013, 12:59 a.m., Paul Belanger wrote:
Thanks for this, I plan to spend some time over the weekend trying this
out. My comments are mostly from the ARIs POV, specifically what is a
device in ARI? and is that the same as an endpoint?
I'd akin this more to a virtual generic entity/device. It's not a real
device, and it's certainly not an endpoint you can dial.
- Joshua
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/#review10226
---
On Nov. 20, 2013, 10:25 p.m., Kevin Harwell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/
---
(Updated Nov. 20, 2013, 10:25 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 402915
branches/12/rest-api/api-docs/events.json 402915
branches/12/rest-api/api-docs/device_states.json PRE-CREATION
branches/12/rest-api/api-docs/applications.json 402915
branches/12/res/stasis/app.c 402915
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 402915
branches/12/res/res_ari_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.h PRE-CREATION
branches/12/res/ari/resource_applications.h 402915
branches/12/res/ari/ari_model_validators.c 402915
branches/12/res/ari/ari_model_validators.h 402915
branches/12/res/ari.make 402915
branches/12/main/devicestate.c 402915
branches/12/include/asterisk/stasis_app_device_state.h PRE-CREATION
branches/12/include/asterisk/stasis_app.h 402915
branches/12/include/asterisk/devicestate.h 402915
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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
On Nov. 21, 2013, 6:04 p.m., Mark Michelson wrote:
branches/12/res/res_stasis_device_state.c, lines 142-149
https://reviewboard.asterisk.org/r/3025/diff/1/?file=48549#file48549line142
Instead of constructing a device_state_subscription and searching by
OBJ_SEARCH_OBJECT, just use the name that was passed in and search by
OBJ_SEARCH_KEY. That way, you save the overhead of the creation and
destruction of an ao2_object whenever this function is called.
I need to search by the app name and device name in order to find the device
subscribed to and OBJ_SEARCH_KEY only allows me to search using a single string
field, so I create the object and search on that instead.
On Nov. 21, 2013, 6:04 p.m., Mark Michelson wrote:
branches/12/res/res_stasis_device_state.c, lines 329-331
https://reviewboard.asterisk.org/r/3025/diff/1/?file=48549#file48549line329
You could save some cycles here by just calling
ao2_find(device_state_subscriptions, name, OBJ_SEARCH_KEY | OBJ_UNLINK
| OBJ_NODATA);
Similar to above. The data stored in device_state_subscriptions is essentially
keyed off of two values instead of one: app_name and device_name. This is done
because the list could contain subscriptions with multiple app and device
names. So searches/finds have to be able to pass in both keys if you will.
It first tries to match on the device name and if found then it tries to match
on the app_name. If both match the search returns the located subscription for
that app and device.
- Kevin
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/#review10260
---
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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/
---
(Updated Nov. 21, 2013, 6:25 p.m.)
Review request for Asterisk Developers, David Lee and Matt Jordan.
Changes
---
Addressed review findings.
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 (updated)
-
branches/12/rest-api/resources.json 402993
branches/12/rest-api/api-docs/events.json 402993
branches/12/rest-api/api-docs/deviceStates.json PRE-CREATION
branches/12/rest-api/api-docs/applications.json 402993
branches/12/rest-api-templates/ari.make.mustache 402993
branches/12/res/stasis/app.c 402993
branches/12/res/stasis/app.h 402993
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 402993
branches/12/res/res_ari_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.h PRE-CREATION
branches/12/res/ari/resource_applications.h 402993
branches/12/res/ari/ari_model_validators.c 402993
branches/12/res/ari/ari_model_validators.h 402993
branches/12/res/ari.make 402993
branches/12/main/devicestate.c 402993
branches/12/include/asterisk/stasis_app_device_state.h PRE-CREATION
branches/12/include/asterisk/stasis_app.h 402993
branches/12/include/asterisk/devicestate.h 402993
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
Re: [asterisk-dev] [Code Review] 2994: ari:Add application/json parameter support
On Nov. 20, 2013, 6:07 p.m., Matt Jordan wrote: /branches/12/main/http.c, line 631 https://reviewboard.asterisk.org/r/2994/diff/2/?file=48312#file48312line631 Particularly since this is coming from an external source, we should probably use sscanf. You pretty much do all of your checking for this already in ast_http_get_json, so it might be a moot point, but hey - coding guidelines :-) Matt Jordan wrote: Of course, this is auto-generated... At the time, the handler didn't have a great way of handling parse failures anyways, so atoi wasn't any different than using sscanf. Now I can handle failures, so we could exit early with a 400 if someone gives a string for a numeric input. But that's a change to the code generator, and outside of scope for this patch. - David --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2994/#review10223 --- On Nov. 15, 2013, 12:29 p.m., David Lee wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2994/ --- (Updated Nov. 15, 2013, 12:29 p.m.) Review request for Asterisk Developers and Paul Belanger. Bugs: ASTERISK-22685 https://issues.asterisk.org/jira/browse/ASTERISK-22685 Repository: Asterisk Description --- The patch allows ARI to parse request parameters from an incoming JSON request body, instead of requiring the request to come in as query parameters (which is just weird for POST and DELETE) or form parameters (which is okay, but a bit asymmetric given that all of our responses are JSON). For any operation that does _not_ have a parameter defined of type body (i.e. paramType: body in the API declaration), if a request provides a request body with a Content type of application/json, the provided JSON document is parsed and searched for parameters. The expected fields in the provided JSON document should match the query parameters defined for the operation. If the parameter has 'allowMultiple' set, then the field in the JSON document may optionally be an array of values. Diffs - /branches/12/tests/test_ari.c 402455 /branches/12/rest-api-templates/swagger_model.py 402455 /branches/12/rest-api-templates/res_ari_resource.c.mustache 402455 /branches/12/rest-api-templates/param_parsing.mustache 402455 /branches/12/rest-api-templates/asterisk_processor.py 402455 /branches/12/res/res_ari_sounds.c 402455 /branches/12/res/res_ari_recordings.c 402455 /branches/12/res/res_ari_playback.c 402455 /branches/12/res/res_ari_endpoints.c 402455 /branches/12/res/res_ari_channels.c 402455 /branches/12/res/res_ari_bridges.c 402455 /branches/12/res/res_ari_asterisk.c 402455 /branches/12/res/res_ari_applications.c 402455 /branches/12/res/res_ari.c 402455 /branches/12/main/http.c 402455 /branches/12/include/asterisk/http.h 402455 /branches/12/include/asterisk/ari.h 402455 Diff: https://reviewboard.asterisk.org/r/2994/diff/ Testing --- Testsuite test. See https://reviewboard.asterisk.org/r/2993/ Thanks, David Lee -- _ -- 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
Re: [asterisk-dev] [Code Review] 2959: pjsip: AMI commands and events
On Thu, Nov 21, 2013 at 7:25 AM, Joshua Colp jc...@digium.com wrote: George Joseph wrote: Just FYI... I've got a whole bunch of pjsip cli stuff waiting on this patch to be committed. Any chance of this happening this week? It'll get reviewed today and pending any other findings arising it'll go in shortly. If possible you could just throw your stuff up for review too and mark it as dependent on the AMI commands/events review (you can do that in Reviewboard now! yay!). Cheers, Yeah, I can attempt that this afternoon. There's no way it would compile even with 2959 applied first because of some workarounds I had to do. It would be a start though. -- _ -- 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
Re: [asterisk-dev] [Code Review] 3023: Add MixMonitor() option to specify channel variable into which to store the recording filename
On Nov. 21, 2013, 2:17 p.m., Joshua Colp wrote: The code itself looks fine, the only nagging question I have is why not just treat the variable as a temporary return variable in the case when you have multiple MixMonitor instances and use Set in the dialplan to set another variable to the value. *shrug* Two reasons: 1) MixMonitor's invocation is not limited to the dialplan. It has the potential to be started by AMI as well. Therefore, if you had an oddball situation where both a dialplan invocation and an AMI invocation occurred simultaneously, there's a race condition where they each will think that MIXMONITOR_FILENAME is their filename. 2) There is precedent in MixMonitor for such an option. The 'i' option allows for you to specify a channel variable into which the MixMonitor's unique ID will be set. Having a similar option for a filename fits the mold pretty well also. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/#review10231 --- On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This change introduces an 'f' option for the MixMonitor() application. The argument for this option specifies a channel variable into which the application should store the filename (as in, the full path) instead of the standard MIXMONITOR_FILENAME. The rationale for this option is that MixMonitor is structured in such a way that a single channel may have multiple simultaneous MixMonitors running at any given time. Thus, the MixMonitor application should not have any assumptions that the channel has only a single MixMonitor being run on it. Setting a single MIXMONITOR_FILENAME channel variable violates such assumptions by overwriting the value set from previous invocations of MixMonitor. By using the 'f' and 'i' options for MixMonitor, a user can easily manage multiple recordings on a single channel. Diffs - /trunk/apps/app_mixmonitor.c 402883 Diff: https://reviewboard.asterisk.org/r/3023/diff/ Testing --- See https://reviewboard.asterisk.org/r/3024/ Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3023: Add MixMonitor() option to specify channel variable into which to store the recording filename
On Nov. 21, 2013, 4:40 p.m., Tilghman Lesher wrote:
Shouldn't something like this be a channel function from which you retrieve
the value, instead of specifying a variable into which the name is placed?
Seems like a significant regression in spitting things out to channel
variables, instead of using dialplan functions to retrieve values when
needed.
So you're thinking something like:
exten = blah,1,Answer()
same = n,MixMonitor(file.wav,i(ID))
same = n,NoOp(Filename is MIXMONITOR(${ID},file))
?
I'd be okay with it. The main reason I went with the channel variable approach
was because of app_mixmonitor's pre-existing 'i' option. My thought was that
this feels more consistent than providing a separate method of getting an
instance-specific value.
Of course the attraction of the dialplan function route is that it is more
scalable in case there are other instance-specific values that people wish to
retrieve from a mixmonitor.
- Mark
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3023/#review10242
---
On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3023/
---
(Updated Nov. 20, 2013, 9:12 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This change introduces an 'f' option for the MixMonitor() application. The
argument for this option specifies a channel variable into which the
application should store the filename (as in, the full path) instead of the
standard MIXMONITOR_FILENAME.
The rationale for this option is that MixMonitor is structured in such a way
that a single channel may have multiple simultaneous MixMonitors running at
any given time. Thus, the MixMonitor application should not have any
assumptions that the channel has only a single MixMonitor being run on it.
Setting a single MIXMONITOR_FILENAME channel variable violates such
assumptions by overwriting the value set from previous invocations of
MixMonitor. By using the 'f' and 'i' options for MixMonitor, a user can
easily manage multiple recordings on a single channel.
Diffs
-
/trunk/apps/app_mixmonitor.c 402883
Diff: https://reviewboard.asterisk.org/r/3023/diff/
Testing
---
See https://reviewboard.asterisk.org/r/3024/
Thanks,
Mark Michelson
--
_
-- 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
Re: [asterisk-dev] [Code Review] 3003: ARI: Add the ability to snoop (spy/whisper) on channels
On Nov. 14, 2013, 4:53 p.m., Mark Michelson wrote: /branches/12/res/res_stasis_snoop.c, line 142 https://reviewboard.asterisk.org/r/3003/diff/3/?file=48153#file48153line142 Hm, would it be more useful to publish the snoop messages on the spyee channel's topic? I would suspect that most people would think of the spyee's channel as the real channel and would expect to see the publications on that channel instead of the snoop channel. Joshua Colp wrote: It's possible when spying is started to do that but doing it at the end and guaranteeing you get it would require holding a reference to the channel, which I avoided at all costs. As long as we're fine with that... then sure. I'd prefer not, actually. Usually, the channel that initiates the Spying is the one whose topic has the messages published on it. Granted, in this case, the Snoop channel is a little odd, but it is still the channel doing the Spying, not the target of the Spying operation. Switching the order of the channels here takes it out of alignment with the messages published in ChanSpy. From a consumer perspective, most things that receive ChanSpy messages have also already subscribed to the ast_channel_topic_all topic, which means the individual channel topics are kind of a moot point. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3003/#review10183 --- On Nov. 12, 2013, 6:43 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3003/ --- (Updated Nov. 12, 2013, 6:43 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22780 https://issues.asterisk.org/jira/browse/ASTERISK-22780 Repository: Asterisk Description --- This patch adds a snoop operation to channels which allows spying and whispering to occur. The operation creates a Snoop channel, sends it into a provided Stasis application, and returns a snapshot of it. Once in the Stasis application any normal operation can be done with it (such as recording or placing into a bridge). Diffs - /branches/12/rest-api/api-docs/channels.json 402707 /branches/12/res/res_stasis_snoop.exports.in PRE-CREATION /branches/12/res/res_stasis_snoop.c PRE-CREATION /branches/12/res/res_ari_channels.c 402707 /branches/12/res/ari/resource_channels.c 402707 /branches/12/res/ari/resource_channels.h 402707 /branches/12/main/audiohook.c 402707 /branches/12/include/asterisk/stasis_app_snoop.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3003/diff/ Testing --- Used my softphone to call in and then swagger-ui to create snoop channels to do various things. Testsuite test coming soon! Thanks, Joshua Colp -- _ -- 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
Re: [asterisk-dev] [Code Review] 3017: PickupChan: Add ability to specify channel uniqueids as well as channel names.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3017/#review10233 --- Ship it! - Joshua Colp On Nov. 18, 2013, 9:25 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3017/ --- (Updated Nov. 18, 2013, 9:25 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- * Made PickupChan() search by channel uniqueids if the search could not find a channel by name. * Ensured PickupChan() never considers the picking channel for pickup. * Made PickupChan() option p use a common search by name routine. The original search was erroneously case sensitive. Diffs - /trunk/apps/app_directed_pickup.c 402883 /trunk/CHANGES 402883 Diff: https://reviewboard.asterisk.org/r/3017/diff/ Testing --- Picked up channels by name and uniqueid. Thanks, rmudgett -- _ -- 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
Re: [asterisk-dev] [Code Review] 3023: Add MixMonitor() option to specify channel variable into which to store the recording filename
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/#review10231 --- The code itself looks fine, the only nagging question I have is why not just treat the variable as a temporary return variable in the case when you have multiple MixMonitor instances and use Set in the dialplan to set another variable to the value. *shrug* - Joshua Colp On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3023/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This change introduces an 'f' option for the MixMonitor() application. The argument for this option specifies a channel variable into which the application should store the filename (as in, the full path) instead of the standard MIXMONITOR_FILENAME. The rationale for this option is that MixMonitor is structured in such a way that a single channel may have multiple simultaneous MixMonitors running at any given time. Thus, the MixMonitor application should not have any assumptions that the channel has only a single MixMonitor being run on it. Setting a single MIXMONITOR_FILENAME channel variable violates such assumptions by overwriting the value set from previous invocations of MixMonitor. By using the 'f' and 'i' options for MixMonitor, a user can easily manage multiple recordings on a single channel. Diffs - /trunk/apps/app_mixmonitor.c 402883 Diff: https://reviewboard.asterisk.org/r/3023/diff/ Testing --- See https://reviewboard.asterisk.org/r/3024/ Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3018: testsuite: Add tests for Say application jump behavior with SAY_DTMF_INTERRUPT enabled and disabled
On Nov. 21, 2013, 6:09 p.m., opticron wrote: /asterisk/trunk/tests/apps/say_interrupt/configs/ast1/sip.conf, lines 1-12 https://reviewboard.asterisk.org/r/3018/diff/1/?file=48290#file48290line1 This entire file seems irrelevant to the the test being added and directs calls to a context that does not exist. Confirmed. File removed. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3018/#review10245 --- On Nov. 15, 2013, 12:07 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3018/ --- (Updated Nov. 15, 2013, 12:07 a.m.) Review request for Asterisk Developers, Mark Michelson and rmudgett. Repository: testsuite Description --- https://reviewboard.asterisk.org/r/3011/ Tests the functionality of this feature, both when set and not set Diffs - /asterisk/trunk/tests/apps/tests.yaml 4343 /asterisk/trunk/tests/apps/say_interrupt/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/say_interrupt/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/say_interrupt/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3018/diff/ Testing --- It's a test. It runs and completes successfully on my box with all the expected extensions being hit according to the logs. I changed some things around and was easily able to make it fail in ways that I expected it to fail, which is good. Changing the SAY_DTMF_INTERRUPT value in either test from what it currently is causes failures, which is also good. All DTMF values in the test must be issued for each test in order for the test to be considered successful. Thanks, Jonathan Rose -- _ -- 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
Re: [asterisk-dev] [Code Review] 3019: ari: Add silence generator controls
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3019/ --- (Updated Nov. 21, 2013, 9:55 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and Joshua Colp. Changes --- Committed in revision 402926 Bugs: ASTERISK-22514 https://issues.asterisk.org/jira/browse/ASTERISK-22514 Repository: Asterisk Description --- This patch adds the ability to start a silence generator on a channel via ARI. This generator will play silence on the channel (avoiding audio timeouts on the peer) until it is stopped, or some other media operation is started (like playing media, starting music on hold, etc.). Diffs - /branches/12/rest-api/api-docs/channels.json 402842 /branches/12/res/stasis/control.c 402842 /branches/12/res/res_ari_channels.c 402842 /branches/12/res/ari/resource_channels.c 402842 /branches/12/res/ari/resource_channels.h 402842 /branches/12/include/asterisk/stasis_app.h 402842 Diff: https://reviewboard.asterisk.org/r/3019/diff/ Testing --- Manual testing. Thanks, David Lee -- _ -- 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
Re: [asterisk-dev] [Code Review] 3020: Test for consistent from tag on outbound REGISTER
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3020/#review10244 --- Ship it! Extraneous white space and a suggestion. Nice work! /asterisk/trunk/tests/channels/SIP/outbound_register_from/sipp/register.xml https://reviewboard.asterisk.org/r/3020/#comment19582 Red blob /asterisk/trunk/tests/channels/SIP/outbound_register_from/test-config.yaml https://reviewboard.asterisk.org/r/3020/#comment19581 Not that we use it terribly often, but the YAML actually 'supports' a dedicated tag for issues: testinfo: # If true, skip execution of this test skip : 'Brief reason for skipping test' # OPTIONAL # A summary of what the test does summary: 'Put a short one liner summary of the test here' # A detailed description of what functionality is covered by the test description: | 'Put a more verbose description of the test here. This may span multiple lines.' # A sequence of key/value pairs specifying issues in an issue tracker related to this test issues: # OPTIONAL # List of issue numbers associated with this test - jira : 'ASTERISK-12345' - jira : 'ASTERISK-10101' It may be nice to put that in here, although it isn't necessary. /asterisk/trunk/tests/channels/SIP/outbound_reregister_from/sipp/register.xml https://reviewboard.asterisk.org/r/3020/#comment19584 And here /asterisk/trunk/tests/channels/SIP/outbound_reregister_from/sipp/register.xml https://reviewboard.asterisk.org/r/3020/#comment19583 Here too - Matt Jordan On Nov. 15, 2013, 9:42 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3020/ --- (Updated Nov. 15, 2013, 9:42 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-12117 https://issues.asterisk.org/jira/browse/ASTERISK-12117 Repository: testsuite Description --- This pair of tests were rewritten (from https://reviewboard.asterisk.org/r/2985/) to use SIPp instead of pcap which requires running as root. The From header is examined by SIPp (acting as registrar) to insure that it is consistent for the duration of the test (reactor-timeout). If it changes, SIPp aborts early to indicate a failure. Diffs - /asterisk/trunk/tests/channels/SIP/tests.yaml 4344 /asterisk/trunk/tests/channels/SIP/outbound_reregister_from/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/SIP/outbound_reregister_from/sipp/register.xml PRE-CREATION /asterisk/trunk/tests/channels/SIP/outbound_reregister_from/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/channels/SIP/outbound_register_from/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/SIP/outbound_register_from/sipp/register.xml PRE-CREATION /asterisk/trunk/tests/channels/SIP/outbound_register_from/configs/ast1/sip.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3020/diff/ Testing --- Ran the tests with 1.8 prior to r402604 (patch to 12117) and after. Test properly indicates failure with unpatched code. Thanks, Scott Griepentrog -- _ -- 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
Re: [asterisk-dev] [Code Review] 2947: ARI: Adding a channel to a bridge while a live recording is active blocks
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2947/ --- (Updated Nov. 21, 2013, 2:24 p.m.) Review request for Asterisk Developers. Changes --- Changed function to return enumeration. Bugs: ASTERISK-22624 https://issues.asterisk.org/jira/browse/ASTERISK-22624 Repository: Asterisk Description --- Added a channel recording flag that is checked before adding a channel to a bridge. If the the channel is currently being recorded a 409 Conflict is returned and the channel is not added to the bridge. Diffs (updated) - branches/12/rest-api/api-docs/bridges.json 402969 branches/12/res/stasis/control.c 402969 branches/12/res/stasis/command.c 402969 branches/12/res/stasis/command.h 402969 branches/12/res/res_stasis_recording.c 402969 branches/12/res/res_stasis_playback.c 402969 branches/12/res/res_stasis_answer.c 402969 branches/12/res/res_ari_bridges.c 402969 branches/12/res/ari/resource_bridges.c 402969 branches/12/include/asterisk/stasis_app_impl.h 402969 branches/12/include/asterisk/stasis_app.h 402969 Diff: https://reviewboard.asterisk.org/r/2947/diff/ Testing --- Ran a manual test and also wrote a testsuite test: https://reviewboard.asterisk.org/r/2951/ 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
On Nov. 22, 2013, 12:04 a.m., Mark Michelson wrote:
branches/12/res/res_stasis_device_state.c, lines 142-149
https://reviewboard.asterisk.org/r/3025/diff/1/?file=48549#file48549line142
Instead of constructing a device_state_subscription and searching by
OBJ_SEARCH_OBJECT, just use the name that was passed in and search by
OBJ_SEARCH_KEY. That way, you save the overhead of the creation and
destruction of an ao2_object whenever this function is called.
Kevin Harwell wrote:
I need to search by the app name and device name in order to find the
device subscribed to and OBJ_SEARCH_KEY only allows me to search using a
single string field, so I create the object and search on that instead.
This is not true. The OBJ_SEARCH_KEY could be anything you setup the container
callbacks to take as the key. The template code on the wiki assumes the key is
a simple string.
In this case you could pass a struct pointer that contains two string pointers.
struct device_container_key {
const char *app_name;
const char *device_name;
}
- rmudgett
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/#review10260
---
On Nov. 22, 2013, 12:25 a.m., Kevin Harwell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3025/
---
(Updated Nov. 22, 2013, 12:25 a.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 402993
branches/12/rest-api/api-docs/events.json 402993
branches/12/rest-api/api-docs/deviceStates.json PRE-CREATION
branches/12/rest-api/api-docs/applications.json 402993
branches/12/rest-api-templates/ari.make.mustache 402993
branches/12/res/stasis/app.c 402993
branches/12/res/stasis/app.h 402993
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 402993
branches/12/res/res_ari_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.c PRE-CREATION
branches/12/res/ari/resource_device_states.h PRE-CREATION
branches/12/res/ari/resource_applications.h 402993
branches/12/res/ari/ari_model_validators.c 402993
branches/12/res/ari/ari_model_validators.h 402993
branches/12/res/ari.make 402993
branches/12/main/devicestate.c 402993
branches/12/include/asterisk/stasis_app_device_state.h PRE-CREATION
branches/12/include/asterisk/stasis_app.h 402993
branches/12/include/asterisk/devicestate.h 402993
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
Re: [asterisk-dev] [Code Review] 3015: Testsuite: CONFBRIDGE_RESULT test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3015/#review10243 --- Ship it! Ship It! /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/extensions.conf https://reviewboard.asterisk.org/r/3015/#comment19580 I'm picky but to a casual reader of this perhaps something descriptive instead of sausage would be nice? - Joshua Colp On Nov. 21, 2013, 3:40 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3015/ --- (Updated Nov. 21, 2013, 3:40 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This creates a test scenario that ensures that the five different CONFBRIDGE_RESULT values are set correctly upon exit. The test sets up a conference with five participants: Alice, Bob, Carol, Darnell, and Egbert. Alice, Bob, and Carol enter initially. Alice has the end_marked option set on her user profile. Bob attempts to use a nonexistent user profile sausage to enter the conference, and Carol has access to a basic user DTMF menu. When Carol enters, this triggers Darnell to enter the conference. Darnell has access to a basic admin menu and is a marked user. It also triggers Carol to enter a DTMF sequence to leave the conference. When Darnell enters, this triggers Egbert to enter the conference. When Egbert enters, this triggers Darnell to press a DTMF sequence to kick Egbert out. When Egbert exits the conference, this triggers Darnell to hang up. Since Darnell is the final marked user in the conference, this should result in Alice being ejected from the conference. Under this scenario, the CONFBRIDGE_RESULT variables should be as follows: Alice: ENDMARKED Bob: ERROR Carol: DTMF Darnell: HANGUP Egbert: KICKED Diffs - /asterisk/trunk/tests/apps/confbridge/tests.yaml 4331 /asterisk/trunk/tests/apps/confbridge/confbridge_result/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/manager.general.conf.inc PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/confbridge.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/apptest.py 4331 Diff: https://reviewboard.asterisk.org/r/3015/diff/ Testing --- Ran the test in a while loop and ensured that in 30+ executions, the test passed each time. Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 2996: allow uncached realtime sip peers to be queue members
On Nov. 7, 2013, 3:37 p.m., Joshua Colp wrote:
To make sure I better understand the situation...
Is it caching an old stale entry without address?
Then when device state is queried it uses that old stale entry instead of
requerying realtime... so it thinks it has no address when in realtime
there really is an address?
Joshua Colp wrote:
Well, no, cause your title says uncached... I'm more confused about how
it's returning a peer because devicestate purposely doesn't check realtime.
wdoekes wrote:
Thanks for the review. We are indeed talking about uncached:
rtcachefriends=no
The relevant code:
sip_devicestate(...)
sip_find_peer(host, NULL, realtime=FALSE, FINDALLDEVICES,
devstate_only=TRUE, 0)
if (!p (realtime || devstate_only)) {
p = realtime_peer(peer, addr, devstate_only, which_objects);
realtime_peer(...)
if (ast_test_flag(global_flags[1], SIP_PAGE2_RTCACHEFRIENDS)
!devstate_only) {
...
ao2_t_link(peers, peer, link peer into peers table);
So, the devstate_only=TRUE initiates a realtime lookup anyway, but
ensures that the found peer isn't linked in the peers table (if
rtcachefriends were to be true).
The comments in sip_devicestate could probably use some clarification.
As for your question:
Is it caching an old stale entry without address?
No, it's returning the freshly loaded peer, but it will never have an
p-addr (unless it is currently in a call); the proxy takes care of that
stuff. So I need the devicestate to return what it knows, which is *nothing*
(unknown), not *unavailable*.
Joshua Colp wrote:
Proxy? An outbound proxy?
wdoekes wrote:
Indeed. A dialog-based one. The peer doesn't know it has one until I hit
Dial.
Yeah... that is the underlying problem here, which is why I'm uncomfortable
with the change. Every other realtime user who isn't doing what you are doing
loses out on knowing if the peer is available or not in a more granular
fashion, since in their case the check is valid.
- Joshua
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2996/#review10134
---
On Nov. 5, 2013, 2:41 p.m., wdoekes wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2996/
---
(Updated Nov. 5, 2013, 2:41 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
When using realtime SIP peers as queue members, the device state as returned
by sip_devicestate() is AST_DEVICE_UNAVAILABLE when the peer is not loaded
into memory.
For uncached realtime SIP peers, this is practically always. That means that
my queue doesn't have any available members.
This patch changes all p-realtime peers with a null p-addr to return
AST_DEVICE_UNKNOWN instead. If we set the queue to accept unknown members,
the queue correctly copes with devices with this UNKNOWN state:
static int member_status_available(int status)
{
return status == AST_DEVICE_NOT_INUSE || status == AST_DEVICE_UNKNOWN;
}
A different approach would have been to set the p-defaddr to non-null, but
that would put the device in NOT_INUSE state which is not entirely correct,
since I don't know if the device *is* available. And I haven't checked what
other side-effects setting the p-defaddr has.
Diffs
-
/branches/1.8/channels/chan_sip.c 402467
Diff: https://reviewboard.asterisk.org/r/2996/diff/
Testing
---
Tested on asterisk 10 and 11 with the SIP-devices in the state_interface
column. Without the patch, the device state is unavailable, and the queue
will report the members as unavailable. With the patch, the members have the
unknown state and things work properly.
Thanks,
wdoekes
--
_
-- 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
Re: [asterisk-dev] [Code Review] 3023: Add MixMonitor() option to specify channel variable into which to store the recording filename
On Nov. 21, 2013, 4:40 p.m., Tilghman Lesher wrote:
Shouldn't something like this be a channel function from which you retrieve
the value, instead of specifying a variable into which the name is placed?
Seems like a significant regression in spitting things out to channel
variables, instead of using dialplan functions to retrieve values when
needed.
Mark Michelson wrote:
So you're thinking something like:
exten = blah,1,Answer()
same = n,MixMonitor(file.wav,i(ID))
same = n,NoOp(Filename is MIXMONITOR(${ID},file))
?
I'd be okay with it. The main reason I went with the channel variable
approach was because of app_mixmonitor's pre-existing 'i' option. My thought
was that this feels more consistent than providing a separate method of
getting an instance-specific value.
Of course the attraction of the dialplan function route is that it is
more scalable in case there are other instance-specific values that people
wish to retrieve from a mixmonitor.
${MIXMONITOR(${ID},file)} but sure.
- Tilghman
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3023/#review10242
---
On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3023/
---
(Updated Nov. 20, 2013, 9:12 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This change introduces an 'f' option for the MixMonitor() application. The
argument for this option specifies a channel variable into which the
application should store the filename (as in, the full path) instead of the
standard MIXMONITOR_FILENAME.
The rationale for this option is that MixMonitor is structured in such a way
that a single channel may have multiple simultaneous MixMonitors running at
any given time. Thus, the MixMonitor application should not have any
assumptions that the channel has only a single MixMonitor being run on it.
Setting a single MIXMONITOR_FILENAME channel variable violates such
assumptions by overwriting the value set from previous invocations of
MixMonitor. By using the 'f' and 'i' options for MixMonitor, a user can
easily manage multiple recordings on a single channel.
Diffs
-
/trunk/apps/app_mixmonitor.c 402883
Diff: https://reviewboard.asterisk.org/r/3023/diff/
Testing
---
See https://reviewboard.asterisk.org/r/3024/
Thanks,
Mark Michelson
--
_
-- 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
Re: [asterisk-dev] [Code Review] 3018: testsuite: Add tests for Say application jump behavior with SAY_DTMF_INTERRUPT enabled and disabled
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3018/#review10245 --- /asterisk/trunk/tests/apps/say_interrupt/configs/ast1/sip.conf https://reviewboard.asterisk.org/r/3018/#comment19585 This entire file seems irrelevant to the the test being added and directs calls to a context that does not exist. - opticron On Nov. 14, 2013, 6:07 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3018/ --- (Updated Nov. 14, 2013, 6:07 p.m.) Review request for Asterisk Developers, Mark Michelson and rmudgett. Repository: testsuite Description --- https://reviewboard.asterisk.org/r/3011/ Tests the functionality of this feature, both when set and not set Diffs - /asterisk/trunk/tests/apps/tests.yaml 4343 /asterisk/trunk/tests/apps/say_interrupt/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/say_interrupt/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/say_interrupt/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3018/diff/ Testing --- It's a test. It runs and completes successfully on my box with all the expected extensions being hit according to the logs. I changed some things around and was easily able to make it fail in ways that I expected it to fail, which is good. Changing the SAY_DTMF_INTERRUPT value in either test from what it currently is causes failures, which is also good. All DTMF values in the test must be issued for each test in order for the test to be considered successful. Thanks, Jonathan Rose -- _ -- 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
Re: [asterisk-dev] [Code Review] 3024: Testsuite: Test the MixMonitor 'f' option
On Nov. 21, 2013, 3:38 p.m., Joshua Colp wrote: I don't see where the user event is actually checked... shouldn't you have specified the requirement of a Yay event in your test-config.yaml? Nope, SimpleTestCase determines pass/fail based solely on the number of UserEvents that are received. So in my test, I send a UserEvent if the value is as expected; otherwise I NoOp. It doesn't matter what the content of the UserEvent is for SimpleTestCase, just that it exists. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3024/#review10236 --- On Nov. 20, 2013, 9:12 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3024/ --- (Updated Nov. 20, 2013, 9:12 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This adds a simple test that ensures the 'f' option to MixMonitor performs as documented. Diffs - /asterisk/trunk/tests/apps/tests.yaml 4331 /asterisk/trunk/tests/apps/mixmonitor_f_option/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/mixmonitor_f_option/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3024/diff/ Testing --- Test consistently passes. Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3015: Testsuite: CONFBRIDGE_RESULT test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3015/ --- (Updated Nov. 21, 2013, 3:40 p.m.) Review request for Asterisk Developers. Changes --- Get full review up. Repository: testsuite Description --- This creates a test scenario that ensures that the five different CONFBRIDGE_RESULT values are set correctly upon exit. The test sets up a conference with five participants: Alice, Bob, Carol, Darnell, and Egbert. Alice, Bob, and Carol enter initially. Alice has the end_marked option set on her user profile. Bob attempts to use a nonexistent user profile sausage to enter the conference, and Carol has access to a basic user DTMF menu. When Carol enters, this triggers Darnell to enter the conference. Darnell has access to a basic admin menu and is a marked user. It also triggers Carol to enter a DTMF sequence to leave the conference. When Darnell enters, this triggers Egbert to enter the conference. When Egbert enters, this triggers Darnell to press a DTMF sequence to kick Egbert out. When Egbert exits the conference, this triggers Darnell to hang up. Since Darnell is the final marked user in the conference, this should result in Alice being ejected from the conference. Under this scenario, the CONFBRIDGE_RESULT variables should be as follows: Alice: ENDMARKED Bob: ERROR Carol: DTMF Darnell: HANGUP Egbert: KICKED Diffs (updated) - /asterisk/trunk/tests/apps/confbridge/tests.yaml 4331 /asterisk/trunk/tests/apps/confbridge/confbridge_result/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/manager.general.conf.inc PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_result/configs/ast1/confbridge.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/apptest.py 4331 Diff: https://reviewboard.asterisk.org/r/3015/diff/ Testing --- Ran the test in a while loop and ensured that in 30+ executions, the test passed each time. Thanks, Mark Michelson -- _ -- 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
Re: [asterisk-dev] [Code Review] 3025: ARI: Implement device state API
---
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.
Changes
---
Addressed various review findings. Also, Added a listing command: GET
/deviceStates
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 (updated)
-
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
Re: [asterisk-dev] [Code Review] 2947: ARI: Adding a channel to a bridge while a live recording is active blocks
On Nov. 21, 2013, 12:53 p.m., opticron wrote: branches/12/include/asterisk/stasis_app.h, line 180 https://reviewboard.asterisk.org/r/2947/diff/5/?file=48327#file48327line180 This should return the enum instead of int. The problem with making this an enum is currently it is assumed different enum types could be returned. So instead of having a single master enum for all result types, resources could define and use their expected result types specific to their modules. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2947/#review10246 --- On Nov. 15, 2013, 1:41 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2947/ --- (Updated Nov. 15, 2013, 1:41 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22624 https://issues.asterisk.org/jira/browse/ASTERISK-22624 Repository: Asterisk Description --- Added a channel recording flag that is checked before adding a channel to a bridge. If the the channel is currently being recorded a 409 Conflict is returned and the channel is not added to the bridge. Diffs - branches/12/rest-api/api-docs/bridges.json 402853 branches/12/res/stasis/control.c 402853 branches/12/res/stasis/command.c 402853 branches/12/res/stasis/command.h 402853 branches/12/res/res_stasis_recording.c 402853 branches/12/res/res_stasis_playback.c 402853 branches/12/res/res_stasis_answer.c 402853 branches/12/res/res_ari_bridges.c 402853 branches/12/res/ari/resource_bridges.c 402853 branches/12/include/asterisk/stasis_app_impl.h 402853 branches/12/include/asterisk/stasis_app.h 402853 Diff: https://reviewboard.asterisk.org/r/2947/diff/ Testing --- Ran a manual test and also wrote a testsuite test: https://reviewboard.asterisk.org/r/2951/ 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
