----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3105/#review10523 -----------------------------------------------------------
Ship it! Your findings regarding the to-tag with chan_sip.c caused me to have a look at the code to see why the old tests were passing. Dialog-matching in chan_sip is...weird. For BYE, INFO, and ACK requests, the lack of a to-tag results in an immediate rejection of the request. For other types of requests, we can match dialogs if there is no to-tag; however, we will not match an existing dialog if the incoming request contains a to-tag that does not match our local tag. This means that the chan_sip SIPp scenarios worked, even though the reinvites should have contained to-tags. The changes you made here for the pjsip tests could be done to the chan_sip tests, but I don't think that it's a high-priority change to make. - Mark Michelson On Jan. 6, 2014, 9:59 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3105/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2014, 9:59 p.m.) > > > Review request for Asterisk Developers, Joshua Colp, Matt Jordan, and Mark > Michelson. > > > Repository: testsuite > > > Description > ------- > > Creates tests similar to the channels/SIP/sip_hold tests. Using the PJSIP > channel driver, the following scenarios are tested: > > 1. Put a call on hold by setting media attributes to sendonly. Unhold by > setting media back to sendrecv > 2. (new) as above, but unhold by simply not including an SDP (some devices > are known to do this apparently and a patch is on reviewboard to handle that > scenario in PJSIP). > 3. Set on hold by setting the contact IP to 0.0.0.0, return to normal by > providing normal SDP > 4. Combine both the IP hold and media restriction hold methods. > > There are a few noteworthy differences here to the original tests in SIP hold > that might require some additional attention. > > First, simply running the SIPP scenarios as is with PJSIP yielded an > inability to match the new INVITES to the existing PJSIP dialogs. In addition > to one invite from each scenario appearing to point to the wrong peer in the > invite used to resume/unhold the call, the appropriate To tag was left out of > the reinvites for both holding and unholding. I'm not sure how this worked in > the first place. > > Second, I noticed that verifying for hold/unhold behavior in the test script > is performed by checking for Music On Hold start and stop events. This > mostly works for hold, but for unhold in particular it's unreliable since > music on hold stop events will also be issued if the call simply ends. > Because of that, I went ahead and added listeners for Hold and Unhold events > which are required in a similar fashion to the existing MOH start and stop > events. > > I'm willing to apply these changes to the SIP hold test, but this does appear > to be a noteworthy variance in behavior which needs to be taken note of... > particularly the differences in the SIPP scenarios that needed to be made. > > > Diffs > ----- > > /asterisk/trunk/tests/channels/pjsip/tests.yaml 4539 > /asterisk/trunk/tests/channels/pjsip/hold/test-config.yaml PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_unhold_sans_sdp.xml > PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_media_restrict.xml > PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_restrict.xml > PRE-CREATION > > /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_media_restrict.xml > PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_A.xml PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/sipp/inject.csv PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/run-test PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/pjsip.conf > PRE-CREATION > /asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/extensions.conf > PRE-CREATION > > Diff: https://reviewboard.asterisk.org/r/3105/diff/ > > > Testing > ------- > > Ran the tests. Checked output of user events in Asterisk. Confirmed that > without the patch in 3106 that this test fails due to not receiving an Unhold > event for the second scenario described above. > > > 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
