[asterisk-dev] BUG? Asterisk V10 SIP Message To: non numeric IP (mobile1.xyz.com) fails
Hi, I’m using SIP MESSAGE to asterisk V10 and it fails to be received. I’m not super sure of the reason but I’m making this guess: Due to I’m using non ipaddress in the to field, which contains sip:mobil1.xyz.com, Asterisk makes the mistake to try matching this name ”mobil1.testserver.com” in extensions.conf and no extension/peer is found in the sip-message context I’ve configured. It works when the TO: field contains an numeric ipadress. Is this a bug or an intentional limitation? /Johan LOG [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: --- SIP read from UDP:83.186.238.111:5060 --- MESSAGE sip:mobil1.xyz.com SIP/2.0 Via: SIP/2.0/UDP 83.186.238.111:5060;branch=z9hG4bK-3f138a53 To: sip:mobil1.xyz.com From: sip:83.186.238.111;tag=7a82b127 Call-ID: 857d4ed8@83.186.238.111mailto:857d4ed8@83.186.238.111 CSeq: 245 MESSAGE Max-Forwards: 70 User-Agent: CareIP 7813409 v1.2.4.0 Content-Type: application/scaip+xml Content-Length: 138 My message - [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 0 [ 49]: MESSAGE sip:mobil1.xyz.com SIP/2.0 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 1 [ 60]: Via: SIP/2.0/UDP 83.186.238.111:5060;branch=z9hG4bK-3f138a53 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 2 [ 39]: To: sip:mobil1.xyz.com [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 3 [ 39]: From: sip:83.186.238.111;tag=7a82b127 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 4 [ 32]: Call-ID: 857d4ed8@83.186.238.111mailto:857d4ed8@83.186.238.111 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 5 [ 17]: CSeq: 245 MESSAGE [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 6 [ 16]: Max-Forwards: 70 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 7 [ 35]: User-Agent: CareIP 7813409 v1.2.4.0 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 8 [ 35]: Content-Type: application/scaip+xml [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 9 [ 19]: Content-Length: 138 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Header 10 [ 0]: [Feb 12 15:13:59] DEBUG[25824] chan_sip.c:Body 0 [138]: My message [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: --- (10 headers 1 lines) --- [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: = Looking for Call ID: 857d4ed8@83.186.238.111mailto:857d4ed8@83.186.238.111 (Checking From) --From tag 7a82b127 --To-tag [Feb 12 15:13:59] DEBUG[25824] acl.c: For destination '83.186.238.111', our source address is '172.26.19.13'. [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Target address 83.186.238.111:5060 is not local, substituting externaddr [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Setting SIP_TRANSPORT_UDP with address 212.105.99.108:5060 [Feb 12 15:13:59] DEBUG[25824] chan_sip.c: Allocating new SIP dialog for 857d4ed8@83.186.238.111mailto:857d4ed8@83.186.238.111 - MESSAGE (No RTP) [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: No matching peer for '83.186.238.111' from '83.186.238.111:5060' [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: Looking for s in sipmessage (domain mobil1.xyz.com) [Feb 12 15:13:59] WARNING[25812] pbx.c: Channel 'Message/ast_msg_queue' sent into invalid extension 'mobil1.xyz.com' in context 'sipmessage', but no invalid handler Johan Sandgren Software Engineer Svep Design Center AB S:t Lars väg 42A 222 70 Lund, Sweden Phone +46 46 192 722 E-mail j...@svep.semailto:j...@svep.se Website www.svep.sehttp://www.svep.se -- _ -- 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] AMI and Sorcery
On 14-02-12 03:26 AM, George Joseph wrote: In the process of creating the dialplan function AST_SORCERY (a companion to AST_CONFIG) I've run into an issue with how AMI interacts with sorcery particularly related to objects that can have multiple occurrences of the same field like contact in aor and match in identify. snip Notice the inconsistency? In the config, both contact and match are singular but in the output, Contacts is plural, Match is singular. The difference comes from how the 2 objects construct the AMI event. Contact has custom code that spits out the literal Contacts: then the list, whereas Identify uses the generic sorcery code which spits out the name of the field (hence the singular) and the list. If the fact that one is plural and the other is singular is a bug, tell me which is correct (hopefully the singular, see below) and you don't have to read further unless you've got some time to kill. If it's a feature, read on... The returned name should be the name of the field, thus contact. Background: On Monday I started work on the AST_SORCERY dialplan function which allows you to retrieve arbitrary fields from a sorcery based config like pjsip.conf. The function itself is quite simple. Basically, it calls ast_sorcery_objectset_create and iterates over the resulting ast_variable list to find the field of interest. Works great for single occurrence fields but as I found out, not at all for multiple occurrence fields like aor's contacts and identify's matches. As it turns out, the structure that defines a sorcery field does have a virtual function member for returning multiple occurrence fields as an ast_variable list but none of the objects implement it and besides, none of the ast_sorcery_object_field_register functions allow you to set it. In the end, without coding object-specific logic in AST_SORCERY, there was no way to retrieve the multiple occurrence fields because ast_sorcery_objectset_create wasn't returning them. The multiple_fields functionality is actually exposed, just only through the regex support. Well, I fixed all of that and now ast_sorcery_objectset_create can return an ast_variable list with as many entries for contact and match (and any others) as there are entries in the conf file. Works great...except for the AMI. I'm told that that an AMI event can't have multiple fields with the same name so multiple Contact or Match fields wouldn't work. Ok, you've always been able to register a handler that spits out a single string for multiple occurrences and you can now tell ast_sorcery_objectset_create which behavior you want. Except (to bring this back to the beginning), that would change the hard coded Contacts in the event to the generically generated Contact. :) Do you have unit tests for that? :D So, can I remove the s? Can I, Please?? Or I guess I can still use the generic code and do a search and replace in the AMI output buffer and add the s back in. I suppose the question really is... do we make this incompatible with the past. Either way, this'll be good background info for why a patch to add 1 simple dialplan function is as complex as it is. Bonus Question: Why are aors and auths specified as a single comma-separated string where contacts and matches are specified on multiple lines? Hilarity, that's why. Okay, not really. No immediate answer springs to mind. -- 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] 3212: Makefile: fix so make main works without compiling deps (unless needed)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3212/#review10866 --- Ship it! Ship It! - Joshua Colp On Feb. 12, 2014, 8:34 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3212/ --- (Updated Feb. 12, 2014, 8:34 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When I want to test that my svn-merged code works, I look at the diff and I check that the compilation succeeds. The quickest way is then to build only the subdirectory where the changed code resides. E.g. when changing something in res_config_pgsql.so: make res However, when something is changed in main, it will compile *all* the other dirs first. Making me wait. ifeq ($(findstring $(OSARCH), mingw32 cygwin ),) # Non-windows: # ensure that all module subdirectories are processed before 'main' during # a parallel build, since if there are modules selected to be embedded the # directories containing them must be completed before the main Asterisk # binary can be built main: $(filter-out main,$(MOD_SUBDIRS)) I doubt that more than 1% of the people uses embedded modules. Don't make the rest of us wait! This change adds a second check for the $(MENUSELECT_EMBED) variable: if it is empty, we do not add the dependency. Diffs - /trunk/Makefile 407969 Diff: https://reviewboard.asterisk.org/r/3212/diff/ Testing --- Ran a bunch of make, make clean, make distclean, make main, with and without module embedding selected. If any module is embedded, the dependency is enforced like before. If no module embedding is selected, main compiles directly. 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
[asterisk-dev] Fwd: patch
Hello everyone.
We have been struggling for one week on a bug with H264 video in
asterisk11. I do not know if it also applies to Asterisk 12.
Apparently res_format_attr_h264.c does not write properly the codec
attributes in the SDP, causing a codec mismatch when sending the video
frames.
The patch that follows is basically a
sed -i'' '/profile-level-id/ s#%X%X%X#%02X%02X%02X#'
to ensure that the profile-level-id is always six-hex-digits long.
I am not too sure about how to integrate the patch or submit a merge
request, and really lack the time now to investigate further.
Kind Regards,
Guillaume.
---
--- res_format_attr_h264.c (revision 13930)
+++ res_format_attr_h264.c (working copy)
@@ -268,11 +272,11 @@
} else if (i == H264_ATTR_KEY_PROFILE_IDC
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IDC]
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IOP]
format_attr-format_attr[H264_ATTR_KEY_LEVEL]) {
if (!added) {
- ast_str_append(str, 0, a=fmtp:%d
profile-level-id=%X%X%X, payload,
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IDC],
+ ast_str_append(str, 0, a=fmtp:%d
profile-level-id=%02X%02X%02X, payload,
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IDC],
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IOP],
format_attr-format_attr[H264_ATTR_KEY_LEVEL]);
added = 1;
} else {
- ast_str_append(str, 0,
;profile-level-id=%X%X%X,
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IDC],
+ ast_str_append(str, 0,
;profile-level-id=%02X%02X%02X,
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IDC],
format_attr-format_attr[H264_ATTR_KEY_PROFILE_IOP],
format_attr-format_attr[H264_ATTR_KEY_LEVEL]);
}
} else if ((name = h264_attr_key_to_str(i))
h264_attr_key_addable(format_attr, i)) {
--
Guillaume Maudoux
Product Development Engineer
ESCAUX
ESCAUX, the nr 1 alternative in Unified Communication
Chaussée de Bruxelles 408, 1300 Wavre, Belgium
Direct: +3227887560
Main: +3226860900
www.escaux.com
--
_
-- 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] BUG? Asterisk V10 SIP Message To: non numeric IP (mobile1.xyz.com) fails
On Thu, Feb 13, 2014 at 3:28 AM, Johan Sandgren j...@svep.se wrote: Hi, snip [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: --- SIP read from UDP:83.186.238.111:5060 --- MESSAGE sip:mobil1.xyz.com SIP/2.0 Via: SIP/2.0/UDP 83.186.238.111:5060;branch=z9hG4bK-3f138a53 To: sip:mobil1.xyz.com From: sip:83.186.238.111;tag=7a82b127 Call-ID: 857d4ed8@83.186.238.111 CSeq: 245 MESSAGE Max-Forwards: 70 User-Agent: CareIP 7813409 v1.2.4.0 Content-Type: application/scaip+xml Content-Length: 138 snip [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: No matching peer for '83.186.238.111' from '83.186.238.111:5060' [Feb 12 15:13:59] VERBOSE[25824] chan_sip.c: Looking for s in sipmessage (domain mobil1.xyz.com) [Feb 12 15:13:59] WARNING[25812] pbx.c: Channel 'Message/ast_msg_queue' sent into invalid extension 'mobil1.xyz.com' in context 'sipmessage', but no invalid handler It isn't a bug. It is telling you how it will attempt to match the inbound request: (1) It looks for a peer that matches what sent the MESSAGE request, in this case, sip:83.186.238.111. That fails. (2) Since the request URI is simply a domain and not a destination, it falls back to looking for an 's' extension in context 'sipmessage'. That fails. (3) Now, truly panicking, it looks for the 'i' extension in the same context. Since you don't have an invalid extension handler, that fails too. Despondent, it throws in the towel. I'm not sure where you thought it would end up, but it certainly tried lots of different places. And the 'rules' for it doing so are (for chan_sip, at any rate) relatively consistent with how inbound requests are matched for INVITE requests as well. chan_sip tries to figure out who sent the request to Asterisk, and then use that peer definition. If chan_sip can't find that, it falls back to using a general entry point. Also: please don't stay on Asterisk 10. That version is no longer supported and is no longer receiving security fixes. You should move to Asterisk 11, which is an LTS release, as soon as possible. Matt -- Matthew Jordan Digium, Inc. | Engineering Manager 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com http://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] 3214: chan_sip: Set SIP_DEFER_BYE_ON_TRANSFER prior to calling ast_bridge_transfer_blind
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/#review10867 --- It's fine to set the flag early, but I think you need to clear the defer flag if the transfer does not succeed. Sending a NOTIFY stating that the transfer failed could result in the transferrer resuming the call. If that happens, we don't want to defer the BYE anymore. - Mark Michelson On Feb. 13, 2014, 4:29 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/ --- (Updated Feb. 13, 2014, 4:29 a.m.) Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-23290 https://issues.asterisk.org/jira/browse/ASTERISK-23290 Repository: Asterisk Description --- This patch moves setting SIP_DEFER_BY_ON_TRANSFER prior to calling ast_bridge_transfer_blind. The blind-transfer-accountcode test will sporadically fail due to a BYE request being sent for the transferor prior to the NOTIFY request being sent. This is due to the PVT being unlocked while ast_bridge_transfer_blind, allowing the thread ejecting the channel from the bridge to hangup the channel. Note that it should be safe to move this prior to the ast_bridge_transfer_blind call, as the attended transfer handling does this exact same thing. It should also be okay (and probably correct) to set this flag on the channel before sending any of the NOTIFY requests, regardless of it notifying the transferor of the success or failure of the transfer. Diffs - /branches/12/channels/chan_sip.c 407986 Diff: https://reviewboard.asterisk.org/r/3214/diff/ Testing --- The test passes; however, it always did on my machine anyway... but this should fix the bug, given what the testsuite shows. 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] Fwd: patch
On Thu, Feb 13, 2014 at 7:55 AM, Guillaume Maudoux guillaume.maud...@escaux.com wrote: Hello everyone. We have been struggling for one week on a bug with H264 video in asterisk11. I do not know if it also applies to Asterisk 12. Apparently res_format_attr_h264.c does not write properly the codec attributes in the SDP, causing a codec mismatch when sending the video frames. The patch that follows is basically a sed -i'' '/profile-level-id/ s#%X%X%X#%02X%02X%02X#' to ensure that the profile-level-id is always six-hex-digits long. After a cursory reading of RFC 6184 (https://tools.ietf.org/html/rfc6184), that looks correct. I am not too sure about how to integrate the patch or submit a merge request, and really lack the time now to investigate further. Patches have to be filed in the Asterisk issue tracker at issues.asterisk.org. Simply create a new issue for the bug, sign a license contributor agreement, and attach the patch after the agreement has been approved. Instructions for submitting code back to the Asterisk project can be found on asterisk.org (http://www.asterisk.org/community/developers) and in more detail on the wiki (https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines) Please do contribute the patch back! It would be great to get that bug fixed. Thanks - Matt -- _ -- 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] 3214: chan_sip: Set SIP_DEFER_BYE_ON_TRANSFER prior to calling ast_bridge_transfer_blind
On Feb. 13, 2014, 8:17 a.m., Mark Michelson wrote: It's fine to set the flag early, but I think you need to clear the defer flag if the transfer does not succeed. Sending a NOTIFY stating that the transfer failed could result in the transferrer resuming the call. If that happens, we don't want to defer the BYE anymore. Works for me. In that case, we should probably do the same in local_attended_transfer - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/#review10867 --- On Feb. 12, 2014, 10:29 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/ --- (Updated Feb. 12, 2014, 10:29 p.m.) Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-23290 https://issues.asterisk.org/jira/browse/ASTERISK-23290 Repository: Asterisk Description --- This patch moves setting SIP_DEFER_BY_ON_TRANSFER prior to calling ast_bridge_transfer_blind. The blind-transfer-accountcode test will sporadically fail due to a BYE request being sent for the transferor prior to the NOTIFY request being sent. This is due to the PVT being unlocked while ast_bridge_transfer_blind, allowing the thread ejecting the channel from the bridge to hangup the channel. Note that it should be safe to move this prior to the ast_bridge_transfer_blind call, as the attended transfer handling does this exact same thing. It should also be okay (and probably correct) to set this flag on the channel before sending any of the NOTIFY requests, regardless of it notifying the transferor of the success or failure of the transfer. Diffs - /branches/12/channels/chan_sip.c 407986 Diff: https://reviewboard.asterisk.org/r/3214/diff/ Testing --- The test passes; however, it always did on my machine anyway... but this should fix the bug, given what the testsuite shows. 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] 3214: chan_sip: Set SIP_DEFER_BYE_ON_TRANSFER prior to calling ast_bridge_transfer_blind
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/ --- (Updated Feb. 13, 2014, 9:01 a.m.) Review request for Asterisk Developers and Mark Michelson. Changes --- Clear the SIP_DEFER_BYE_ON_TRANSFER on failure Bugs: ASTERISK-23290 https://issues.asterisk.org/jira/browse/ASTERISK-23290 Repository: Asterisk Description --- This patch moves setting SIP_DEFER_BY_ON_TRANSFER prior to calling ast_bridge_transfer_blind. The blind-transfer-accountcode test will sporadically fail due to a BYE request being sent for the transferor prior to the NOTIFY request being sent. This is due to the PVT being unlocked while ast_bridge_transfer_blind, allowing the thread ejecting the channel from the bridge to hangup the channel. Note that it should be safe to move this prior to the ast_bridge_transfer_blind call, as the attended transfer handling does this exact same thing. It should also be okay (and probably correct) to set this flag on the channel before sending any of the NOTIFY requests, regardless of it notifying the transferor of the success or failure of the transfer. Diffs (updated) - /branches/12/channels/chan_sip.c 407986 Diff: https://reviewboard.asterisk.org/r/3214/diff/ Testing --- The test passes; however, it always did on my machine anyway... but this should fix the bug, given what the testsuite shows. 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] 3214: chan_sip: Set SIP_DEFER_BYE_ON_TRANSFER prior to calling ast_bridge_transfer_blind
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/#review10869 --- Ship it! Ship It! - Mark Michelson On Feb. 13, 2014, 3:01 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3214/ --- (Updated Feb. 13, 2014, 3:01 p.m.) Review request for Asterisk Developers and Mark Michelson. Bugs: ASTERISK-23290 https://issues.asterisk.org/jira/browse/ASTERISK-23290 Repository: Asterisk Description --- This patch moves setting SIP_DEFER_BY_ON_TRANSFER prior to calling ast_bridge_transfer_blind. The blind-transfer-accountcode test will sporadically fail due to a BYE request being sent for the transferor prior to the NOTIFY request being sent. This is due to the PVT being unlocked while ast_bridge_transfer_blind, allowing the thread ejecting the channel from the bridge to hangup the channel. Note that it should be safe to move this prior to the ast_bridge_transfer_blind call, as the attended transfer handling does this exact same thing. It should also be okay (and probably correct) to set this flag on the channel before sending any of the NOTIFY requests, regardless of it notifying the transferor of the success or failure of the transfer. Diffs - /branches/12/channels/chan_sip.c 407986 Diff: https://reviewboard.asterisk.org/r/3214/diff/ Testing --- The test passes; however, it always did on my machine anyway... but this should fix the bug, given what the testsuite shows. 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] 3185: Logger: Allow creation and removal of dynamic logger channels
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3185/ --- (Updated Feb. 13, 2014, 9:52 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Bugs: AST-1150 https://issues.asterisk.org/jira/browse/AST-1150 Repository: Asterisk Description --- This adds the ability to dynamically add and remove logger channels from Asterisk via the CLI. Diffs - trunk/main/logger.c 407954 trunk/CHANGES 407954 Diff: https://reviewboard.asterisk.org/r/3185/diff/ Testing --- Verified that the logs created this way showed up where expected containing the correct logging levels. logger add channel /tmp/testchannel warning,error,verbose,notice logger remove channel /tmp/testchannel 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] 3212: Makefile: fix so make main works without compiling deps (unless needed)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3212/#review10870 --- I'd say that we could consider this a 'bug' in the build system. We shouldn't be building things that don't need to be built. What do you think about merging this in in 1.8+? - Matt Jordan On Feb. 12, 2014, 2:34 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3212/ --- (Updated Feb. 12, 2014, 2:34 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When I want to test that my svn-merged code works, I look at the diff and I check that the compilation succeeds. The quickest way is then to build only the subdirectory where the changed code resides. E.g. when changing something in res_config_pgsql.so: make res However, when something is changed in main, it will compile *all* the other dirs first. Making me wait. ifeq ($(findstring $(OSARCH), mingw32 cygwin ),) # Non-windows: # ensure that all module subdirectories are processed before 'main' during # a parallel build, since if there are modules selected to be embedded the # directories containing them must be completed before the main Asterisk # binary can be built main: $(filter-out main,$(MOD_SUBDIRS)) I doubt that more than 1% of the people uses embedded modules. Don't make the rest of us wait! This change adds a second check for the $(MENUSELECT_EMBED) variable: if it is empty, we do not add the dependency. Diffs - /trunk/Makefile 407969 Diff: https://reviewboard.asterisk.org/r/3212/diff/ Testing --- Ran a bunch of make, make clean, make distclean, make main, with and without module embedding selected. If any module is embedded, the dependency is enforced like before. If no module embedding is selected, main compiles directly. 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] 3182: testsuite: LinkedID Propagation test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3182/#review10871 --- If you don't mind, close out the findings that you've fixed (just hit the close button). I'll take another look through after that. - Matt Jordan On Feb. 10, 2014, 2:37 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3182/ --- (Updated Feb. 10, 2014, 2:37 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23120 https://issues.asterisk.org/jira/browse/ASTERISK-23120 Repository: testsuite Description --- This monitors bridge enter/exit and CEL events, and builds a picture of which channels are bridged together, and when their LinkedID changes, and then verifies that each change is correct. Note: The BridgeEnter and CEL 'BRIDGE_ENTER' events can be in either order - so wait for both to arrive before making a judgement on the LinkedID. This test can be added to any other bridging test to add a layer of LinkedId Propagation checking to it. This test has now been added to the existing bridge_action test rather than have a separate test for it. Diffs - /asterisk/trunk/tests/bridge/bridge_action/test-config.yaml 4675 /asterisk/trunk/tests/bridge/bridge_action/bridge_action.py 4675 /asterisk/trunk/lib/python/asterisk/linkedid_check.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3182/diff/ Testing --- This test fails spuriously due to what appears to be CEL events being sent on BRIDGE_ENTER prior to the LinkedId being updated. This bug will be fixed along with the other changes for 23120. 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] 3182: testsuite: LinkedID Propagation test
On Feb. 6, 2014, 10:44 a.m., Matt Jordan wrote: /asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py, lines 3-4 https://reviewboard.asterisk.org/r/3182/diff/2/?file=53430#file53430line3 I didn't write this :-) Or... maybe I did? If so, and multiple tests need to use this logic, there's no need to copy it. In fact, having duplicate copies of what drives the same test isn't needed. There are two ways to handle this: (1) Add the linkedid propagation verification to the existing test. This is probably preferable, if what drives the test is exactly the same as another test. (2) If there are differences between the Asterisk actions that drive the test, then refactor the common code into a shared module. You have two choices on where to put that shared module: (a) In lib/python/asterisk (b) In a Python module in a base directory. The YAML for defining pluggable modules lets you specify search paths for modules, which you can then specify to the location of the module. I pulled a copy of bridge_action.py into linkedid_propgation test dir as-is. However, I agree that the linkedid check can just be added to the existing bridge_action tests - posting that as an update. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3182/#review10782 --- On Feb. 10, 2014, 2:37 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3182/ --- (Updated Feb. 10, 2014, 2:37 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23120 https://issues.asterisk.org/jira/browse/ASTERISK-23120 Repository: testsuite Description --- This monitors bridge enter/exit and CEL events, and builds a picture of which channels are bridged together, and when their LinkedID changes, and then verifies that each change is correct. Note: The BridgeEnter and CEL 'BRIDGE_ENTER' events can be in either order - so wait for both to arrive before making a judgement on the LinkedID. This test can be added to any other bridging test to add a layer of LinkedId Propagation checking to it. This test has now been added to the existing bridge_action test rather than have a separate test for it. Diffs - /asterisk/trunk/tests/bridge/bridge_action/test-config.yaml 4675 /asterisk/trunk/tests/bridge/bridge_action/bridge_action.py 4675 /asterisk/trunk/lib/python/asterisk/linkedid_check.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3182/diff/ Testing --- This test fails spuriously due to what appears to be CEL events being sent on BRIDGE_ENTER prior to the LinkedId being updated. This bug will be fixed along with the other changes for 23120. 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] AMI and Sorcery
On Thu, Feb 13, 2014 at 5:38 AM, Joshua Colp jc...@digium.com wrote: On 14-02-12 03:26 AM, George Joseph wrote: In the process of creating the dialplan function AST_SORCERY (a companion to AST_CONFIG) I've run into an issue with how AMI interacts with sorcery particularly related to objects that can have multiple occurrences of the same field like contact in aor and match in identify. snip Notice the inconsistency? In the config, both contact and match are singular but in the output, Contacts is plural, Match is singular. The difference comes from how the 2 objects construct the AMI event. Contact has custom code that spits out the literal Contacts: then the list, whereas Identify uses the generic sorcery code which spits out the name of the field (hence the singular) and the list. If the fact that one is plural and the other is singular is a bug, tell me which is correct (hopefully the singular, see below) and you don't have to read further unless you've got some time to kill. If it's a feature, read on... The returned name should be the name of the field, thus contact. Yay! Background: On Monday I started work on the AST_SORCERY dialplan function which allows you to retrieve arbitrary fields from a sorcery based config like pjsip.conf. The function itself is quite simple. Basically, it calls ast_sorcery_objectset_create and iterates over the resulting ast_variable list to find the field of interest. Works great for single occurrence fields but as I found out, not at all for multiple occurrence fields like aor's contacts and identify's matches. As it turns out, the structure that defines a sorcery field does have a virtual function member for returning multiple occurrence fields as an ast_variable list but none of the objects implement it and besides, none of the ast_sorcery_object_field_register functions allow you to set it. In the end, without coding object-specific logic in AST_SORCERY, there was no way to retrieve the multiple occurrence fields because ast_sorcery_objectset_create wasn't returning them. The multiple_fields functionality is actually exposed, just only through the regex support. Well, I fixed all of that and now ast_sorcery_objectset_create can return an ast_variable list with as many entries for contact and match (and any others) as there are entries in the conf file. Works great...except for the AMI. I'm told that that an AMI event can't have multiple fields with the same name so multiple Contact or Match fields wouldn't work. Ok, you've always been able to register a handler that spits out a single string for multiple occurrences and you can now tell ast_sorcery_objectset_create which behavior you want. Except (to bring this back to the beginning), that would change the hard coded Contacts in the event to the generically generated Contact. :) Do you have unit tests for that? :D Wrote the tests first. :) One for the function, then updated the others. So, can I remove the s? Can I, Please?? Or I guess I can still use the generic code and do a search and replace in the AMI output buffer and add the s back in. I suppose the question really is... do we make this incompatible with the past. Exactly. This really wouldn't have been an issue if it was all internal but since the AMI is an external contract... Either way, this'll be good background info for why a patch to add 1 simple dialplan function is as complex as it is. Bonus Question: Why are aors and auths specified as a single comma-separated string where contacts and matches are specified on multiple lines? Hilarity, that's why. Okay, not really. No immediate answer springs to mind. -- _ -- 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] 3212: Makefile: fix so make main works without compiling deps (unless needed)
On Feb. 13, 2014, 3:55 p.m., Matt Jordan wrote: I'd say that we could consider this a 'bug' in the build system. We shouldn't be building things that don't need to be built. What do you think about merging this in in 1.8+? Fine with me. If I don't get any nays in the next 16hrs, that's what I'll do. - wdoekes --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3212/#review10870 --- On Feb. 12, 2014, 8:34 a.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3212/ --- (Updated Feb. 12, 2014, 8:34 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When I want to test that my svn-merged code works, I look at the diff and I check that the compilation succeeds. The quickest way is then to build only the subdirectory where the changed code resides. E.g. when changing something in res_config_pgsql.so: make res However, when something is changed in main, it will compile *all* the other dirs first. Making me wait. ifeq ($(findstring $(OSARCH), mingw32 cygwin ),) # Non-windows: # ensure that all module subdirectories are processed before 'main' during # a parallel build, since if there are modules selected to be embedded the # directories containing them must be completed before the main Asterisk # binary can be built main: $(filter-out main,$(MOD_SUBDIRS)) I doubt that more than 1% of the people uses embedded modules. Don't make the rest of us wait! This change adds a second check for the $(MENUSELECT_EMBED) variable: if it is empty, we do not add the dependency. Diffs - /trunk/Makefile 407969 Diff: https://reviewboard.asterisk.org/r/3212/diff/ Testing --- Ran a bunch of make, make clean, make distclean, make main, with and without module embedding selected. If any module is embedded, the dependency is enforced like before. If no module embedding is selected, main compiles directly. 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
[asterisk-dev] [Code Review] 3217: pjsip/subscribe/missing_aor: Fix test by using MessageWaiting AMI event
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3217/ --- Review request for Asterisk Developers and Kevin Harwell. Repository: testsuite Description --- The pjsip/subscribe/missing_aor has been failing consistently on the 32-bit build agent. Interestingly, it has been passing on the much slower 64-bit build agent (the CPU architecture here is a red herring - the 64-bit build agent is running on the crappiest hardware we have. On purpose. Slow things catch problems!) The reason why is due to how the test was written. Each step in the test is triggered on the success of the previous step, where the previous step was typically an AMI action execution. However, the AMI actions for updating external MWI occur when the Stasis message for modifying the mailbox state has been created and dispatched, not when the core has actually bothered to update the MWI. Hence, the test was running through its various AMI actions and then sending the SIP SUBSCRIBE request before all of the mailbox state had been updated. On the fast build agent, this typically failed. On the slow build agent, it typically succeeded due to Asterisk actually running faster than the test python script. This patch refactors the test to use the AMI event MessageWaiting to trigger the next stage. The AMI event is raised when the core itself has actually updated the mailbox, so the test knows for sure that the mailboxes are in the correct state before it subscribes. Diffs - /asterisk/trunk/tests/channels/pjsip/subscribe/missing_aor/run-test 4701 Diff: https://reviewboard.asterisk.org/r/3217/diff/ Testing --- 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] 3216: Add SIP User-Agent to contacts
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3216/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Fairly straightforward patch that reads the SIP User-Agent header from REGISTER requests and stores this value in all contacts that are being bound. Diffs - /branches/12/res/res_pjsip_registrar.c 407957 /branches/12/res/res_pjsip/location.c 407988 /branches/12/res/res_pjsip.c 407957 /branches/12/include/asterisk/res_pjsip.h 407957 /branches/12/contrib/ast-db-manage/config/versions/8b89128752a_add_user_agent.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3216/diff/ Testing --- See https://reviewboard.asterisk.org/r/3218/ 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
[asterisk-dev] [Code Review] 3218: PJSIP User Agent tests
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3218/ --- Review request for Asterisk Developers. Repository: testsuite Description --- Tests changes made in https://reviewboard.asterisk.org/r/3216/ Test description can be found in the test-config.yaml file on this review. Diffs - /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/user_agent/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/user_agent/sipp/alice-register.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/user_agent/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/tests.yaml 4701 Diff: https://reviewboard.asterisk.org/r/3218/diff/ Testing --- 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] 3218: PJSIP User Agent tests
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3218/#review10875 --- Ship it! Ship It! - Matt Jordan On Feb. 13, 2014, 12:07 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3218/ --- (Updated Feb. 13, 2014, 12:07 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- Tests changes made in https://reviewboard.asterisk.org/r/3216/ Test description can be found in the test-config.yaml file on this review. Diffs - /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/user_agent/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/user_agent/sipp/alice-register.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/user_agent/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/tests.yaml 4701 Diff: https://reviewboard.asterisk.org/r/3218/diff/ Testing --- 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] 3216: Add SIP User-Agent to contacts
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3216/#review10874 --- I'd go ahead and add a note to the UPGRADE file that the schema changed to support storing the user agent string. /branches/12/res/res_pjsip.c https://reviewboard.asterisk.org/r/3216/#comment20485 Similar to the path option, I would note that this value is stored from the registration, and is not in fact something that would typically be configured (and would be blown away if it was configured). - Matt Jordan On Feb. 13, 2014, 12:07 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3216/ --- (Updated Feb. 13, 2014, 12:07 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Fairly straightforward patch that reads the SIP User-Agent header from REGISTER requests and stores this value in all contacts that are being bound. Diffs - /branches/12/res/res_pjsip_registrar.c 407957 /branches/12/res/res_pjsip/location.c 407988 /branches/12/res/res_pjsip.c 407957 /branches/12/include/asterisk/res_pjsip.h 407957 /branches/12/contrib/ast-db-manage/config/versions/8b89128752a_add_user_agent.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3216/diff/ Testing --- See https://reviewboard.asterisk.org/r/3218/ 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] 3205: Remove PJSIP's MWI-specific function calls
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3205/ --- (Updated Feb. 13, 2014, 6:52 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Repository: Asterisk Description --- This is just to clear up something that's been bothering me for a while now. PJSIP provides an MWI-specific API that can be used to handle incoming MWI subscriptions. The way we are using it nets us pretty much no benefit at all, and it comes at the expense of having to have odd corner cases in our code for MWI subscriptions. This patch removes all of PJSIP's MWI library code in favor of generic PJSIP evsub code. This makes the pubsub core code much cleaner, and in my opinion, makes the res_pjsip_mwi.c code more concise. Diffs - /branches/12/res/res_pjsip_pubsub.c 407935 /branches/12/res/res_pjsip_mwi.c 407935 Diff: https://reviewboard.asterisk.org/r/3205/diff/ Testing --- Ran a manual test with a Digium phone that subscribes to MWI. Tested unsolicited MWI using the testsuite's MWI test. 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] 3217: pjsip/subscribe/missing_aor: Fix test by using MessageWaiting AMI event
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3217/#review10876 --- Ship it! Thanks for fixing this. I will take it off my todo list. - Kevin Harwell On Feb. 13, 2014, 11:11 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3217/ --- (Updated Feb. 13, 2014, 11:11 a.m.) Review request for Asterisk Developers and Kevin Harwell. Repository: testsuite Description --- The pjsip/subscribe/missing_aor has been failing consistently on the 32-bit build agent. Interestingly, it has been passing on the much slower 64-bit build agent (the CPU architecture here is a red herring - the 64-bit build agent is running on the crappiest hardware we have. On purpose. Slow things catch problems!) The reason why is due to how the test was written. Each step in the test is triggered on the success of the previous step, where the previous step was typically an AMI action execution. However, the AMI actions for updating external MWI occur when the Stasis message for modifying the mailbox state has been created and dispatched, not when the core has actually bothered to update the MWI. Hence, the test was running through its various AMI actions and then sending the SIP SUBSCRIBE request before all of the mailbox state had been updated. On the fast build agent, this typically failed. On the slow build agent, it typically succeeded due to Asterisk actually running faster than the test python script. This patch refactors the test to use the AMI event MessageWaiting to trigger the next stage. The AMI event is raised when the core itself has actually updated the mailbox, so the test knows for sure that the mailboxes are in the correct state before it subscribes. Diffs - /asterisk/trunk/tests/channels/pjsip/subscribe/missing_aor/run-test 4701 Diff: https://reviewboard.asterisk.org/r/3217/diff/ Testing --- 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] 3216: Add SIP User-Agent to contacts
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3216/ --- (Updated Feb. 13, 2014, 7:51 p.m.) Review request for Asterisk Developers. Changes --- Updated UPGRADE.txt and tweaked XML documentation as recommended. Repository: Asterisk Description --- Fairly straightforward patch that reads the SIP User-Agent header from REGISTER requests and stores this value in all contacts that are being bound. Diffs (updated) - /branches/12/res/res_pjsip_registrar.c 407957 /branches/12/res/res_pjsip/location.c 407988 /branches/12/res/res_pjsip.c 407957 /branches/12/include/asterisk/res_pjsip.h 407957 /branches/12/UPGRADE.txt 407957 Diff: https://reviewboard.asterisk.org/r/3216/diff/ Testing --- See https://reviewboard.asterisk.org/r/3218/ 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
[asterisk-dev] Co-Op students at Digium!
Hey all - Recently, we've had two co-op students join here at Digium on the software team working on Asterisk - Benjamin Ford (of Auburn University) and Scott Emidy (of Mississippi State University). Both have been learning Asterisk and the Asterisk Test Suite, and have been hard at work writing their first tests for the Asterisk Test Suite. They've now reached a point where their tests have to be put up for review, so it makes sense to let everyone know of their existence :-) Over the next few months, you'll see one of the folks here at Digium put up some code reviews on review board under our name, but written by either Ben or Scott. Currently, we're doing some QA on the reviews before they go up for public consumption, but as the two learn, that will most likely change as well. As you review the changes put up by Ben or Scott, please remember that like all of us, they're learning - and we all have to start learning somewhere. Thanks! Matt -- Matthew Jordan Digium, Inc. | Engineering Manager 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com http://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
[asterisk-dev] [Code Review] 3221: testsuite: Test for Marked/Normal(Unmarked) user Conference Interaction
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3221/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23198 https://issues.asterisk.org/jira/browse/ASTERISK-23198 Repository: testsuite Description --- This test was written by Scott Emidy. It continues the ConfBridge state machine tests. In this case, it checks the interactions between marked and normal (unmarked) users There are three scenarios: 1) Marked user enters first and leaves last. 2) Marked user enters first and normal user leaves last, which signifies that a MoH event will be played when the normal user is in a conference alone. 3) Normal user enters first and leaves last. Diffs - /asterisk/trunk/tests/apps/confbridge/tests.yaml 4701 /asterisk/trunk/tests/apps/confbridge/confbridge_marked_unmarked/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_marked_unmarked/configs/ast1/musiconhold.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_marked_unmarked/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_marked_unmarked/configs/ast1/confbridge.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_marked/configs/ast1/extensions.conf 4701 Diff: https://reviewboard.asterisk.org/r/3221/diff/ Testing --- 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] 3220: testsuite: Add basic tests for the DB function
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3220/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23187 https://issues.asterisk.org/jira/browse/ASTERISK-23187 Repository: testsuite Description --- This test was written by Ben Ford. This test verifies that the following AstDB functions and variables work as intended - DB, DB_RESULT, DB_EXISTS, DB_KEYS, DB_DELETE. The first iteration tests input and output using DB(family/key) as well as the DB_RESULT variable. The second iteration tests DB_EXISTS(family/key) with a non-existant and existant database, and also tests DB_KEYS(family) with a non-existant database, a family with one key, and a family with multiple keys. The last iteration tests DB_DELETE(family/key) on a non-existant database and an existant database. If it is called when there is nothing to delete, it will return an empty string. Diffs - /asterisk/trunk/tests/pbx/tests.yaml 4701 /asterisk/trunk/tests/pbx/ast_db/test-config.yaml PRE-CREATION /asterisk/trunk/tests/pbx/ast_db/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3220/diff/ Testing --- 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] 3191: channel uniqueid phase 1: convert string uniqueid values to structure with time
On Feb. 6, 2014, 4:23 p.m., rmudgett wrote:
/branches/12/res/res_stasis_snoop.c, lines 326-327
https://reviewboard.asterisk.org/r/3191/diff/1/?file=53594#file53594line326
This looks like another bug fix. You probably need to fix the line
wrapping length here also.
It wasn't inheriting the linkedid on channel creation. However, after
consulting with jcolp, it shouldn't be, so this was actually my bug.
- Scott
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/#review10811
---
On Feb. 13, 2014, 2:55 p.m., Scott Griepentrog wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/
---
(Updated Feb. 13, 2014, 2:55 p.m.)
Review request for Asterisk Developers.
Bugs: ASTERISK-23120
https://issues.asterisk.org/jira/browse/ASTERISK-23120
Repository: Asterisk
Description
---
This is the first phase of channel uniqueid changes for ASTERISK-23120.
* ast_channel_uniqueid structure replaces ast_string values uniqueid and
linkedid in channel structure
struct ast_channel_id {
char unique_id[AST_MAX_UNIQUEID]; /*! Unique Identifier - can be set
on originate */
time_t creation_time; /*! Creation time */
int creation_unique;/*! sub-second unique value */
}
* ast_channel_linkedid() and ast_channel_uniqueid() now return ptr to struct,
not char *
* all references to uniqueid linkedid updated to either pass entire
structure because full uniqueid with time must be propagated, or just the
-unique_id string element.
* an issue with argument order to ast_channel_alloc() in chan_mgcp.c was
corrected [BUGFIX].
* an issue with argument order to ast_channel_alloc() in chan_gtalk.c was
corrected [BUGFIX].
* there should be a slight performance improvement by removing the ast_string
handling of id's, but at the cost of +~250 bytes to the channel structure.
* defines for AST_MAX_UNIQUEID (channel.h) and MAX_CHANNEL_ID (rtp_engine.h)
have been changed from 150 to 128 to reduce structure alignment issues (and
also just because 150 is ridiculously large)
Diffs
-
/branches/12/tests/test_substitution.c 408019
/branches/12/tests/test_cel.c 408019
/branches/12/tests/test_cdr.c 408019
/branches/12/res/stasis/control.c 408019
/branches/12/res/stasis/app.c 408019
/branches/12/res/snmp/agent.c 408019
/branches/12/res/res_stasis_snoop.c 408019
/branches/12/res/res_stasis_recording.c 408019
/branches/12/res/res_stasis_playback.c 408019
/branches/12/res/res_stasis.c 408019
/branches/12/res/res_pjsip_refer.c 408019
/branches/12/res/res_musiconhold.c 408019
/branches/12/res/res_monitor.c 408019
/branches/12/res/res_fax.c 408019
/branches/12/res/res_agi.c 408019
/branches/12/res/parking/parking_bridge_features.c 408019
/branches/12/res/parking/parking_applications.c 408019
/branches/12/res/ari/resource_channels.c 408019
/branches/12/main/stasis_channels.c 408019
/branches/12/main/stasis_bridges.c 408019
/branches/12/main/pbx.c 408019
/branches/12/main/manager.c 408019
/branches/12/main/features.c 408019
/branches/12/main/endpoints.c 408019
/branches/12/main/core_unreal.c 408019
/branches/12/main/channel_internal_api.c 408019
/branches/12/main/channel.c 408019
/branches/12/main/cel.c 408019
/branches/12/main/bridge_channel.c 408019
/branches/12/include/asterisk/rtp_engine.h 408019
/branches/12/include/asterisk/channel_internal.h 408019
/branches/12/include/asterisk/channel.h 408019
/branches/12/funcs/func_channel.c 408019
/branches/12/channels/chan_unistim.c 408019
/branches/12/channels/chan_skinny.c 408019
/branches/12/channels/chan_sip.c 408019
/branches/12/channels/chan_pjsip.c 408019
/branches/12/channels/chan_phone.c 408019
/branches/12/channels/chan_oss.c 408019
/branches/12/channels/chan_multicast_rtp.c 408019
/branches/12/channels/chan_motif.c 408019
/branches/12/channels/chan_mgcp.c 408019
/branches/12/channels/chan_jingle.c 408019
/branches/12/channels/chan_iax2.c 408019
/branches/12/channels/chan_gtalk.c 408019
/branches/12/channels/chan_console.c 408019
/branches/12/channels/chan_alsa.c 408019
/branches/12/apps/app_voicemail.c 408019
/branches/12/apps/app_queue.c 408019
/branches/12/apps/app_minivm.c 408019
/branches/12/apps/app_followme.c 408019
/branches/12/apps/app_dumpchan.c 408019
/branches/12/apps/app_confbridge.c 408019
/branches/12/apps/app_chanspy.c 408019
/branches/12/addons/chan_ooh323.c 408019
Re: [asterisk-dev] [Code Review] 3191: channel uniqueid phase 1: convert string uniqueid values to structure with time
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/
---
(Updated Feb. 13, 2014, 2:55 p.m.)
Review request for Asterisk Developers.
Changes
---
Updated code per review comments
Bugs: ASTERISK-23120
https://issues.asterisk.org/jira/browse/ASTERISK-23120
Repository: Asterisk
Description (updated)
---
This is the first phase of channel uniqueid changes for ASTERISK-23120.
* ast_channel_uniqueid structure replaces ast_string values uniqueid and
linkedid in channel structure
struct ast_channel_id {
char unique_id[AST_MAX_UNIQUEID]; /*! Unique Identifier - can be set
on originate */
time_t creation_time; /*! Creation time */
int creation_unique;/*! sub-second unique value */
}
* ast_channel_linkedid() and ast_channel_uniqueid() now return ptr to struct,
not char *
* all references to uniqueid linkedid updated to either pass entire structure
because full uniqueid with time must be propagated, or just the -unique_id
string element.
* an issue with argument order to ast_channel_alloc() in chan_mgcp.c was
corrected [BUGFIX].
* an issue with argument order to ast_channel_alloc() in chan_gtalk.c was
corrected [BUGFIX].
* there should be a slight performance improvement by removing the ast_string
handling of id's, but at the cost of +~250 bytes to the channel structure.
* defines for AST_MAX_UNIQUEID (channel.h) and MAX_CHANNEL_ID (rtp_engine.h)
have been changed from 150 to 128 to reduce structure alignment issues (and
also just because 150 is ridiculously large)
Diffs (updated)
-
/branches/12/tests/test_substitution.c 408019
/branches/12/tests/test_cel.c 408019
/branches/12/tests/test_cdr.c 408019
/branches/12/res/stasis/control.c 408019
/branches/12/res/stasis/app.c 408019
/branches/12/res/snmp/agent.c 408019
/branches/12/res/res_stasis_snoop.c 408019
/branches/12/res/res_stasis_recording.c 408019
/branches/12/res/res_stasis_playback.c 408019
/branches/12/res/res_stasis.c 408019
/branches/12/res/res_pjsip_refer.c 408019
/branches/12/res/res_musiconhold.c 408019
/branches/12/res/res_monitor.c 408019
/branches/12/res/res_fax.c 408019
/branches/12/res/res_agi.c 408019
/branches/12/res/parking/parking_bridge_features.c 408019
/branches/12/res/parking/parking_applications.c 408019
/branches/12/res/ari/resource_channels.c 408019
/branches/12/main/stasis_channels.c 408019
/branches/12/main/stasis_bridges.c 408019
/branches/12/main/pbx.c 408019
/branches/12/main/manager.c 408019
/branches/12/main/features.c 408019
/branches/12/main/endpoints.c 408019
/branches/12/main/core_unreal.c 408019
/branches/12/main/channel_internal_api.c 408019
/branches/12/main/channel.c 408019
/branches/12/main/cel.c 408019
/branches/12/main/bridge_channel.c 408019
/branches/12/include/asterisk/rtp_engine.h 408019
/branches/12/include/asterisk/channel_internal.h 408019
/branches/12/include/asterisk/channel.h 408019
/branches/12/funcs/func_channel.c 408019
/branches/12/channels/chan_unistim.c 408019
/branches/12/channels/chan_skinny.c 408019
/branches/12/channels/chan_sip.c 408019
/branches/12/channels/chan_pjsip.c 408019
/branches/12/channels/chan_phone.c 408019
/branches/12/channels/chan_oss.c 408019
/branches/12/channels/chan_multicast_rtp.c 408019
/branches/12/channels/chan_motif.c 408019
/branches/12/channels/chan_mgcp.c 408019
/branches/12/channels/chan_jingle.c 408019
/branches/12/channels/chan_iax2.c 408019
/branches/12/channels/chan_gtalk.c 408019
/branches/12/channels/chan_console.c 408019
/branches/12/channels/chan_alsa.c 408019
/branches/12/apps/app_voicemail.c 408019
/branches/12/apps/app_queue.c 408019
/branches/12/apps/app_minivm.c 408019
/branches/12/apps/app_followme.c 408019
/branches/12/apps/app_dumpchan.c 408019
/branches/12/apps/app_confbridge.c 408019
/branches/12/apps/app_chanspy.c 408019
/branches/12/addons/chan_ooh323.c 408019
/branches/12/addons/chan_mobile.c 408019
Diff: https://reviewboard.asterisk.org/r/3191/diff/
Testing
---
Ran new linkedid_check test and received same results. Also ran some bridge
tests to check for asserts.
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] 3191: channel uniqueid phase 1: convert string uniqueid values to structure with time
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/#review10877
---
Is changing AST_MAX_UNIQUEID and MAX_CHANNEL_ID from 150 really a good idea?
The length was potentialy dependent upon the choice of the system name. With
the impending change of the user choosing the channel uniqueid value it could
be longer. Users have been known to pack other information into strings.
/branches/12/include/asterisk/channel.h
https://reviewboard.asterisk.org/r/3191/#comment20486
...purposes of call...
/branches/12/main/cel.c
https://reviewboard.asterisk.org/r/3191/#comment20487
Use sizeof(channelid.unique_id) instead of AST_MAX_UNIQUEID.
/branches/12/main/cel.c
https://reviewboard.asterisk.org/r/3191/#comment20488
Use sizeof(channelid.unique_id) instead of AST_MAX_UNIQUEID.
/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3191/#comment20489
Parentheses are not necessary.
tmp_id = *ast_channel_uniqueid(clonechan);
/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3191/#comment20490
Parenthese are not neede here also.
/branches/12/res/res_stasis_snoop.c
https://reviewboard.asterisk.org/r/3191/#comment20491
This really should be using ast_copy_string().
It was most definitely not safe before this patch and this module should
not be assuming that uniqueid is going to fit.
- rmudgett
On Feb. 13, 2014, 2:55 p.m., Scott Griepentrog wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/
---
(Updated Feb. 13, 2014, 2:55 p.m.)
Review request for Asterisk Developers.
Bugs: ASTERISK-23120
https://issues.asterisk.org/jira/browse/ASTERISK-23120
Repository: Asterisk
Description
---
This is the first phase of channel uniqueid changes for ASTERISK-23120.
* ast_channel_uniqueid structure replaces ast_string values uniqueid and
linkedid in channel structure
struct ast_channel_id {
char unique_id[AST_MAX_UNIQUEID]; /*! Unique Identifier - can be set
on originate */
time_t creation_time; /*! Creation time */
int creation_unique;/*! sub-second unique value */
}
* ast_channel_linkedid() and ast_channel_uniqueid() now return ptr to struct,
not char *
* all references to uniqueid linkedid updated to either pass entire
structure because full uniqueid with time must be propagated, or just the
-unique_id string element.
* an issue with argument order to ast_channel_alloc() in chan_mgcp.c was
corrected [BUGFIX].
* an issue with argument order to ast_channel_alloc() in chan_gtalk.c was
corrected [BUGFIX].
* there should be a slight performance improvement by removing the ast_string
handling of id's, but at the cost of +~250 bytes to the channel structure.
* defines for AST_MAX_UNIQUEID (channel.h) and MAX_CHANNEL_ID (rtp_engine.h)
have been changed from 150 to 128 to reduce structure alignment issues (and
also just because 150 is ridiculously large)
Diffs
-
/branches/12/tests/test_substitution.c 408019
/branches/12/tests/test_cel.c 408019
/branches/12/tests/test_cdr.c 408019
/branches/12/res/stasis/control.c 408019
/branches/12/res/stasis/app.c 408019
/branches/12/res/snmp/agent.c 408019
/branches/12/res/res_stasis_snoop.c 408019
/branches/12/res/res_stasis_recording.c 408019
/branches/12/res/res_stasis_playback.c 408019
/branches/12/res/res_stasis.c 408019
/branches/12/res/res_pjsip_refer.c 408019
/branches/12/res/res_musiconhold.c 408019
/branches/12/res/res_monitor.c 408019
/branches/12/res/res_fax.c 408019
/branches/12/res/res_agi.c 408019
/branches/12/res/parking/parking_bridge_features.c 408019
/branches/12/res/parking/parking_applications.c 408019
/branches/12/res/ari/resource_channels.c 408019
/branches/12/main/stasis_channels.c 408019
/branches/12/main/stasis_bridges.c 408019
/branches/12/main/pbx.c 408019
/branches/12/main/manager.c 408019
/branches/12/main/features.c 408019
/branches/12/main/endpoints.c 408019
/branches/12/main/core_unreal.c 408019
/branches/12/main/channel_internal_api.c 408019
/branches/12/main/channel.c 408019
/branches/12/main/cel.c 408019
/branches/12/main/bridge_channel.c 408019
/branches/12/include/asterisk/rtp_engine.h 408019
/branches/12/include/asterisk/channel_internal.h 408019
/branches/12/include/asterisk/channel.h 408019
/branches/12/funcs/func_channel.c 408019
/branches/12/channels/chan_unistim.c 408019
/branches/12/channels/chan_skinny.c 408019
/branches/12/channels/chan_sip.c
Re: [asterisk-dev] [Code Review] 3191: channel uniqueid phase 1: convert string uniqueid values to structure with time
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/#review10878
---
chan_dahdi doesn't compile.
chan_h323 doesn't compile.
chan_nbs doesn't compile.
chan_vpb doesn't compile.
app_meetme doesn't compile.
- rmudgett
On Feb. 13, 2014, 2:55 p.m., Scott Griepentrog wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3191/
---
(Updated Feb. 13, 2014, 2:55 p.m.)
Review request for Asterisk Developers.
Bugs: ASTERISK-23120
https://issues.asterisk.org/jira/browse/ASTERISK-23120
Repository: Asterisk
Description
---
This is the first phase of channel uniqueid changes for ASTERISK-23120.
* ast_channel_uniqueid structure replaces ast_string values uniqueid and
linkedid in channel structure
struct ast_channel_id {
char unique_id[AST_MAX_UNIQUEID]; /*! Unique Identifier - can be set
on originate */
time_t creation_time; /*! Creation time */
int creation_unique;/*! sub-second unique value */
}
* ast_channel_linkedid() and ast_channel_uniqueid() now return ptr to struct,
not char *
* all references to uniqueid linkedid updated to either pass entire
structure because full uniqueid with time must be propagated, or just the
-unique_id string element.
* an issue with argument order to ast_channel_alloc() in chan_mgcp.c was
corrected [BUGFIX].
* an issue with argument order to ast_channel_alloc() in chan_gtalk.c was
corrected [BUGFIX].
* there should be a slight performance improvement by removing the ast_string
handling of id's, but at the cost of +~250 bytes to the channel structure.
* defines for AST_MAX_UNIQUEID (channel.h) and MAX_CHANNEL_ID (rtp_engine.h)
have been changed from 150 to 128 to reduce structure alignment issues (and
also just because 150 is ridiculously large)
Diffs
-
/branches/12/tests/test_substitution.c 408019
/branches/12/tests/test_cel.c 408019
/branches/12/tests/test_cdr.c 408019
/branches/12/res/stasis/control.c 408019
/branches/12/res/stasis/app.c 408019
/branches/12/res/snmp/agent.c 408019
/branches/12/res/res_stasis_snoop.c 408019
/branches/12/res/res_stasis_recording.c 408019
/branches/12/res/res_stasis_playback.c 408019
/branches/12/res/res_stasis.c 408019
/branches/12/res/res_pjsip_refer.c 408019
/branches/12/res/res_musiconhold.c 408019
/branches/12/res/res_monitor.c 408019
/branches/12/res/res_fax.c 408019
/branches/12/res/res_agi.c 408019
/branches/12/res/parking/parking_bridge_features.c 408019
/branches/12/res/parking/parking_applications.c 408019
/branches/12/res/ari/resource_channels.c 408019
/branches/12/main/stasis_channels.c 408019
/branches/12/main/stasis_bridges.c 408019
/branches/12/main/pbx.c 408019
/branches/12/main/manager.c 408019
/branches/12/main/features.c 408019
/branches/12/main/endpoints.c 408019
/branches/12/main/core_unreal.c 408019
/branches/12/main/channel_internal_api.c 408019
/branches/12/main/channel.c 408019
/branches/12/main/cel.c 408019
/branches/12/main/bridge_channel.c 408019
/branches/12/include/asterisk/rtp_engine.h 408019
/branches/12/include/asterisk/channel_internal.h 408019
/branches/12/include/asterisk/channel.h 408019
/branches/12/funcs/func_channel.c 408019
/branches/12/channels/chan_unistim.c 408019
/branches/12/channels/chan_skinny.c 408019
/branches/12/channels/chan_sip.c 408019
/branches/12/channels/chan_pjsip.c 408019
/branches/12/channels/chan_phone.c 408019
/branches/12/channels/chan_oss.c 408019
/branches/12/channels/chan_multicast_rtp.c 408019
/branches/12/channels/chan_motif.c 408019
/branches/12/channels/chan_mgcp.c 408019
/branches/12/channels/chan_jingle.c 408019
/branches/12/channels/chan_iax2.c 408019
/branches/12/channels/chan_gtalk.c 408019
/branches/12/channels/chan_console.c 408019
/branches/12/channels/chan_alsa.c 408019
/branches/12/apps/app_voicemail.c 408019
/branches/12/apps/app_queue.c 408019
/branches/12/apps/app_minivm.c 408019
/branches/12/apps/app_followme.c 408019
/branches/12/apps/app_dumpchan.c 408019
/branches/12/apps/app_confbridge.c 408019
/branches/12/apps/app_chanspy.c 408019
/branches/12/addons/chan_ooh323.c 408019
/branches/12/addons/chan_mobile.c 408019
Diff: https://reviewboard.asterisk.org/r/3191/diff/
Testing
---
Ran new linkedid_check test and received same results. Also ran some bridge
tests to check for asserts.
Thanks,
Scott Griepentrog
--
[asterisk-dev] Attended transfers and MOH
Greetings, There is a bug currently in Asterisk that essentially boils down to MOH not being [re]started for an attended transfered call when the transferor was listening to it. For example, here is a specific scenario (issue ASTERISK-19499): Alice calls Bob, Bob starts a SIP attended transfer into a confbridge that is set to play music when empty. Alice hears music since she is now on hold and Bob hears music from the confbridge. Bob then completes the transfer and then Alice enters the confbridge, but hears no music. This situation occurs because during the transfer and channel masquerade the application has no way to know the channel was swapped out. There are a couple of ways to approach a fix for this: 1) Use a channel fixup in the applications that are affected. A small change to the do masquerade function has to be made to get this to work too. 2) In the relevant channel drivers when an attended transfer occurs check to see if MOH is currently playing on the transferor. If so, once the transfer occurs start MOH on the transferee. This idea was put forth by Olle Johansson (currently implemented in his rana-moh-queue-transfer-1.8 team branch). Albeit a few changes will have to be made in order to use it in the main branches, but the idea will essentially remain. I like option 2 as it seems to be a bit less intrusive (no changes to masquerades and such). Also in 12+ the logic can be put into the bridge transfer code reducing the changes further. Thoughts? Ideas? Something I have missed or need to be aware of? I'll start to move forward with #2 soon and put it up for review. Comments and feedback always welcome. Thanks! -- Kevin Harwell Digium, Inc. | Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com http://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
[asterisk-dev] [Code Review] 3222: pbx: If someone managed to get Asterisk to load with no dialplan at all, survive
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3222/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23297 https://issues.asterisk.org/jira/browse/ASTERISK-23297 Repository: Asterisk Description --- Currently, if you load Asterisk with an empty extensions.conf, and you noload all other pbx modules (such as pbx_ael), then loading a later module - such as res_parking - that attempts to add something to the dialplan will explode. This is because ast_merge_contexts_and_delete assumes there is at least one context loaded. This patch modifies the routine to simply set the contexts_table/contexts pointers to the merged entries if they don't exist yet. Note that this is probably a problem in 1.8/11, however, it probably never occurred due to features.c creating the parking extensions. Now that the core no longer does that, it's slightly more plausible to end up in this situation. It still wouldn't hurt to apply this patch to 1.8/11 as well. Diffs - ./branches/12/main/pbx.c 407935 Diff: https://reviewboard.asterisk.org/r/3222/diff/ Testing --- The slightly contrived scenario no longer crashes. 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] 3202: install_subst: helper script for installing with path substitution
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3202/#review10879 --- Ship it! Ship It! - Corey Farrell On Feb. 10, 2014, 1:46 p.m., Tzafrir Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3202/ --- (Updated Feb. 10, 2014, 1:46 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- The Makefile repeats several time installing some scripts with paths (as denoted in __special name__) substituted with their actual values. Substituting this, along with handling the errors correctly, is something that should not be repeated again and again in the makefile. This commit moves it to a single script: ./build_tools/install_subst [-d] src dst It installs src to dst but first substitutes some variables in it. -d (data) installs it as a data file (-m644). Otherwise it is an executable (install's default permissions). I want this in order to avoid having yet another copy of the above for the systemd unit file (review 3062). Diffs - /trunk/build_tools/install_subst PRE-CREATION /trunk/Makefile 407855 Diff: https://reviewboard.asterisk.org/r/3202/diff/ Testing --- Thanks, Tzafrir Cohen -- _ -- 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] 3206: HEP: Add test for PJSIP HEP packet capture
On Feb. 11, 2014, 10:54 a.m., Mark Michelson wrote: I think it's worth it to create an IPv6 test in addition to the IPv4 test you've created here. In retrospect, I should have something that sends an authkey as well. I'll add both. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3206/#review10852 --- On Feb. 11, 2014, 6:22 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3206/ --- (Updated Feb. 11, 2014, 6:22 a.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This patch adds the following: * A pluggable module that emulates a HEP capture server for verifying packets. * A function that can, in a rudimentary fashion, verify SIP packets. It's simplistic, but good enough to verify that Asterisk sent the server the SIP message traffic we sent to it from SIPp. * A test that verifies that the res_hep/res_hep_pjsip modules forward SIP message traffic to our little capture server. Diffs - /asterisk/trunk/tests/tests.yaml 4685 /asterisk/trunk/tests/hep/tests.yaml PRE-CREATION /asterisk/trunk/tests/hep/pjsip/test-config.yaml PRE-CREATION /asterisk/trunk/tests/hep/pjsip/sipp/echo_with_deferred_sdp.xml PRE-CREATION /asterisk/trunk/tests/hep/pjsip/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/hep/pjsip/configs/ast1/hep.conf PRE-CREATION /asterisk/trunk/tests/hep/pjsip/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/hep/hep_capture_node.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3206/diff/ Testing --- 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] 3207: HEP: Add a Homer Encapsulation Protocol (HEP) v3 capture agent module and a packet logger for PJSIP
On Feb. 11, 2014, 10:06 a.m., Mark Michelson wrote: /branches/12/include/asterisk/res_hep.h, lines 76-78 https://reviewboard.asterisk.org/r/3207/diff/2/?file=53825#file53825line76 I don't understand this line. Does this refer to the payload parameter to this function? If so, then reword this to be more explicit. If not, then...I have no idea what this means. It's the payload and the uuid, really. It's saying that the hepv3_capture_info object that is returned from this function has pointers to things. If you point those things to something on the stack, that's a bad idea, and you're going to have a bad time. I'll clarify. On Feb. 11, 2014, 10:06 a.m., Mark Michelson wrote: /branches/12/res/res_hep.c, lines 92-93 https://reviewboard.asterisk.org/r/3207/diff/2/?file=53826#file53826line92 This is a very strange default for the HEP server. I suspect the port is common, but the IP address is making an odd assumption. I agree, it was in Alexandr's original code and I kept it. I'll set the value to , and if we don't have an IP address, the way the option is registered will cause the module load to fail. On Feb. 11, 2014, 10:06 a.m., Mark Michelson wrote: /branches/12/res/res_hep.c, lines 197-200 https://reviewboard.asterisk.org/r/3207/diff/2/?file=53826#file53826line197 I think there's an issue with this and the hep_chunk_payload structure. If this is expected to be sent over the wire, then you can't send a pointer. You need to have a buffer the size of the chunk's length field and copy the data into that buffer. EDIT: After looking further at the code, it appears that this and hep_chunk_payload aren't actually being used. The payload is being sent by manually placing a hep_chunk and then the payload onto the socket buffer. To discourage the use of hep_chunk_str and hep_chunk_payload, you should probably just get rid of these structures. Yup, I'll just remove them. Some of the other HEP data items (not HEPv3) are also leftovers; I'll remove them as well. On Feb. 11, 2014, 10:06 a.m., Mark Michelson wrote: /branches/12/res/res_hep.c, lines 431-435 https://reviewboard.asterisk.org/r/3207/diff/2/?file=53826#file53826line431 I believe you're leaking capture_info here. Given the multitude of return paths, RAII_VAR is probably a good fit for it here. Egads. Fixed. On Feb. 11, 2014, 10:06 a.m., Mark Michelson wrote: /branches/12/res/res_hep_pjsip.c, lines 62-68 https://reviewboard.asterisk.org/r/3207/diff/2/?file=53828#file53828line62 Is this the intended use of the UUID in capture info? The same Call-ID will be repeated over many packets. (Same comment applies for the logging_on_rx_msg function) Yes, that's the intended purpose. It's used to tie messages together, not necessarily to give each individual message a unique identifier. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3207/#review10851 --- On Feb. 11, 2014, 6:21 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3207/ --- (Updated Feb. 11, 2014, 6:21 a.m.) Review request for Asterisk Developers, Joshua Colp and Olle E Johansson. Repository: Asterisk Description --- This patch adds the following: (1) A new module, res_hep, which implements a generic packet capture agent for the Homer Encapsulation Protocol (HEP) version 3. Note that this code is heavily based on a patch provided by Alexandr Dubovikov; I basically just wrapped it up, added configuration via the configuration framework, and threw in a taskprocessor. (2) A new module, res_hep_pjsip, which performs packet capturing for the PJSIP SIP stack. This is one of those modules that I think really showcases how nice the new stack is - we're able to add a new module that inserts itself into the stack and forwards the message traffic off to the res_hep module without modifying the core parts of the stack itself. This means a system administrator could load this at will on certain Asterisk systems and - if the capturing isn't needed - unload it and keep the stack 'slim'. A few notes: * This code exists in the following branch: http://svn.asterisk.org/svn/asterisk/team/mjordan/12-hep * The code in the branch also contains a module for RTCP. While that actually *does* send RTCP information over HEP, it does so as a JSON blob, which is not super useful. It's an open question as to what the formatting should be, i.e., a SNOM-esque encoding, RFC 6035, etc. I'm open to suggestions on this, which is why I deferred that functionality for a
Re: [asterisk-dev] [Code Review] 3207: HEP: Add a Homer Encapsulation Protocol (HEP) v3 capture agent module and a packet logger for PJSIP
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3207/ --- (Updated Feb. 13, 2014, 9:58 p.m.) Review request for Asterisk Developers, Joshua Colp and Olle E Johansson. Changes --- Addressed Mark's findings. Repository: Asterisk Description --- This patch adds the following: (1) A new module, res_hep, which implements a generic packet capture agent for the Homer Encapsulation Protocol (HEP) version 3. Note that this code is heavily based on a patch provided by Alexandr Dubovikov; I basically just wrapped it up, added configuration via the configuration framework, and threw in a taskprocessor. (2) A new module, res_hep_pjsip, which performs packet capturing for the PJSIP SIP stack. This is one of those modules that I think really showcases how nice the new stack is - we're able to add a new module that inserts itself into the stack and forwards the message traffic off to the res_hep module without modifying the core parts of the stack itself. This means a system administrator could load this at will on certain Asterisk systems and - if the capturing isn't needed - unload it and keep the stack 'slim'. A few notes: * This code exists in the following branch: http://svn.asterisk.org/svn/asterisk/team/mjordan/12-hep * The code in the branch also contains a module for RTCP. While that actually *does* send RTCP information over HEP, it does so as a JSON blob, which is not super useful. It's an open question as to what the formatting should be, i.e., a SNOM-esque encoding, RFC 6035, etc. I'm open to suggestions on this, which is why I deferred that functionality for a later review. * Much thanks to Alexandr for his Asterisk patch for this code and for a *lot* of patience waiting for me to port it to 12/trunk. Due to some dithering on my part, this has taken the better part of a year to port forward (I still blame CDRs for the delay). Diffs (updated) - /branches/12/res/res_hep_pjsip.c PRE-CREATION /branches/12/res/res_hep.exports.in PRE-CREATION /branches/12/res/res_hep.c PRE-CREATION /branches/12/include/asterisk/res_hep.h PRE-CREATION /branches/12/configs/hep.conf.sample PRE-CREATION /branches/12/CHANGES 407945 Diff: https://reviewboard.asterisk.org/r/3207/diff/ Testing --- An automated test that emulates a SIP capture server was written and is up for review here: https://reviewboard.asterisk.org/r/3206 This admittedly needs some *real* testing, as I have yet to stand up Kamailio with HEP. I think the code is far enough along to get some eyes on it however. 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] Attended transfers and MOH
On 14 Feb 2014, at 00:43, Kevin Harwell kharw...@digium.com wrote: Greetings, There is a bug currently in Asterisk that essentially boils down to MOH not being [re]started for an attended transfered call when the transferor was listening to it. For example, here is a specific scenario (issue ASTERISK-19499): Alice calls Bob, Bob starts a SIP attended transfer into a confbridge that is set to play music when empty. Alice hears music since she is now on hold and Bob hears music from the confbridge. Bob then completes the transfer and then Alice enters the confbridge, but hears no music. This situation occurs because during the transfer and channel masquerade the application has no way to know the channel was swapped out. There are a couple of ways to approach a fix for this: 1) Use a channel fixup in the applications that are affected. A small change to the do masquerade function has to be made to get this to work too. 2) In the relevant channel drivers when an attended transfer occurs check to see if MOH is currently playing on the transferor. If so, once the transfer occurs start MOH on the transferee. This idea was put forth by Olle Johansson (currently implemented in his rana-moh-queue-transfer-1.8 team branch). Albeit a few changes will have to be made in order to use it in the main branches, but the idea will essentially remain. I like option 2 as it seems to be a bit less intrusive (no changes to masquerades and such). Also in 12+ the logic can be put into the bridge transfer code reducing the changes further. Thoughts? Ideas? Something I have missed or need to be aware of? I'll start to move forward with #2 soon and put it up for review. Comments and feedback always welcome. CHeck this branch: http://svnview.digium.com/svn/asterisk/team/oej/rana-moh-queue-transfer-1.8/README.rana-moh-queue-transfer.txt?revision=393703 /O -- _ -- 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
