[asterisk-dev] Change in testsuite[master]: sip_attended_transfer now supports pre-12 Asterisk versions.

Matt Jordan (Code Review) asteriskteam at digium.com
Tue Apr 7 14:48:23 CDT 2015


Matt Jordan has submitted this change and it was merged.

Change subject: sip_attended_transfer now supports pre-12 Asterisk versions.
......................................................................


sip_attended_transfer now supports pre-12 Asterisk versions.

The sip_attended transfer test was recently rewritten to prevent it from
bouncing during automatic test runs. The rewrite attempted to break into
two tests in an attempt to separate the logic of different versions from
one another.

Ashley pointed out on my original Asterisk 11 version of the patch that
there are only small difference between the Asterisk 11 and 12 versions
of the test, resulting in a lot of repeated boilerplate code that could
otherwise be avoided.

This change alters the 12+ specific test by separating the bridge logic
for different versions into their own classes. I have verified that the
test passes using both Asterisk 11 and Asterisk 13.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Fix a typo where a function call was evaluated as a boolean.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Address review feedback from Ashley and Matt

* Make efforts to make pylint less angry
* Add debugging to the 11 version since it's weird
* Changed some variable names. I went with the suggested names in some
  case, but in others I did not.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
---
M tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
2 files changed, 199 insertions(+), 45 deletions(-)

Approvals:
  Matt Jordan: Looks good to me, approved; Verified
  Ashley Sanders: Looks good to me, but someone else must approve



diff --git a/tests/channels/SIP/sip_attended_transfer/attended_transfer.py b/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
index cb133f3..ddf8736 100644
--- a/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
+++ b/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
@@ -8,7 +8,11 @@
 
 import logging
 import pjsua as pj
+import sys
 
+sys.path.append('lib/python/asterisk')
+
+from version import AsteriskVersion
 from twisted.internet import reactor
 
 LOGGER = logging.getLogger(__name__)
@@ -52,8 +56,8 @@
             reactor.callFromThread(self.on_answered)
 
 
-class BridgeState(object):
-    '''Object for tracking state of a bridge
+class BridgeTwelve(object):
+    '''Object for tracking attributes of an Asterisk 12+ bridge
 
     The main data the test cares about is the bridge's unique id and whether two
     channels have been bridged together by the bridge.
@@ -61,6 +65,159 @@
     def __init__(self):
         self.unique_id = None
         self.bridged = False
+
+
+class BridgeStateTwelve(object):
+    '''Tracker of Bridge State for Asterisk 12+
+
+    Since Asterisk 12+ has the concept of Bridge objects, this tracks the
+    bridges by detecting when they are created. Once bridges are created, we
+    determine that channels are bridged when BridgeEnter events indicate that
+    two channels are present.
+    '''
+    def __init__(self, test_object, controller, ami):
+        self.test_object = test_object
+        self.controller = controller
+        self.ami = ami
+        self.bridge1 = BridgeTwelve()
+        self.bridge2 = BridgeTwelve()
+
+        self.ami.registerEvent('BridgeCreate', self.bridge_create)
+        self.ami.registerEvent('BridgeEnter', self.bridge_enter)
+
+    def bridge_create(self, _, event):
+        '''AMI event callback for BridgeCreate
+
+        This method is responsible for gleaning the unique IDs of bridges that
+        are created and saving them for later.
+        '''
+
+        if not self.bridge1.unique_id:
+            self.bridge1.unique_id = event.get('bridgeuniqueid')
+        elif not self.bridge2.unique_id:
+            self.bridge2.unique_id = event.get('bridgeuniqueid')
+        else:
+            LOGGER.error("Unexpected third bridge created")
+            self.test_object.set_passed(False)
+            self.test_object.stop_reactor()
+
+    def bridge_enter(self, _, event):
+        '''AMI event callback for BridgeEnter
+
+        This method makes the determination of whether the controller
+        is allowed to attempt to move on to the next state based on
+        the states of the two bridges involved
+        '''
+
+        if (event.get('bridgeuniqueid') == self.bridge1.unique_id and
+                event.get('bridgenumchannels') == '2'):
+            self.bridge1.bridged = True
+            if self.controller.state == BOB_CALLED:
+                self.controller.call_carol()
+            elif self.controller.state == TRANSFERRED:
+                self.controller.hangup_calls()
+            else:
+                LOGGER.error("Unexpected BridgeEnter event")
+                self.test_object.set_passed(False)
+                self.test_object.stop_reactor()
+        elif (event.get('bridgeuniqueid') == self.bridge2.unique_id and
+              event.get('bridgenumchannels') == '2'):
+            self.bridge2.bridged = True
+            if self.controller.state == CAROL_CALLED:
+                self.controller.transfer_call()
+            elif self.controller.state == TRANSFERRED:
+                self.controller.hangup_calls()
+            else:
+                LOGGER.error("Unexpected BridgeEnter event")
+                self.test_object.set_passed(False)
+                self.test_object.stop_reactor()
+
+    def bridge1_bridged(self):
+        '''Indicates if Alice and Bob are bridged'''
+
+        return self.bridge1.bridged
+
+    def bridge2_bridged(self):
+        '''Indicates if Alice and Carol ar bridged'''
+
+        return self.bridge2.bridged
+
+
+class BridgeStateEleven(object):
+    '''Tracker of bridge state for Asterisk 11-
+
+    Since in Asterisk versions prior to 12, there are no bridge objects, the
+    only way we can track the state of bridges in Asterisk is via Bridge events
+    and our own channel count. We count unique bridges by using the "channel2"
+    header in Bridge events from Asterisk.
+    '''
+    def __init__(self, test_object, controller, ami):
+        self.test_object = test_object
+        self.controller = controller
+        self.ami = ami
+        self.bridge_channels = []
+        self.final_bridge_participants = 0
+
+        self.ami.registerEvent('Bridge', self.bridge)
+        self.ami.registerEvent('VarSet', self.bridge_peer)
+
+    def bridge(self, _, event):
+        '''AMI Bridge event callback.
+
+        The Bridge callback in Asterisk 11- can fire at seemingly random
+        times, but it always has two channels indicated in it. This function
+        will log each unique 'channel2' channel that it sees, and assume that
+        a newly-discovered 'channel2' indicates that a new bridge has been
+        formed.
+        '''
+
+        if event['channel2'] in self.bridge_channels:
+            LOGGER.debug('channel {0} already seen in previous Bridge event. '
+                         'Ignoring'.format(event['channel2']))
+            return
+
+        LOGGER.debug('New bridge between {0} and {1} detected'.format(
+            event['channel1'], event['channel2']))
+        self.bridge_channels.append(event['channel2'])
+        numchans = len(self.bridge_channels)
+        if numchans == 1:
+            LOGGER.debug('Bridge between Alice and Bob established')
+            self.controller.call_carol()
+        elif numchans == 2:
+            LOGGER.debug('Bridge between Alice and Carol established')
+            self.controller.transfer_call()
+
+    def bridge_peer(self, _, event):
+        '''AMI VarSet event callback.
+
+        We are interested in BRIDGEPEER settings. When we get a BRIDGEPEER
+        that indicates that Bob and Carol have been bridged, then we consider
+        the transfer to have succeeded
+        '''
+
+        if event['variable'] != "BRIDGEPEER" or len(self.bridge_channels) < 2:
+            return
+
+        LOGGER.debug("After transfer, {0} is bridged to {1}".format(
+            event['channel'], event['value']))
+
+        # We should get an event indicating that the Bob channel's
+        # BRIDGEPEER variable is set to Carol's channel, and vice versa
+        if self.bridge_channels[:2] == [event['channel'], event['value']] or\
+            self.bridge_channels[:2] == [event['value'], event['channel']]:
+            self.final_bridge_participants += 1
+            if self.final_bridge_participants == 2:
+                LOGGER.debug("Bob and Carol bridged. Scenario complete.")
+                # success!
+                self.controller.hangup_calls()
+
+    def bridge1_bridged(self):
+        '''Indicates that Alice and Bob have been bridged'''
+        return len(self.bridge_channels) == 1
+
+    def bridge2_bridged(self):
+        '''Indicates that Alice and Carol have been bridged'''
+        return len(self.bridge_channels) == 2
 
 
 class Transfer(object):
@@ -74,13 +231,11 @@
     def __init__(self, test_object, accounts):
         super(Transfer, self).__init__()
         self.ami = test_object.ami[0]
-        self.ami.registerEvent('BridgeCreate', self.bridge_create)
-        self.ami.registerEvent('BridgeEnter', self.bridge_enter)
 
-        # bridge1 bridges Alice and Bob
-        self.bridge1 = BridgeState()
-        # bridge2 bridges Alice and Carol
-        self.bridge2 = BridgeState()
+        if AsteriskVersion() < AsteriskVersion('12'):
+            self.bridge_state = BridgeStateEleven(test_object, self, self.ami)
+        else:
+            self.bridge_state = BridgeStateTwelve(test_object, self, self.ami)
 
         self.bob_call_answered = False
         self.carol_call_answered = False
@@ -98,65 +253,64 @@
         self.test_object = test_object
         self.state = INIT
 
-    def bridge_create(self, ami, event):
-        if not self.bridge1.unique_id:
-            self.bridge1.unique_id = event.get('bridgeuniqueid')
-        elif not self.bridge2.unique_id:
-            self.bridge2.unique_id = event.get('bridgeuniqueid')
-        else:
-            LOGGER.error("Unexpected third bridge created")
-            self.test_object.set_passed(False)
-            self.test_object.stop_reactor()
-
-    def bridge_enter(self, ami, event):
-        if (event.get('bridgeuniqueid') == self.bridge1.unique_id and
-                event.get('bridgenumchannels') == '2'):
-            self.bridge1.bridged = True
-            if self.state == BOB_CALLED:
-                self.call_carol()
-            elif self.state == TRANSFERRED:
-                self.hangup_calls()
-            else:
-                LOGGER.error("Unexpected BridgeEnter event")
-                self.test_object.set_passed(False)
-                self.test_object.stop_reactor()
-        elif (event.get('bridgeuniqueid') == self.bridge2.unique_id and
-                event.get('bridgenumchannels') == '2'):
-            self.bridge2.bridged = True
-            if self.state == CAROL_CALLED:
-                self.transfer_call()
-            elif self.state == TRANSFERRED:
-                self.hangup_calls()
-            else:
-                LOGGER.error("Unexpected BridgeEnter event")
-                self.test_object.set_passed(False)
-                self.test_object.stop_reactor()
-
     def bob_call_confirmed(self):
+        '''PJSUA indication that the answer from Alice's call to Bob has been
+        received
+        '''
+
         self.bob_call_answered = True
         self.call_carol()
 
     def carol_call_confirmed(self):
+        '''PJSUA indication that the answer from Alice's call to Carol has been
+        received
+        '''
+
         self.carol_call_answered = True
         self.transfer_call()
 
     def call_bob(self):
+        '''Have Alice place a call to Bob'''
+
         self.call_to_bob = self.alice.make_call('sip:bob at 127.0.0.1',
                 CallCallback(None, self.bob_call_confirmed))
         self.state = BOB_CALLED
 
     def call_carol(self):
-        if self.bridge1.bridged and self.bob_call_answered:
+        '''Have Alice place a call to Carol
+
+        Alice's PJSUA instance needs to have confirmed that the answer from
+        Bob has arrived and Asterisk has to have Alice and Bob bridged before
+        the call to Carol may be attempted. Therefore, events that indicate
+        that these two prerequisites have been met will each attempt to have
+        Alice call Carol.
+        '''
+        if self.bridge_state.bridge1_bridged() and self.bob_call_answered:
             self.call_to_carol = self.alice.make_call('sip:carol at 127.0.0.1',
                     CallCallback(None, self.carol_call_confirmed))
             self.state = CAROL_CALLED
 
     def transfer_call(self):
-        if self.bridge2.bridged and self.carol_call_answered:
+        '''Have Alice transfer Bob to Carol
+
+        Alice's PJSUA instance needs to have confirmed that the answer from
+        Carol has arrived and Asterisk has to have Alice and Carol bridged
+        before the call to Carol may be attempted. Therefore, events that
+        indicate that these two prerequisites have been met will each attempt
+        to have Alice transfer the call.
+        '''
+
+        if self.bridge_state.bridge2_bridged() and self.carol_call_answered:
             self.call_to_bob.transfer_to_call(self.call_to_carol)
             self.state = TRANSFERRED
 
     def hangup_calls(self):
+        '''Hang up remaining calls and end the test
+
+        Once the transfer has completed, Bob and Carol will be bridged. We have
+        the test hang them both up in order to clean up resources before marking
+        the test as passed and stopping the reactor
+        '''
         bob_hangup = {
             'Action': 'Hangup',
             'Channel': '/SIP/bob-.*/',
diff --git a/tests/channels/SIP/sip_attended_transfer/test-config.yaml b/tests/channels/SIP/sip_attended_transfer/test-config.yaml
index 7fe5c60..5b8e3b2 100644
--- a/tests/channels/SIP/sip_attended_transfer/test-config.yaml
+++ b/tests/channels/SIP/sip_attended_transfer/test-config.yaml
@@ -47,7 +47,7 @@
             transport: 'local-ipv4'
 
 properties:
-    minversion: '12.0.0'
+    minversion: '1.8.0'
     dependencies:
         - python : 'twisted'
         - python : 'starpy'

-- 
To view, visit https://gerrit.asterisk.org/29
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>



More information about the asterisk-dev mailing list