Re: [asterisk-dev] [Code Review] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-13 Thread Mark Michelson


 On Feb. 12, 2015, 6:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py, lines 258-259
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71349#file71349line258
 
  Is this change necessary since you ended up overriding reg_success() in 
  your new pluggable object?
 
 jbigelow wrote:
 If that condition is removed then it will blow up if callback_module  
 callback_method are not defined in YAML and pjsua_mod.PJsua is being used as 
 a test module. If it remains with the same scenario then things won't blow up 
 and no user code is called into(maybe there's a need for that?). Suggestions?

Hm, I guess my thoughts here are that if someone is trying to use the 
pjsua_mod.PJsua pluggable module directly (as opposed to using the phone 
controller), then it is an error currently to not specify a callback_module and 
callback_method.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/#review14450
---


On Feb. 13, 2015, 12:27 a.m., jbigelow wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4413/
 ---
 
 (Updated Feb. 13, 2015, 12:27 a.m.)
 
 
 Review request for Asterisk Developers and Mark Michelson.
 
 
 Bugs: ASTERISK-24578
 https://issues.asterisk.org/jira/browse/ASTERISK-24578
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 Pluggable modules to place, receive, and transfer (blind/attended) calls to 
 simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
 transferred using the new pluggable action module. This should allow many 
 currrent and future tests to easily send/receive calls to/from Asterisk along 
 with transferring calls within YAML configuration.
 
 The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
 accounts and once all have registered, the account callbacks are setup and 
 are ready to receive calls. The pluggable action module 
 (pluggable_modules.PjsuaPhoneActionModule) provides the ability to place 
 calls and transfer calls using the accounts from YAML and the action is 
 referenced with 'pjsua_phone'. The only time a call is hung up by this is 
 when a transfer is performed and a 200 OK sipfrag NOTIFY is received. None of 
 the modules set a pass/fail result and are only for driving and manipulating 
 calls.
 
 See attached file for YAML demo.
 
 
 Diffs
 -
 
   /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
   /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
   /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/4413/diff/
 
 
 Testing
 ---
 
 * Tested placing calls, receiving calls, transfering via blind  attended.
 * Pylint score of 9.40/10 for phones.py
 * See attached test-config.yaml for a demonstration.
 
 
 File Attachments
 
 
 Demonstration
   
 https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml
 
 
 Thanks,
 
 jbigelow
 


-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-13 Thread Mark Michelson

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/#review14459
---

Ship it!


Ship It!

- Mark Michelson


On Feb. 13, 2015, 3:48 p.m., jbigelow wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4413/
 ---
 
 (Updated Feb. 13, 2015, 3:48 p.m.)
 
 
 Review request for Asterisk Developers and Mark Michelson.
 
 
 Bugs: ASTERISK-24578
 https://issues.asterisk.org/jira/browse/ASTERISK-24578
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 Pluggable modules to place, receive, and transfer (blind/attended) calls to 
 simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
 transferred using the new pluggable action module. This should allow many 
 currrent and future tests to easily send/receive calls to/from Asterisk along 
 with transferring calls within YAML configuration.
 
 The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
 accounts and once all have registered, the account callbacks are setup and 
 are ready to receive calls. The pluggable action module 
 (pluggable_modules.PjsuaPhoneActionModule) provides the ability to place 
 calls and transfer calls using the accounts from YAML and the action is 
 referenced with 'pjsua_phone'. The only time a call is hung up by this is 
 when a transfer is performed and a 200 OK sipfrag NOTIFY is received. None of 
 the modules set a pass/fail result and are only for driving and manipulating 
 calls.
 
 See attached file for YAML demo.
 
 
 Diffs
 -
 
   /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
   /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
   /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/4413/diff/
 
 
 Testing
 ---
 
 * Tested placing calls, receiving calls, transfering via blind  attended.
 * Pylint score of 9.40/10 for phones.py
 * See attached test-config.yaml for a demonstration.
 
 
 File Attachments
 
 
 Demonstration
   
 https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml
 
 
 Thanks,
 
 jbigelow
 


-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-13 Thread jbigelow


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py, lines 258-259
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71349#file71349line258
 
  Is this change necessary since you ended up overriding reg_success() in 
  your new pluggable object?
 
 jbigelow wrote:
 If that condition is removed then it will blow up if callback_module  
 callback_method are not defined in YAML and pjsua_mod.PJsua is being used as 
 a test module. If it remains with the same scenario then things won't blow up 
 and no user code is called into(maybe there's a need for that?). Suggestions?
 
 Mark Michelson wrote:
 Hm, I guess my thoughts here are that if someone is trying to use the 
 pjsua_mod.PJsua pluggable module directly (as opposed to using the phone 
 controller), then it is an error currently to not specify a callback_module 
 and callback_method.

Changed to log an error, stop reactor, and then return.


- jbigelow


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/#review14450
---


On Feb. 12, 2015, 6:27 p.m., jbigelow wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4413/
 ---
 
 (Updated Feb. 12, 2015, 6:27 p.m.)
 
 
 Review request for Asterisk Developers and Mark Michelson.
 
 
 Bugs: ASTERISK-24578
 https://issues.asterisk.org/jira/browse/ASTERISK-24578
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 Pluggable modules to place, receive, and transfer (blind/attended) calls to 
 simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
 transferred using the new pluggable action module. This should allow many 
 currrent and future tests to easily send/receive calls to/from Asterisk along 
 with transferring calls within YAML configuration.
 
 The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
 accounts and once all have registered, the account callbacks are setup and 
 are ready to receive calls. The pluggable action module 
 (pluggable_modules.PjsuaPhoneActionModule) provides the ability to place 
 calls and transfer calls using the accounts from YAML and the action is 
 referenced with 'pjsua_phone'. The only time a call is hung up by this is 
 when a transfer is performed and a 200 OK sipfrag NOTIFY is received. None of 
 the modules set a pass/fail result and are only for driving and manipulating 
 calls.
 
 See attached file for YAML demo.
 
 
 Diffs
 -
 
   /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
   /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
   /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/4413/diff/
 
 
 Testing
 ---
 
 * Tested placing calls, receiving calls, transfering via blind  attended.
 * Pylint score of 9.40/10 for phones.py
 * See attached test-config.yaml for a demonstration.
 
 
 File Attachments
 
 
 Demonstration
   
 https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml
 
 
 Thanks,
 
 jbigelow
 


-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-13 Thread jbigelow

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/
---

(Updated Feb. 13, 2015, 9:48 a.m.)


Review request for Asterisk Developers and Mark Michelson.


Changes
---

Addressed finding.


Bugs: ASTERISK-24578
https://issues.asterisk.org/jira/browse/ASTERISK-24578


Repository: testsuite


Description
---

Pluggable modules to place, receive, and transfer (blind/attended) calls to 
simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
transferred using the new pluggable action module. This should allow many 
currrent and future tests to easily send/receive calls to/from Asterisk along 
with transferring calls within YAML configuration.

The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
accounts and once all have registered, the account callbacks are setup and are 
ready to receive calls. The pluggable action module 
(pluggable_modules.PjsuaPhoneActionModule) provides the ability to place calls 
and transfer calls using the accounts from YAML and the action is referenced 
with 'pjsua_phone'. The only time a call is hung up by this is when a transfer 
is performed and a 200 OK sipfrag NOTIFY is received. None of the modules set a 
pass/fail result and are only for driving and manipulating calls.

See attached file for YAML demo.


Diffs (updated)
-

  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
  /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/4413/diff/


Testing
---

* Tested placing calls, receiving calls, transfering via blind  attended.
* Pylint score of 9.40/10 for phones.py
* See attached test-config.yaml for a demonstration.


File Attachments


Demonstration
  
https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml


Thanks,

jbigelow

-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-13 Thread jbigelow

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/
---

(Updated Feb. 13, 2015, 12:28 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and Mark Michelson.


Changes
---

Committed in revision 6406


Bugs: ASTERISK-24578
https://issues.asterisk.org/jira/browse/ASTERISK-24578


Repository: testsuite


Description
---

Pluggable modules to place, receive, and transfer (blind/attended) calls to 
simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
transferred using the new pluggable action module. This should allow many 
currrent and future tests to easily send/receive calls to/from Asterisk along 
with transferring calls within YAML configuration.

The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
accounts and once all have registered, the account callbacks are setup and are 
ready to receive calls. The pluggable action module 
(pluggable_modules.PjsuaPhoneActionModule) provides the ability to place calls 
and transfer calls using the accounts from YAML and the action is referenced 
with 'pjsua_phone'. The only time a call is hung up by this is when a transfer 
is performed and a 200 OK sipfrag NOTIFY is received. None of the modules set a 
pass/fail result and are only for driving and manipulating calls.

See attached file for YAML demo.


Diffs
-

  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
  /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/4413/diff/


Testing
---

* Tested placing calls, receiving calls, transfering via blind  attended.
* Pylint score of 9.40/10 for phones.py
* See attached test-config.yaml for a demonstration.


File Attachments


Demonstration
  
https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml


Thanks,

jbigelow

-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-12 Thread Mark Michelson

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/#review14450
---



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24971

I think this is a good case for using is instead of equality. I imagine 
that callers of this function are going to be passing a reference to the same 
account object that is stored on the phone_obj rather than a separate object 
with equivalent properties.

Also, to avoid the continue statement, you can just use a positive 
comparison instead of a negative one:

if account is phone_obj.account:
return phone_obj



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24970

A couple of PEP 8 points:

1) When catching exceptions, use the syntax except ErrorType as 
ErrorInstance instead of except ErrorType, ErrorInstance. So here, it would 
be except pj.Error as err

2) A PEP 8 checker will complain about the indentation of str(err)) on the 
final line I've highlighted. It will claim it should be lined up like this:

raise Exception(Exception occurred while making call: '%s' %
str(err))

See how str is lined up to show how it belongs to the parentheses?



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24972

I suggest breaking this into two separate functions: blind_transfer and 
attended_transfer.

1) The nomenclature for the transfer_type has to be looked up in the source 
in order to know what values are accepted.

2) The transfer_uri is only useful for blind transfers. By having two 
separate functions, you can more clearly group which parameters are relevant 
and which are irrelevant for a specific type of transfer. So the attended 
transfer, as currently defined, would take no parameters and a blind transfer 
would take a transfer URI. If it became useful later to optionally specify that 
an attended transfer should go the other direction or should affect different 
calls, that could be added solely to the attended transfer function.



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24973

self.current_call doesn't seem to be used for anything.



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24974

Should this only be done if self.phone is None? Seems like this would be 
redundant after the initial setting.



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24975

I suspect it's not an issue, but is there ever a chance that self.call 
could be None when the on_state() callback is called?



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24976

This seems overly complicated for what you are trying to do here. Why not 
just

try:
self.phone.calls.remove(self.call)
except ValueError:
# Log an error or something

If that doesn't work because equality of calls doesn't evaluate as you'd 
expect, then this is a situation where I expect that a single object reference 
is being passed around everywhere rather than separate call objects. You could 
take the approach of reconstructing the self.phone.calls list like this:

self.phone.calls = [call for call in self.phone.calls if call is not 
self.call]



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24977

I think this needs to be altered to always call self.call.hangup() if final 
is 1. If a transfer attempt fails, the phone is still responsible for hanging 
up the call in that situation. So if you perform a blind transfer to a 
non-existent extension in the dialplan, or you perform a blind transfer to an 
extension that exists but hangs up with Congestion() or Busy() for some reason, 
the phone still needs to hang up the call when receiving the final transfer 
response.



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24978

This function's name is misleading because it does not actually log 
anything. It should either

1) Be renamed to something indicating that it is just gathering call 
information into a string

or

2) Be altered to actually log the call info using LOGGER.debug()



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24979

Can you add a comment about why the lock is being used here?



/asterisk/trunk/lib/python/asterisk/phones.py
https://reviewboard.asterisk.org/r/4413/#comment24980

If this exception occurs, does lock get properly unlocked?



/asterisk/trunk/lib/python/asterisk/phones.py

Re: [asterisk-dev] [Code Review] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-12 Thread jbigelow

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/
---

(Updated Feb. 12, 2015, 6:27 p.m.)


Review request for Asterisk Developers and Mark Michelson.


Changes
---

Addressed all but one issue which I'm looking for feedback on.


Bugs: ASTERISK-24578
https://issues.asterisk.org/jira/browse/ASTERISK-24578


Repository: testsuite


Description
---

Pluggable modules to place, receive, and transfer (blind/attended) calls to 
simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
transferred using the new pluggable action module. This should allow many 
currrent and future tests to easily send/receive calls to/from Asterisk along 
with transferring calls within YAML configuration.

The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
accounts and once all have registered, the account callbacks are setup and are 
ready to receive calls. The pluggable action module 
(pluggable_modules.PjsuaPhoneActionModule) provides the ability to place calls 
and transfer calls using the accounts from YAML and the action is referenced 
with 'pjsua_phone'. The only time a call is hung up by this is when a transfer 
is performed and a 200 OK sipfrag NOTIFY is received. None of the modules set a 
pass/fail result and are only for driving and manipulating calls.

See attached file for YAML demo.


Diffs (updated)
-

  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
  /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/4413/diff/


Testing
---

* Tested placing calls, receiving calls, transfering via blind  attended.
* Pylint score of 9.40/10 for phones.py
* See attached test-config.yaml for a demonstration.


File Attachments


Demonstration
  
https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml


Thanks,

jbigelow

-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-12 Thread jbigelow


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, lines 86-88
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line86
 
  I think this is a good case for using is instead of equality. I 
  imagine that callers of this function are going to be passing a reference 
  to the same account object that is stored on the phone_obj rather than a 
  separate object with equivalent properties.
  
  Also, to avoid the continue statement, you can just use a positive 
  comparison instead of a negative one:
  
  if account is phone_obj.account:
  return phone_obj

Yup, that's what I meant to do :)


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, lines 115-117
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line115
 
  A couple of PEP 8 points:
  
  1) When catching exceptions, use the syntax except ErrorType as 
  ErrorInstance instead of except ErrorType, ErrorInstance. So here, it 
  would be except pj.Error as err
  
  2) A PEP 8 checker will complain about the indentation of str(err)) on 
  the final line I've highlighted. It will claim it should be lined up like 
  this:
  
  raise Exception(Exception occurred while making call: '%s' %
  str(err))
  
  See how str is lined up to show how it belongs to the parentheses?

I blame my python plugin for vim which just shifts twice. I installed the pep8 
utility which pointed out a couple others which I have also fixed.


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, line 156
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line156
 
  self.current_call doesn't seem to be used for anything.

Removed.


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, lines 190-191
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line190
 
  Should this only be done if self.phone is None? Seems like this would 
  be redundant after the initial setting.

That would make sense.


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, line 192
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line192
 
  I suspect it's not an issue, but is there ever a chance that self.call 
  could be None when the on_state() callback is called?

As far as I know self.call would never be None when a pjsua callback is called 
such as on_state().


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, lines 197-204
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line197
 
  This seems overly complicated for what you are trying to do here. Why 
  not just
  
  try:
  self.phone.calls.remove(self.call)
  except ValueError:
  # Log an error or something
  
  If that doesn't work because equality of calls doesn't evaluate as 
  you'd expect, then this is a situation where I expect that a single object 
  reference is being passed around everywhere rather than separate call 
  objects. You could take the approach of reconstructing the self.phone.calls 
  list like this:
  
  self.phone.calls = [call for call in self.phone.calls if call is not 
  self.call]

The first suggestion is actually what I initially was using. However I found 
that the call object wasn't always being removed from the list. Debugging 
showed the id() of self.call not matching any in the list which I believe is 
because self.call is set to weakref.proxy(call) in pjsua.CallCallback(). 
Comparing the SIP Call-ID of the call objects should be unique enough to find 
the one to remove from the list.


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, line 246
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line246
 
  This function's name is misleading because it does not actually log 
  anything. It should either
  
  1) Be renamed to something indicating that it is just gathering call 
  information into a string
  
  or
  
  2) Be altered to actually log the call info using LOGGER.debug()

Renamed.


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, line 270
  https://reviewboard.asterisk.org/r/4413/diff/1/?file=71348#file71348line270
 
  Can you add a comment about why the lock is being used here?

Fixed this by removing the locking :)

Apparently a lock isn't needed when making call. Unless perhaps a callback 
isn't passed and then set later using call.set_callback() but that's not the 
case here.


 On Feb. 12, 2015, 12:42 p.m., Mark Michelson wrote:
  /asterisk/trunk/lib/python/asterisk/phones.py, lines 272-275
  

Re: [asterisk-dev] [Code Review] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-12 Thread jbigelow

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/
---

(Updated Feb. 12, 2015, 10:25 a.m.)


Review request for Asterisk Developers and Mark Michelson.


Bugs: ASTERISK-24578
https://issues.asterisk.org/jira/browse/ASTERISK-24578


Repository: testsuite


Description (updated)
---

Pluggable modules to place, receive, and transfer (blind/attended) calls to 
simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
transferred using the new pluggable action module. This should allow many 
currrent and future tests to easily send/receive calls to/from Asterisk along 
with transferring calls within YAML configuration.

The pluggable test module (phones.PjsuaPhoneController) initializes the PJSUA 
accounts and once all have registered, the account callbacks are setup and are 
ready to receive calls. The pluggable action module 
(pluggable_modules.PjsuaPhoneActionModule) provides the ability to place calls 
and transfer calls using the accounts from YAML and the action is referenced 
with 'pjsua_phone'. The only time a call is hung up by this is when a transfer 
is performed and a 200 OK sipfrag NOTIFY is received. None of the modules set a 
pass/fail result and are only for driving and manipulating calls.

See attached file for YAML demo.


Diffs
-

  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
  /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/4413/diff/


Testing
---

* Tested placing calls, receiving calls, transfering via blind  attended.
* Pylint score of 9.40/10 for phones.py
* See attached test-config.yaml for a demonstration.


File Attachments


Demonstration
  
https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml


Thanks,

jbigelow

-- 
_
-- 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] 4413: Testsuite: Simulate phones and control from YAML.

2015-02-10 Thread jbigelow

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4413/
---

(Updated Feb. 11, 2015, 12:32 a.m.)


Review request for Asterisk Developers.


Repository: testsuite


Description
---

Pluggable modules to place, receive, and transfer (blind/attended) calls to 
simulate phones using PJSUA and YAML configuration. Calls are placed and/or 
transferred using the new pluggable action module. This should allow many 
currrent and future tests to easily send/receive calls to/from Asterisk along 
with transferring calls within YAML configuration.

See attached file for YAML demo.


Diffs
-

  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 6379 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 6379 
  /asterisk/trunk/lib/python/asterisk/phones.py PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/4413/diff/


Testing
---

* Tested placing calls, receiving calls, transfering via blind  attended.
* Pylint score of 9.40/10 for phones.py
* See attached test-config.yaml for a demonstration.


File Attachments


Demonstration
  
https://reviewboard.asterisk.org/media/uploaded/files/2015/02/11/659ab31f-8401-4f24-be5e-da1db0be3156__test-config.yaml


Thanks,

jbigelow

-- 
_
-- 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