Re: [asterisk-dev] [Code Review] 3811: Move main/manager_*.c to loadable modules
On July 17, 2014, 5:41 p.m., Matt Jordan wrote: /trunk/main/stasis_channels.c, lines 1071-1077 https://reviewboard.asterisk.org/r/3811/diff/1/?file=64565#file64565line1071 I'm not sure why these changes (removal of the .to_ami callback) were necessary. Generally, I prefer the .to_ami callbacks to explicit subscription to message types and construction of messages in the various manager_* modules: (1) Obtaining the messages in the appropriate modules is done by simply forwarding the topics to the manager topic. That substantially reduces the boilerplate code required. (2) Co-locating the generation of formatting of messages makes it very easy to update all consumers of a message when a new field is added, helping keep the code/events similar for all consumers of that message. Generally, I would much prefer these to be kept, and to have the other channel related message have .to_ami callbacks implemented. If anything, the res_manager_channels module should be very small: it should set up a forwarding relationship between the channel topics and the manager topic and be done. Corey Farrell wrote: (1) I couldn't determine what causes the current code (stasis_channels.c / manager_channels.c) to have manager subscribe these events (or not). Maybe I was wrong to assume the existence of .to_ami causes the messages to be broadcast to AMI? If I am wrong then what causes ast_channel_varset_type to be subscribed by the manager topic? (2) As for reducing boilerplate code, I don't understand how this is the case - the new code for these events are almost the same as the old code. Yes (3) The primary goal of this change is to allow res_manager_channels to be excluded from a build, and replaced with something that produces selected events with a different/reduced format, or use other custom filters. I view AMI on two levels - a transport protocol (name value pairs resembling HTTP headers), and an application protocol (the default events produced by Asterisk). Removal of .to_ami isolates the application layer to modules so it can be replaced. For example ast_manager_build_channel_state_string provides 1 field that is useful to me - UniqueID. All other fields are clock cycles wasted during production, transmit and consumption. (4) After this change .to_ami would be dead-code at best when res_manager_channels.so is not loaded. At worst I'm concerned that .to_ami might prevent me from producing custom events. Mark Michelson wrote: I can answer your question from (1). In main/manager_channels.c, there is the following code: manager_topic = ast_manager_get_topic(); if (!manager_topic) { return -1; } /* lines snipped */ channel_topic = ast_channel_topic_all_cached(); if (!channel_topic) { return -1; } topic_forwarder = stasis_forward_all(channel_topic, manager_topic); if (!topic_forwarder) { return -1; } What's happening here is that messages on any channel topic get forwarded to the manager as a result of the topic forwarder. Since varset messages are published on a specific channel topic, they get forwarded to the manager, which calls the to_ami vtable callback to format the message and then send it out. Even without a .to_ami callback, the stasis publication is still forwarded to the manager topic. The manager topic still gets the message, sees that there is no to_ami callback and does nothing with the forwarded message. The current use of the topic forwarder in main/manager_channels.c means that the easiest way I know of to do what you're after (with varset or any other channel topic publication) is exactly what you've done in this review: withhold a to_ami callback and create a subscription to the specific event type in a loadable module. While this is easier to implement, it results in some extra cycles wasted on forwarding to the manager topic when it really doesn't need to be done at all. The other, more complicated option would be to define the varset message type in a loadable module. You'd need to create a public function in that module that is used to publish the message since core modules would not be able to reliably reference the message type defined in the module. With the OPTIONAL_API, you can make it so that attempting to call the publication function provided by the module when the module is not loaded will result in a no-op. This would make it so the varset message type would only exist if the appropriate module were loaded. Therefore, attempting to publish a varset message would be a no-op if the module were not loaded. It also means, though, that if you are attempting to create your own
[asterisk-dev] [Code Review] 3867: [chan_sip] Default DTLS settings to use if peer misses own settings
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3867/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24128 https://issues.asterisk.org/jira/browse/ASTERISK-24128 Repository: Asterisk Description --- Load default dtls settings from [general settings] If peer has dtls enabled but misses some of the settings and they are set in default settings it would load them from there. It would be logical as most of sip settings work like that and as well as there is no way to use template in realtime and it would lead to copy paste same settings for every peer. Diffs - trunk/channels/chan_sip.c 419804 Diff: https://reviewboard.asterisk.org/r/3867/diff/ Testing --- Test on development server (ast 11.11.0) Thanks, Michael K. -- _ -- 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] 3603: func_jitterbuffer: fix errors and leaks caused by certain masquerade's
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3603/#review12921 --- /branches/11/funcs/func_jitterbuffer.c https://reviewboard.asterisk.org/r/3603/#comment23297 And function curly on its own line. - rmudgett On June 13, 2014, 1:48 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3603/ --- (Updated June 13, 2014, 1:48 p.m.) Review request for Asterisk Developers, Joshua Colp and Matt Jordan. Bugs: ASTERISK-22409 https://issues.asterisk.org/jira/browse/ASTERISK-22409 Repository: Asterisk Description --- During masquerade it is possible for the AST_JITTERBUFFER_FD to be cleared (set to -1). This change adds a check when copying channel fd's to prevent clearing an FD with -1. This seems to resolve the bad audio quality experienced after the masquerade. When AST_JITTERBUFFER_FD was set to -1, this prevented the channel from polling that timer. This caused RTP packets to be received late, and discarded. The changes to func_jitterbuffer.c were created first. ast_free(jbframe); is needed to prevent jbframe from leaking if it is rejected by jb_impl. This ensures we don't start leaking packets if they are received too late or rejected by jb_impl for any other reason. The second change to func_jitterbuffer prevents a leak of ast_null_frame's that were duplicated (ie with ast_frdup or ast_frisolate). I believe this leak might actually be unrelated to the masquerade issue, and likely occurs for every single ast_null_frame. Diffs - /branches/11/main/channel.c 416102 /branches/11/funcs/func_jitterbuffer.c 416102 Diff: https://reviewboard.asterisk.org/r/3603/diff/ Testing --- Verified the scenario outlined in ASTERISK-22409 no longer experiences audio quality loss, and no longer causes leaks (tested under valgrind). I patched asterisk to ensure that ast_frfree performed an immediate free to ensure valgrind would report any attempted use after free. In early testing, I used debug messages instead of the added ast_frfree's - I verified the leaked frames reported by valgrind matched exactly to the number of debug messages. For the masquerade fix I tested with some debug code that showed the old and new FD, this is how I found the valid FD being replaced by -1. See JIRA ticket for example output. I have not tested this issue or fix against 12+, but the relevant code is the same as 11 - func_jitterbuffer code was moved to core but still the same code. Thanks, Corey Farrell -- _ -- 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] 3833: Allow sending voicemail to multiple email addresses
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/#review12922 --- trunk/apps/app_voicemail.c https://reviewboard.asterisk.org/r/3833/#comment23298 Is this safe if we call ast_strlen_zero() later ? - abelbeck On July 21, 2014, 4:54 p.m., Jason Parker wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/ --- (Updated July 21, 2014, 4:54 p.m.) Review request for Asterisk Developers and Jacob Barber. Bugs: ASTERISK-24045 https://issues.asterisk.org/jira/browse/ASTERISK-24045 Repository: Asterisk Description --- Allow sending voicemail to multiple email addresses. Should this supersede https://reviewboard.asterisk.org/r/3829/ ? I would think so. Diffs - trunk/apps/app_voicemail.c 404951 Diff: https://reviewboard.asterisk.org/r/3833/diff/ Testing --- Patch in FreePBX distro for ~6 months. Thanks, Jason Parker -- _ -- 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] 3833: Allow sending voicemail to multiple email addresses
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/#review12923 --- trunk/apps/app_voicemail.c https://reviewboard.asterisk.org/r/3833/#comment23301 free() and ast_free() are NULL tolerant. However, it is likely better to set vmu-email = NULL after freeing like other places. - rmudgett On July 21, 2014, 4:54 p.m., Jason Parker wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/ --- (Updated July 21, 2014, 4:54 p.m.) Review request for Asterisk Developers and Jacob Barber. Bugs: ASTERISK-24045 https://issues.asterisk.org/jira/browse/ASTERISK-24045 Repository: Asterisk Description --- Allow sending voicemail to multiple email addresses. Should this supersede https://reviewboard.asterisk.org/r/3829/ ? I would think so. Diffs - trunk/apps/app_voicemail.c 404951 Diff: https://reviewboard.asterisk.org/r/3833/diff/ Testing --- Patch in FreePBX distro for ~6 months. Thanks, Jason Parker -- _ -- 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
[asterisk-dev] [Code Review] 3870: alembic: Adjust sippeers, queue_members, and voicemail_messages tables.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3870/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23825, ASTERISK-23847 and ASTERISK-23909 https://issues.asterisk.org/jira/browse/ASTERISK-23825 https://issues.asterisk.org/jira/browse/ASTERISK-23847 https://issues.asterisk.org/jira/browse/ASTERISK-23909 Repository: Asterisk Description --- * Increased the sippeers useragent max string size to 255. * Changed the queue_members uniqueid to an auto incremented integer instead of a string. * Increased the voicemail_messages BLOB size to LONGBLOB on mysql. * Fixed the add_tables_for_pjsip config change version downgrade actions to drop a table it created. * Adjusted the sample alembic.ini files cdr.ini.sample, config.ini.sample, and voicemail.ini.sample to give a mysql and postgres sqlalchemy.url lines. Diffs - /branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py PRE-CREATION /branches/12/contrib/ast-db-manage/voicemail.ini.sample 419804 /branches/12/contrib/ast-db-manage/config/versions/5139253c0423_make_q_member_uniqueid_autoinc.py PRE-CREATION /branches/12/contrib/ast-db-manage/config/versions/43956d550a44_add_tables_for_pjsip.py 419804 /branches/12/contrib/ast-db-manage/config/versions/1758e8bbf6b_increase_useragent_column_size.py PRE-CREATION /branches/12/contrib/ast-db-manage/config.ini.sample 419804 /branches/12/contrib/ast-db-manage/cdr.ini.sample 419804 Diff: https://reviewboard.asterisk.org/r/3870/diff/ Testing --- The alembic table adjustments work in the upgrade and downgrade modes for mysql and postgres. The queue_members change does give a postgres warning message from the postgres alembic backend about a mysql only kind of change. The warning is benign and can be ignored as it is an alembic only warning. 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
[asterisk-dev] [Code Review] 3869: res_pjsip_publish_asterisk: Inbound and outbound device state test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24115 https://issues.asterisk.org/jira/browse/ASTERISK-24115 Repository: testsuite Description --- This change adds two things: 1. The ability for an action to be configured for an AMI event handler. Right now two actions are supported: none and stop. If the stop action is configured the test will be terminated when the minimum number of events is received. 2. A test which establishes a relationship between two running Asterisk instances and publishes device state from one to the other. The test confirms that the second Asterisk receives all device state changes in the expected order. Diffs - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/publish/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/ami.py 5316 Diff: https://reviewboard.asterisk.org/r/3869/diff/ Testing --- Executed test, it passes. Sabotaged test, it fails. 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] 3607: [app_queue] Add the optional ability to load queue rules from realtime.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3607/ --- (Updated July 30, 2014, 3:07 p.m.) Review request for Asterisk Developers. Changes --- fixed more open issues Bugs: ASTERISK-23823 https://issues.asterisk.org/jira/browse/ASTERISK-23823 Repository: Asterisk Description --- This patch gives the ability (optional) to keep queuerules in realtime (important to notice that rules would be loaded from both realtime and file too) To use add to queuerules.conf general section with option to turn the relatime on (note that after patch there is no option to call rule general as it is reserved for queuerules settings): [general] realtime_rules = yes and add the realtime setting to extconfig.conf, for example in my case it's mysql and it looks like(will attach the .sql with create for table): queue_rules = mysql,asterisk,queue_rules in table as you can see you need to specify rule name and as in regular rules you can use relative setings for min and max (with - and +, e.g. +100) if one of the rule parameters is wrong it would be ignored, basically validation is like from file. here is the the example of table context as an example: rule_name, time, min_penalty, max_penalty 'default', '10', '20', '30' 'test2', '20', '30', '55' 'test2', '25', '-11', '+' 'test2', '400', '112', '333' 'test3', '0', '4564', '46546' 'test_rule', '40', '15', '50' which would result in : Rule: default After 10 seconds, adjust QUEUE_MAX_PENALTY to 30 and adjust QUEUE_MIN_PENALTY to 20 Rule: test2 After 20 seconds, adjust QUEUE_MAX_PENALTY to 55 and adjust QUEUE_MIN_PENALTY to 30 After 25 seconds, adjust QUEUE_MAX_PENALTY by and adjust QUEUE_MIN_PENALTY by -11 After 400 seconds, adjust QUEUE_MAX_PENALTY to 333 and adjust QUEUE_MIN_PENALTY to 112 Rule: test3 After 0 seconds, adjust QUEUE_MAX_PENALTY to 46546 and adjust QUEUE_MIN_PENALTY to 4564 Rule: test_rule After 40 seconds, adjust QUEUE_MAX_PENALTY to 50 and adjust QUEUE_MIN_PENALTY to 15 If you use realtime, the rules will be always reloaded (no cache option here), in other words it would delete all rules and re-add them from realtime and file. It's important to understand that with realtime on it would reload rules from file doesn't matter if file was or was not change. While with the option off it would act exactly like it was before patch (file would not be reloaded if it was not changed) Diffs (updated) - trunk/apps/app_queue.c 415442 Diff: https://reviewboard.asterisk.org/r/3607/diff/ Testing --- Currently the tests were made on development machine. Thanks, Michael K. -- _ -- 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] 3607: [app_queue] Add the optional ability to load queue rules from realtime.
On July 29, 2014, 2:57 p.m., Mark Michelson wrote: trunk/apps/app_queue.c, line 2748 https://reviewboard.asterisk.org/r/3607/diff/4/?file=61358#file61358line2748 The file is extconfig.conf, not extconfigs.conf typo :P - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3607/#review12903 --- On July 30, 2014, 3:07 p.m., Michael K. wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3607/ --- (Updated July 30, 2014, 3:07 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23823 https://issues.asterisk.org/jira/browse/ASTERISK-23823 Repository: Asterisk Description --- This patch gives the ability (optional) to keep queuerules in realtime (important to notice that rules would be loaded from both realtime and file too) To use add to queuerules.conf general section with option to turn the relatime on (note that after patch there is no option to call rule general as it is reserved for queuerules settings): [general] realtime_rules = yes and add the realtime setting to extconfig.conf, for example in my case it's mysql and it looks like(will attach the .sql with create for table): queue_rules = mysql,asterisk,queue_rules in table as you can see you need to specify rule name and as in regular rules you can use relative setings for min and max (with - and +, e.g. +100) if one of the rule parameters is wrong it would be ignored, basically validation is like from file. here is the the example of table context as an example: rule_name, time, min_penalty, max_penalty 'default', '10', '20', '30' 'test2', '20', '30', '55' 'test2', '25', '-11', '+' 'test2', '400', '112', '333' 'test3', '0', '4564', '46546' 'test_rule', '40', '15', '50' which would result in : Rule: default After 10 seconds, adjust QUEUE_MAX_PENALTY to 30 and adjust QUEUE_MIN_PENALTY to 20 Rule: test2 After 20 seconds, adjust QUEUE_MAX_PENALTY to 55 and adjust QUEUE_MIN_PENALTY to 30 After 25 seconds, adjust QUEUE_MAX_PENALTY by and adjust QUEUE_MIN_PENALTY by -11 After 400 seconds, adjust QUEUE_MAX_PENALTY to 333 and adjust QUEUE_MIN_PENALTY to 112 Rule: test3 After 0 seconds, adjust QUEUE_MAX_PENALTY to 46546 and adjust QUEUE_MIN_PENALTY to 4564 Rule: test_rule After 40 seconds, adjust QUEUE_MAX_PENALTY to 50 and adjust QUEUE_MIN_PENALTY to 15 If you use realtime, the rules will be always reloaded (no cache option here), in other words it would delete all rules and re-add them from realtime and file. It's important to understand that with realtime on it would reload rules from file doesn't matter if file was or was not change. While with the option off it would act exactly like it was before patch (file would not be reloaded if it was not changed) Diffs - trunk/apps/app_queue.c 415442 Diff: https://reviewboard.asterisk.org/r/3607/diff/ Testing --- Currently the tests were made on development machine. Thanks, Michael K. -- _ -- 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] 3842: Tests the forward and reverse playback ARI functions for both channels and bridges
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3842/ --- (Updated July 30, 2014, 3:27 p.m.) Review request for Asterisk Developers. Changes --- Changed the order of deletion. Bugs: ASTERISK-24092 https://issues.asterisk.org/jira/browse/ASTERISK-24092 Repository: testsuite Description --- Test 1: Puts a local channel half into Stasis, and then plays back a sound on it (tt-monkeys). Upon starting the sound, the playback gets reversed using /control?operation=reverse and a TestEvent gets released that signifies a reversal has occurred. Test 2: Same thing as Test 1 except that you are using the /control?operation=forward ARI command and looking for a FastForward TestEvent. Test 3: Same thing as Test 1 except that a channel gets put into a bridge, and the bridge instead is the one that starts the playback. Test 4: Same thing as Test 3 except that you are testing the /control?operation=forward command on the bridge. Diffs (updated) - /asterisk/trunk/tests/rest_api/channels/playback/tests.yaml 5249 /asterisk/trunk/tests/rest_api/channels/playback/reverse/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/playback/reverse/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/channels/playback/forward/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/playback/forward/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/playback/tests.yaml 5249 /asterisk/trunk/tests/rest_api/bridges/playback/reverse/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/playback/reverse/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/playback/forward/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/playback/forward/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3842/diff/ Testing --- Made sure that expected values were received. These tests were pretty straightforward otherwise. Thanks, Christopher Wolfe -- _ -- 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] 3726: ari: Add message technology agnostic out of call text messaging
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3726/#review12920 --- /branches/12/include/asterisk/json.h https://reviewboard.asterisk.org/r/3726/#comment23295 Drop this change. /branches/12/include/asterisk/manager.h https://reviewboard.asterisk.org/r/3726/#comment23296 Unrelated change? /branches/12/include/asterisk/vector.h https://reviewboard.asterisk.org/r/3726/#comment23299 This seems to be unrelated and has already been committed. /branches/12/main/json.c https://reviewboard.asterisk.org/r/3726/#comment23300 This should check the type of ast_json_object_iter_value(it_json_var) before getting its string value or check that the string value is non-NULL since if it is not a string, it will return NULL and ast_variable_new will segfault. /branches/12/main/message.c https://reviewboard.asterisk.org/r/3726/#comment23305 This can be changed to ao2_bump for clarity. It may actually need to be ao2_replace depending on whether the value in the datastore can be set already. /branches/12/main/message.c https://reviewboard.asterisk.org/r/3726/#comment23304 This needs to be dropped since the dialplan callback no longer owns the reference passed to it. /branches/12/main/message.c https://reviewboard.asterisk.org/r/3726/#comment23302 This is not necessary since the callback already owns a reference passed to it via the taskprocessor via ast_msg_queue which is reference-stealing. /branches/12/main/message.c https://reviewboard.asterisk.org/r/3726/#comment23303 This one is still necessary. /branches/12/rest-api/api-docs/endpoints.json https://reviewboard.asterisk.org/r/3726/#comment23308 Doesn't this conflict with /endpoints/{tech} and /endpoints/{tech}/{resource} below? There are currently no other paths that override a path parameter like this. - opticron On July 27, 2014, 9:20 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3726/ --- (Updated July 27, 2014, 9:20 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23692 and ASTERISK-23969 https://issues.asterisk.org/jira/browse/ASTERISK-23692 https://issues.asterisk.org/jira/browse/ASTERISK-23969 Repository: Asterisk Description --- This patch adds the ability to send and receive text messages from various technology stacks in Asterisk through ARI. This includes chan_sip (sip), res_pjsip_messaging (pjsip), and res_xmpp (xmpp). The following would send the message Hello there to PJSIP endpoint alice with a display URI of sip:aster...@mycooldomain.org: ari/endpoints/sendMessage?to=pjsip:alicefrom=sip:aster...@mycooldomain.orgbody=Hello+There This is equivalent to the following as well: ari/endpoints/PJSIP/alice/sendMessage?from=sip:aster...@mycooldomain.orgbody=Hello+There Both forms are available for message technologies that allow for arbitrary destinations, such as chan_sip. Inbound messages can now be received over ARI. An ARI application that subscribes to endpoints will receive messages from those endpoints: { type: TextMessageReceived, timestamp: 2014-07-12T22:53:13.494-0500, endpoint: { technology: PJSIP, resource: alice, state: online, channel_ids: [] }, message: { from: \alice\ sip:alice@127.0.0.1, to: pjsip:asterisk@127.0.0.1, body: Watson, come here., variables: [] }, application: testsuite } A few interesting things you could do with this: (1) Build your own XMPP to SIP gateway (without ever touching dialplan) (2) Make a conferencing application with built-in text messaging (speech to text would be fun with this... probably should write that too) (3) WebRTC! SIP stacks in the browser can send MESSAGE requests. Why limit yourself to just making calls when you can send arbitrary messages to a communications application? (Note: if you can't mention WebRTC in a release, you're not trying very hard) The above was made possible due to some rather major changes in the message core. This includes (but is not limited to): - Users of the message API can now register message handlers. A handler has two callbacks: one to determine if the handler has a destination for the message, and another to handle it. - All dialplan functionality of handling a message was moved into a message handler provided by the message API. - Messages can now have the technology/endpoint associated with them. Various other properties are also now more easily accessible. - A number of ao2 containers that weren't really needed were replaced with vectors. Iteration over ao2_containers
Re: [asterisk-dev] [Code Review] 3867: [chan_sip] Default DTLS settings to use if peer misses own settings
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3867/#review12924 --- trunk/channels/chan_sip.c https://reviewboard.asterisk.org/r/3867/#comment23310 Use ast_rtp_dtls_cfg_copy() here. The current change has two problems: 1) Many of the default DTLS settings are never copied onto the peer, so setting them doesn't do anything. 2) Since you are doing a shallow copy of the default strings, there is potential to end up freeing the default values when you don't mean to. trunk/channels/chan_sip.c https://reviewboard.asterisk.org/r/3867/#comment23311 If you use ast_rtp_dtls_cfg_copy() above as I recommend, then this entire block can be removed. All default values will already be copied onto the peer. Any peer-specific value encountered in the configuration will be copied onto the peer by ast_rtp_dtls_cfg_parse(). - Mark Michelson On July 30, 2014, 9:15 a.m., Michael K. wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3867/ --- (Updated July 30, 2014, 9:15 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24128 https://issues.asterisk.org/jira/browse/ASTERISK-24128 Repository: Asterisk Description --- Load default dtls settings from [general settings] If peer has dtls enabled but misses some of the settings and they are set in default settings it would load them from there. It would be logical as most of sip settings work like that and as well as there is no way to use template in realtime and it would lead to copy paste same settings for every peer. Diffs - trunk/channels/chan_sip.c 419804 Diff: https://reviewboard.asterisk.org/r/3867/diff/ Testing --- Test on development server (ast 11.11.0) Thanks, Michael K. -- _ -- 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] 3607: [app_queue] Add the optional ability to load queue rules from realtime.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3607/#review12926 --- Ship it! Ship It! - Mark Michelson On July 30, 2014, 3:07 p.m., Michael K. wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3607/ --- (Updated July 30, 2014, 3:07 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23823 https://issues.asterisk.org/jira/browse/ASTERISK-23823 Repository: Asterisk Description --- This patch gives the ability (optional) to keep queuerules in realtime (important to notice that rules would be loaded from both realtime and file too) To use add to queuerules.conf general section with option to turn the relatime on (note that after patch there is no option to call rule general as it is reserved for queuerules settings): [general] realtime_rules = yes and add the realtime setting to extconfig.conf, for example in my case it's mysql and it looks like(will attach the .sql with create for table): queue_rules = mysql,asterisk,queue_rules in table as you can see you need to specify rule name and as in regular rules you can use relative setings for min and max (with - and +, e.g. +100) if one of the rule parameters is wrong it would be ignored, basically validation is like from file. here is the the example of table context as an example: rule_name, time, min_penalty, max_penalty 'default', '10', '20', '30' 'test2', '20', '30', '55' 'test2', '25', '-11', '+' 'test2', '400', '112', '333' 'test3', '0', '4564', '46546' 'test_rule', '40', '15', '50' which would result in : Rule: default After 10 seconds, adjust QUEUE_MAX_PENALTY to 30 and adjust QUEUE_MIN_PENALTY to 20 Rule: test2 After 20 seconds, adjust QUEUE_MAX_PENALTY to 55 and adjust QUEUE_MIN_PENALTY to 30 After 25 seconds, adjust QUEUE_MAX_PENALTY by and adjust QUEUE_MIN_PENALTY by -11 After 400 seconds, adjust QUEUE_MAX_PENALTY to 333 and adjust QUEUE_MIN_PENALTY to 112 Rule: test3 After 0 seconds, adjust QUEUE_MAX_PENALTY to 46546 and adjust QUEUE_MIN_PENALTY to 4564 Rule: test_rule After 40 seconds, adjust QUEUE_MAX_PENALTY to 50 and adjust QUEUE_MIN_PENALTY to 15 If you use realtime, the rules will be always reloaded (no cache option here), in other words it would delete all rules and re-add them from realtime and file. It's important to understand that with realtime on it would reload rules from file doesn't matter if file was or was not change. While with the option off it would act exactly like it was before patch (file would not be reloaded if it was not changed) Diffs - trunk/apps/app_queue.c 415442 Diff: https://reviewboard.asterisk.org/r/3607/diff/ Testing --- Currently the tests were made on development machine. Thanks, Michael K. -- _ -- 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
[asterisk-dev] [Code Review] 3872: Tests the various ARI channel muting applications (the both, in, and out directions)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3872/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24094 https://issues.asterisk.org/jira/browse/ASTERISK-24094 Repository: testsuite Description --- Shows what happens when a Local channel's first half is muted (in the both, in, and out directions) and playback occurs in one of 4 ways: 1) Both channel halves play back a sound. 2) The first (muted) half plays back a sound only. 3) The second (unmuted) half plays back a sound only 4) Both channel halves play back a sound, the first channel half gets unmuted and then both channels play back again (shows that unmuting works). Uses a TALK_DETECT hook to check whether muting has occurred or not. Because muting was more powerful than expected, the conditions listed in the issue do not match what actually is being checked in the test. Diffs - /asterisk/trunk/tests/rest_api/channels/tests.yaml 5316 /asterisk/trunk/tests/rest_api/channels/mute/tests.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/mute/out_only/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/mute/out_only/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/channels/mute/in_only/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/mute/in_only/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/channels/mute/both/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/mute/both/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/sample-yaml/sound-check-config.yaml.sample PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3872/diff/ Testing --- Verified that TalkingEvents happened inside the log files. For some reason, the ChannelTalkingStart events of AMI didn't accurately show whether talking had occurred on a channel, so those event checkers were scrapped. The ARI ChannelTalkingStarted events were more accurate. Made sure that channels have both entered Stasis before starting a test. Only deletes a channel after playback is done. This is so that the results aren't fudged. It was discovered during testing that muting is overzealous, so the test just shows what happens in certain muting events. Thanks, Christopher Wolfe -- _ -- 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
[asterisk-dev] [Code Review] 3873: Testsuite: RLS tests - nominal presence lists
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3873/ --- Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-23870 and ASTERISK-23872 https://issues.asterisk.org/jira/browse/ASTERISK-23870 https://issues.asterisk.org/jira/browse/ASTERISK-23872 Repository: testsuite Description --- Continued from: https://reviewboard.asterisk.org/r/3673/ This changeset implements the nominal resource list tests outlined on this page: https://wiki.asterisk.org/wiki/display/AST/Resource+List+Subscription+Test+Plan There are six tests: 1. Subscription Establishment: Simply ensures that Asterisk responds with a 200 OK when we subscribe to a resource list and that the 200 OK has a Require: eventlist header in it. 2. Initial NOTIFY: Validates the initial NOTIFY body that Asterisk sends when subscribing to a resource list. 3. Full State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with full state of the list. 4. Partial State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with partial state, with only the state of the resource whose state was changed. 5. Resubscription Full State: Establishes a subscription and then resubscribes. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the resubscription has full state of the list. 6. Termination Full State: Establishes a subscription and then terminates the subscription. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the termination has full state of the list. Since that review was posted, I've also added support for lists of lists and MWI bodies to the RLSIntegrity and pcap libraries. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/rls_integrity.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/sipp/resubscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/resubscribe.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/partial_state.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/configs/ast1/pjsip.conf PRE-CREATION
Re: [asterisk-dev] [Code Review] 3807: xmldoc: Add support for an example tag in the Asterisk XML documentation
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3807/#review12927 --- Ship it! Ship It! - Mark Michelson On July 28, 2014, 7:03 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3807/ --- (Updated July 28, 2014, 7:03 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This patch adds support for an example / tag in the XML documentation schema. For CLI help, this doesn't change the formatting too much: - Preceeding white space is removed - Unlike with para elements, new lines are preserved However, having an example / tag in the XML schema allows for the wiki documentation generation script to surround the documentation with {code} or {noformat} tags, generating much better content for the wiki - and allowing us to put dialplan examples (and other code snippets, if desired) into the documentation for an application/function/AMI command/etc. Diffs - /trunk/main/xmldoc.c 419680 /trunk/funcs/func_jitterbuffer.c 419680 /trunk/doc/appdocsxml.dtd 419680 Diff: https://reviewboard.asterisk.org/r/3807/diff/ Testing --- Updated the JITTERBUFFER function. It now displays its dialplan examples in a single example block in the CLI help. Thanks, Matt Jordan -- _ -- 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] 3873: Testsuite: RLS tests - nominal presence lists
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3873/#review12928 --- /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/pjsip.conf https://reviewboard.asterisk.org/r/3873/#comment23313 This should not be full_state. As below with termination, the idea is that this should test that full state is sent for resubscribes even when partial state is set here. /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf https://reviewboard.asterisk.org/r/3873/#comment23312 This doesn't belong here in spite of the name of the test. Actually, the idea is to test that it sends full state on termination even when partial state is used. - Jonathan Rose On July 30, 2014, 11:44 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3873/ --- (Updated July 30, 2014, 11:44 a.m.) Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-23870 and ASTERISK-23872 https://issues.asterisk.org/jira/browse/ASTERISK-23870 https://issues.asterisk.org/jira/browse/ASTERISK-23872 Repository: testsuite Description --- Continued from: https://reviewboard.asterisk.org/r/3673/ This changeset implements the nominal resource list tests outlined on this page: https://wiki.asterisk.org/wiki/display/AST/Resource+List+Subscription+Test+Plan There are six tests: 1. Subscription Establishment: Simply ensures that Asterisk responds with a 200 OK when we subscribe to a resource list and that the 200 OK has a Require: eventlist header in it. 2. Initial NOTIFY: Validates the initial NOTIFY body that Asterisk sends when subscribing to a resource list. 3. Full State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with full state of the list. 4. Partial State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with partial state, with only the state of the resource whose state was changed. 5. Resubscription Full State: Establishes a subscription and then resubscribes. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the resubscription has full state of the list. 6. Termination Full State: Establishes a subscription and then terminates the subscription. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the termination has full state of the list. Since that review was posted, I've also added support for lists of lists and MWI bodies to the RLSIntegrity and pcap libraries. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/rls_integrity.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/extensions.conf PRE-CREATION
Re: [asterisk-dev] [Code Review] 3799: manager: Add ExtensionStateList, PresenceStateList, and DeviceStateList commands
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3799/ --- (Updated July 30, 2014, 1:32 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 419806 Repository: Asterisk Description --- This patch adds three new AMI commands: * ExtensionStateList (pbx.c) - list all known extension state hints and their current statuses. Events emitted by the list action are equivalent to the ExtensionStatus events. * PresenceStateList (res_manager_presencestate) - list all known presence state values. Events emitted are generated by the stasis message type, and hence are PresenceStateChange events. * DeviceStateList (res_manager_devicestate) - list all known device state values. Events emitted are generated by the stasis message type, and hence are DeviceStateChange events. Diffs - /trunk/res/res_manager_presencestate.c 419341 /trunk/res/res_manager_devicestate.c 419341 /trunk/main/pbx.c 419341 /trunk/main/manager.c 419341 Diff: https://reviewboard.asterisk.org/r/3799/diff/ Testing --- Currently, only manual verification: * Made two hints, one with presence. * Ran all three commands and checked the output * Used a Custom device state and a CustomPresence provider and changed their statuses using a Local channel and the DEVICE_STATE/PRESENCE_STATE functions * Ran all three commands again and got back the expected updated values Update: Tests are now available on review 3843: https://reviewboard.asterisk.org/r/3843/ Thanks, Matt Jordan -- _ -- 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
[asterisk-dev] [Code Review] 3874: Add tests for the DeviceStateList, PresenceStateList, and ExtensionStateList AMI actions
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3874/ --- Review request for Asterisk Developers and Matt Jordan. Repository: testsuite Description --- This patch adds tests for the DeviceStateList, PresenceStateList, and ExtensionStateList AMI actions. Each test: * Sets two device/presence/device presence state values (respectively for each of the previously listed actions) * Runs the appropriate list action * Verifies that the action has the expected response * Verifies that the intermediate expected events are received * Verifies that the list complete event is received This review supersedes review 3843 and starts with its most recent patch. Diffs - /asterisk/trunk/tests/manager/tests.yaml 5304 /asterisk/trunk/tests/manager/presence_state_list/test-config.yaml PRE-CREATION /asterisk/trunk/tests/manager/presence_state_list/ami_presence_state_list.py PRE-CREATION /asterisk/trunk/tests/manager/exten_state_list/test-config.yaml PRE-CREATION /asterisk/trunk/tests/manager/exten_state_list/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/manager/exten_state_list/ami_exten_state_list.py PRE-CREATION /asterisk/trunk/tests/manager/device_state_list/test-config.yaml PRE-CREATION /asterisk/trunk/tests/manager/device_state_list/ami_device_state_list.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3874/diff/ Testing --- Ensured the tests worked as expected. 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] 3874: Add tests for the DeviceStateList, PresenceStateList, and ExtensionStateList AMI actions
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3874/ --- (Updated July 30, 2014, 2:47 p.m.) Review request for Asterisk Developers and Matt Jordan. Changes --- This updates the patch according to the review items present on review 3843. Repository: testsuite Description --- This patch adds tests for the DeviceStateList, PresenceStateList, and ExtensionStateList AMI actions. Each test: * Sets two device/presence/device presence state values (respectively for each of the previously listed actions) * Runs the appropriate list action * Verifies that the action has the expected response * Verifies that the intermediate expected events are received * Verifies that the list complete event is received This review supersedes review 3843 and starts with its most recent patch. Diffs (updated) - asterisk/trunk/tests/manager/tests.yaml 5305 asterisk/trunk/tests/manager/presence_state_list/test-config.yaml PRE-CREATION asterisk/trunk/tests/manager/presence_state_list/ami_presence_state_list.py PRE-CREATION asterisk/trunk/tests/manager/exten_state_list/test-config.yaml PRE-CREATION asterisk/trunk/tests/manager/exten_state_list/configs/ast1/extensions.conf PRE-CREATION asterisk/trunk/tests/manager/exten_state_list/ami_exten_state_list.py PRE-CREATION asterisk/trunk/tests/manager/device_state_list/test-config.yaml PRE-CREATION asterisk/trunk/tests/manager/device_state_list/ami_device_state_list.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3874/diff/ Testing --- Ensured the tests worked as expected. 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] 3873: Testsuite: RLS tests - nominal presence lists
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3873/ --- (Updated July 30, 2014, 2:54 p.m.) Review request for Asterisk Developers and Mark Michelson. Changes --- Change the branch... it's not in 12 and I don't think it's going into 12. It's not even in trunk yet. Bugs: ASTERISK-23870 and ASTERISK-23872 https://issues.asterisk.org/jira/browse/ASTERISK-23870 https://issues.asterisk.org/jira/browse/ASTERISK-23872 Repository: testsuite Description --- Continued from: https://reviewboard.asterisk.org/r/3673/ This changeset implements the nominal resource list tests outlined on this page: https://wiki.asterisk.org/wiki/display/AST/Resource+List+Subscription+Test+Plan There are six tests: 1. Subscription Establishment: Simply ensures that Asterisk responds with a 200 OK when we subscribe to a resource list and that the 200 OK has a Require: eventlist header in it. 2. Initial NOTIFY: Validates the initial NOTIFY body that Asterisk sends when subscribing to a resource list. 3. Full State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with full state of the list. 4. Partial State: Establishes a subscription to a resource list and then changes the state of a resource. Ensures that Asterisk sends a NOTIFY with partial state, with only the state of the resource whose state was changed. 5. Resubscription Full State: Establishes a subscription and then resubscribes. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the resubscription has full state of the list. 6. Termination Full State: Establishes a subscription and then terminates the subscription. Ensures that even though partial state is configured, the NOTIFY that Asterisk sends in response to the termination has full state of the list. Since that review was posted, I've also added support for lists of lists and MWI bodies to the RLSIntegrity and pcap libraries. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/rls_integrity.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/sipp/resubscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/resubscribe.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/sipp/list_subscribe.xml PRE-CREATION
[asterisk-dev] [Code Review] 3875: Testsuite: RLS tests - nominal MWI lists
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3875/ --- Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-23870 https://issues.asterisk.org/jira/browse/ASTERISK-23870 Repository: testsuite Description --- Same as https://reviewboard.asterisk.org/r/3873/, only for MWI tests. Uses the MWI additions for the rlsvalidator and pcap libraries to match against MWI type message bodies. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/termination_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/termination_full_state/termination.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/termination_full_state/sipp/termination.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/termination_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/termination_full_state/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/subscription_establishment/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/subscription_establishment/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/subscription_establishment/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/resubscribe_full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/resubscribe_full_state/sipp/resubscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/resubscribe_full_state/resubscribe.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/resubscribe_full_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/partial_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/partial_state/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/partial_state/partial_state.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/partial_state/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/initial_notify/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/initial_notify/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/initial_notify/notify.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/initial_notify/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/full_state/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/full_state/sipp/list_subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/full_state/full_state.py PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/mwi/full_state/configs/ast1/pjsip.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3875/diff/ Testing --- Varied expectations of things like which mailboxes to expect and their contents from results and got failures. Still running against mmichelson's rls-rlmi branch. 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] 3854: manager.c - Improve documentation for manager command Getvar, Setvar
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/ --- (Updated July 30, 2014, 10:40 p.m.) Review request for Asterisk Developers. Changes --- Integrated Josh and Mark's comments. Additionally, I modified the descriptions of the parameters for clarity. For example, for Setvar, the Variable parameter can really be set to a variable name, function or expression. I didn't mention expressions in the descriptions to avoid being overly verbose and explicit. Bugs: ASTERISK-21178 https://issues.asterisk.org/jira/browse/ASTERISK-21178 Repository: Asterisk Description --- The documentation wasn't clear that AMI Getvar and Setvar could accept function calls. This is a slight modification to improve clarity. Diffs (updated) - /branches/1.8/main/manager.c 419821 Diff: https://reviewboard.asterisk.org/r/3854/diff/ Testing --- Once finalized I'll build in dev-mode with it to make sure I didn't screw up any tags. Thanks, rnewton -- _ -- 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] 3854: manager.c - Improve documentation for manager command Getvar, Setvar
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/#review12930 --- /branches/1.8/main/manager.c https://reviewboard.asterisk.org/r/3854/#comment23315 I think I need literal tags for the mentions of underscores here and just below. - rnewton On July 30, 2014, 10:40 p.m., rnewton wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/ --- (Updated July 30, 2014, 10:40 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-21178 https://issues.asterisk.org/jira/browse/ASTERISK-21178 Repository: Asterisk Description --- The documentation wasn't clear that AMI Getvar and Setvar could accept function calls. This is a slight modification to improve clarity. Diffs - /branches/1.8/main/manager.c 419821 Diff: https://reviewboard.asterisk.org/r/3854/diff/ Testing --- Once finalized I'll build in dev-mode with it to make sure I didn't screw up any tags. Thanks, rnewton -- _ -- 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] 3854: manager.c - Improve documentation for manager command Getvar, Setvar
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/ --- (Updated July 30, 2014, 10:45 p.m.) Review request for Asterisk Developers. Changes --- Add literal tags where '_' and '__' were used. Bugs: ASTERISK-21178 https://issues.asterisk.org/jira/browse/ASTERISK-21178 Repository: Asterisk Description --- The documentation wasn't clear that AMI Getvar and Setvar could accept function calls. This is a slight modification to improve clarity. Diffs (updated) - /branches/1.8/main/manager.c 419821 Diff: https://reviewboard.asterisk.org/r/3854/diff/ Testing --- Once finalized I'll build in dev-mode with it to make sure I didn't screw up any tags. Thanks, rnewton -- _ -- 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] 3854: manager.c - Improve documentation for manager command Getvar, Setvar
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/ --- (Updated July 30, 2014, 10:52 p.m.) Review request for Asterisk Developers. Changes --- modified testing field to describe time of test and success. Bugs: ASTERISK-21178 https://issues.asterisk.org/jira/browse/ASTERISK-21178 Repository: Asterisk Description --- The documentation wasn't clear that AMI Getvar and Setvar could accept function calls. This is a slight modification to improve clarity. Diffs - /branches/1.8/main/manager.c 419821 Diff: https://reviewboard.asterisk.org/r/3854/diff/ Testing (updated) --- Once finalized I'll build in dev-mode with it to make sure I didn't screw up any tags. 7/30/14 @ 5:52PM CDT - Builds with no issues in dev-mode. Thanks, rnewton -- _ -- 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] 3603: func_jitterbuffer: fix errors and leaks caused by certain masquerade's
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3603/ --- (Updated July 30, 2014, 7:19 p.m.) Review request for Asterisk Developers, Joshua Colp and Matt Jordan. Changes --- Correct issues in chan_fixup callback. Only ast_frfree(jbframe) when it != frame. jb_helper was split due to channels already being locked during masquerade. Retested 1) with jitterbuffer enabled in confbridge.conf 2) with setvar=JITTERBUFFER(adaptive)=default in sip.conf 3) with both set. All 3 cases no loss of audio. Bugs: ASTERISK-22409 https://issues.asterisk.org/jira/browse/ASTERISK-22409 Repository: Asterisk Description --- During masquerade it is possible for the AST_JITTERBUFFER_FD to be cleared (set to -1). This change adds a check when copying channel fd's to prevent clearing an FD with -1. This seems to resolve the bad audio quality experienced after the masquerade. When AST_JITTERBUFFER_FD was set to -1, this prevented the channel from polling that timer. This caused RTP packets to be received late, and discarded. The changes to func_jitterbuffer.c were created first. ast_free(jbframe); is needed to prevent jbframe from leaking if it is rejected by jb_impl. This ensures we don't start leaking packets if they are received too late or rejected by jb_impl for any other reason. The second change to func_jitterbuffer prevents a leak of ast_null_frame's that were duplicated (ie with ast_frdup or ast_frisolate). I believe this leak might actually be unrelated to the masquerade issue, and likely occurs for every single ast_null_frame. Diffs (updated) - /branches/11/main/channel.c 419821 /branches/11/funcs/func_jitterbuffer.c 419821 Diff: https://reviewboard.asterisk.org/r/3603/diff/ Testing --- Verified the scenario outlined in ASTERISK-22409 no longer experiences audio quality loss, and no longer causes leaks (tested under valgrind). I patched asterisk to ensure that ast_frfree performed an immediate free to ensure valgrind would report any attempted use after free. In early testing, I used debug messages instead of the added ast_frfree's - I verified the leaked frames reported by valgrind matched exactly to the number of debug messages. For the masquerade fix I tested with some debug code that showed the old and new FD, this is how I found the valid FD being replaced by -1. See JIRA ticket for example output. I have not tested this issue or fix against 12+, but the relevant code is the same as 11 - func_jitterbuffer code was moved to core but still the same code. Thanks, Corey Farrell -- _ -- 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