Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.

2014-04-03 Thread rmudgett

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



/trunk/funcs/func_periodic_hook.c


extension?



/trunk/funcs/func_periodic_hook.c


The  tags cause the ${} to be added by output formatting.  They 
should not be in the raw text.



/trunk/funcs/func_periodic_hook.c


ran



/trunk/funcs/func_periodic_hook.c


ast_strdupa() is not NULL tollerant.

Doing
ast_strdupa(S_OR(data, ""))
will then cause the interval message to go out.


- rmudgett


On April 3, 2014, 7:05 p.m., Russell Bryant wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3362/
> ---
> 
> (Updated April 3, 2014, 7:05 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This commit introduces a new dialplan function, PERIODIC_HOOK().
> It allows you run to a dialplan hook on a channel periodically.  The
> original use case that inspired this was the ability to play a beep
> periodically into a call being recorded.  The implementation is much
> more generic though and could be used for many other things.
> 
> The implementation makes heavy use of existing Asterisk components.
> It uses a combination of Local channels and ChanSpy() to run some
> custom dialplan and inject any audio it generates into an active call.
> 
> The other important bit of the implementation is how it figures out
> when to trigger the beep playback.  This implementation uses the
> audiohook API, even though it's not actually touching the audio in any
> way.  It's a convenient way to get a callback and check if it's time
> to kick off another beep.  It would be nice if this was timer event
> based instead of polling based, but unfortunately I don't see a way to
> do it that won't interfere with other things.
> 
> 
> Diffs
> -
> 
>   /trunk/funcs/func_periodic_hook.c PRE-CREATION 
>   /trunk/CHANGES 411684 
> 
> Diff: https://reviewboard.asterisk.org/r/3362/diff/
> 
> 
> Testing
> ---
> 
> Called the following extension (100@test), both letting it run all the way 
> through, as well as hanging up at various points in the middle.
> 
> [hooks]
> 
> exten => beep,1,Answer()
> same => n,Verbose(1,Channel name: ${HOOK_CHANNEL})
> same => n,Verbose(1,Hook ID: ${HOOK_ID})
> same => n,Playback(beep)
> 
> [test]
> 
> exten => 100,1,Answer()
> same => n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
> same => n,Wait(20)
> same => n,Hangup()
> 
> 
> Thanks,
> 
> Russell Bryant
> 
>

-- 
_
-- 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] 3362: func_periodic_hook: New function for periodic hooks.

2014-04-03 Thread Corey Farrell


> On April 2, 2014, 3:44 p.m., rmudgett wrote:
> > /trunk/funcs/func_periodic_hook.c, lines 85-87
> > 
> >
> > These should not be uppercase.  That indicates they are macros.
> 
> Russell Bryant wrote:
> Hrm, I think of UPPERCASE() as a macro and UPPERCASE as a constant.  
> That's why I did it this way.

AST_MODULE is also a macro (#define).  I normally expect all UPPERCASE 
identifiers to be replaced by the C preprocessor.


- Corey


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


On April 3, 2014, 8:05 p.m., Russell Bryant wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3362/
> ---
> 
> (Updated April 3, 2014, 8:05 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This commit introduces a new dialplan function, PERIODIC_HOOK().
> It allows you run to a dialplan hook on a channel periodically.  The
> original use case that inspired this was the ability to play a beep
> periodically into a call being recorded.  The implementation is much
> more generic though and could be used for many other things.
> 
> The implementation makes heavy use of existing Asterisk components.
> It uses a combination of Local channels and ChanSpy() to run some
> custom dialplan and inject any audio it generates into an active call.
> 
> The other important bit of the implementation is how it figures out
> when to trigger the beep playback.  This implementation uses the
> audiohook API, even though it's not actually touching the audio in any
> way.  It's a convenient way to get a callback and check if it's time
> to kick off another beep.  It would be nice if this was timer event
> based instead of polling based, but unfortunately I don't see a way to
> do it that won't interfere with other things.
> 
> 
> Diffs
> -
> 
>   /trunk/funcs/func_periodic_hook.c PRE-CREATION 
>   /trunk/CHANGES 411684 
> 
> Diff: https://reviewboard.asterisk.org/r/3362/diff/
> 
> 
> Testing
> ---
> 
> Called the following extension (100@test), both letting it run all the way 
> through, as well as hanging up at various points in the middle.
> 
> [hooks]
> 
> exten => beep,1,Answer()
> same => n,Verbose(1,Channel name: ${HOOK_CHANNEL})
> same => n,Verbose(1,Hook ID: ${HOOK_ID})
> same => n,Playback(beep)
> 
> [test]
> 
> exten => 100,1,Answer()
> same => n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
> same => n,Wait(20)
> same => n,Hangup()
> 
> 
> Thanks,
> 
> Russell Bryant
> 
>

-- 
_
-- 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] 3362: func_periodic_hook: New function for periodic hooks.

2014-04-03 Thread Corey Farrell

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



/trunk/funcs/func_periodic_hook.c


The test for ast_strlen_zero needs to be reversed.


- Corey Farrell


On April 3, 2014, 8:05 p.m., Russell Bryant wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3362/
> ---
> 
> (Updated April 3, 2014, 8:05 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This commit introduces a new dialplan function, PERIODIC_HOOK().
> It allows you run to a dialplan hook on a channel periodically.  The
> original use case that inspired this was the ability to play a beep
> periodically into a call being recorded.  The implementation is much
> more generic though and could be used for many other things.
> 
> The implementation makes heavy use of existing Asterisk components.
> It uses a combination of Local channels and ChanSpy() to run some
> custom dialplan and inject any audio it generates into an active call.
> 
> The other important bit of the implementation is how it figures out
> when to trigger the beep playback.  This implementation uses the
> audiohook API, even though it's not actually touching the audio in any
> way.  It's a convenient way to get a callback and check if it's time
> to kick off another beep.  It would be nice if this was timer event
> based instead of polling based, but unfortunately I don't see a way to
> do it that won't interfere with other things.
> 
> 
> Diffs
> -
> 
>   /trunk/funcs/func_periodic_hook.c PRE-CREATION 
>   /trunk/CHANGES 411684 
> 
> Diff: https://reviewboard.asterisk.org/r/3362/diff/
> 
> 
> Testing
> ---
> 
> Called the following extension (100@test), both letting it run all the way 
> through, as well as hanging up at various points in the middle.
> 
> [hooks]
> 
> exten => beep,1,Answer()
> same => n,Verbose(1,Channel name: ${HOOK_CHANNEL})
> same => n,Verbose(1,Hook ID: ${HOOK_ID})
> same => n,Playback(beep)
> 
> [test]
> 
> exten => 100,1,Answer()
> same => n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
> same => n,Wait(20)
> same => n,Hangup()
> 
> 
> Thanks,
> 
> Russell Bryant
> 
>

-- 
_
-- 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] 3362: func_periodic_hook: New function for periodic hooks.

2014-04-03 Thread Russell Bryant

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

(Updated April 4, 2014, 12:05 a.m.)


Review request for Asterisk Developers.


Changes
---

Thanks for the reviews!


Repository: Asterisk


Description
---

This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically.  The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded.  The implementation is much
more generic though and could be used for many other things.

The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.

The other important bit of the implementation is how it figures out
when to trigger the beep playback.  This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way.  It's a convenient way to get a callback and check if it's time
to kick off another beep.  It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.


Diffs (updated)
-

  /trunk/funcs/func_periodic_hook.c PRE-CREATION 
  /trunk/CHANGES 411684 

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


Testing
---

Called the following extension (100@test), both letting it run all the way 
through, as well as hanging up at various points in the middle.

[hooks]

exten => beep,1,Answer()
same => n,Verbose(1,Channel name: ${HOOK_CHANNEL})
same => n,Verbose(1,Hook ID: ${HOOK_ID})
same => n,Playback(beep)

[test]

exten => 100,1,Answer()
same => n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same => n,Wait(20)
same => n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same => n,Wait(20)
same => n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same => n,Wait(20)
same => n,Hangup()


Thanks,

Russell Bryant

-- 
_
-- 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] 3362: func_periodic_hook: New function for periodic hooks.

2014-04-03 Thread Russell Bryant


> On April 2, 2014, 7:44 p.m., rmudgett wrote:
> > /trunk/funcs/func_periodic_hook.c, lines 48-57
> > 
> >
> > Document the (On write) hook_id.
> > 
> > Is this the first function that has different parameters for read and 
> > write?

I don't know of another one off hand that has different args.


> On April 2, 2014, 7:44 p.m., rmudgett wrote:
> > /trunk/funcs/func_periodic_hook.c, lines 85-87
> > 
> >
> > These should not be uppercase.  That indicates they are macros.

Hrm, I think of UPPERCASE() as a macro and UPPERCASE as a constant.  That's why 
I did it this way.


> On April 2, 2014, 7:44 p.m., rmudgett wrote:
> > /trunk/funcs/func_periodic_hook.c, lines 301-302
> > 
> >
> > You need to check if args.interval is NULL.  Also the sscanf should be 
> > using "%30u" instead.  Also passing a NULL string pointer to printf will 
> > crash on Solaris.
> > 
> > Missing \n

Ah, I thought args.interval would be guaranteed to at least be an empty string. 
 That doesn't seem to be the case.


> On April 2, 2014, 7:44 p.m., rmudgett wrote:
> > /trunk/funcs/func_periodic_hook.c, lines 384-386
> > 
> >
> > Maybe use ast_true() and ast_false() instead.

Done


- Russell


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


On April 1, 2014, 4:43 p.m., Russell Bryant wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3362/
> ---
> 
> (Updated April 1, 2014, 4:43 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This commit introduces a new dialplan function, PERIODIC_HOOK().
> It allows you run to a dialplan hook on a channel periodically.  The
> original use case that inspired this was the ability to play a beep
> periodically into a call being recorded.  The implementation is much
> more generic though and could be used for many other things.
> 
> The implementation makes heavy use of existing Asterisk components.
> It uses a combination of Local channels and ChanSpy() to run some
> custom dialplan and inject any audio it generates into an active call.
> 
> The other important bit of the implementation is how it figures out
> when to trigger the beep playback.  This implementation uses the
> audiohook API, even though it's not actually touching the audio in any
> way.  It's a convenient way to get a callback and check if it's time
> to kick off another beep.  It would be nice if this was timer event
> based instead of polling based, but unfortunately I don't see a way to
> do it that won't interfere with other things.
> 
> 
> Diffs
> -
> 
>   /trunk/funcs/func_periodic_hook.c PRE-CREATION 
>   /trunk/CHANGES 411583 
> 
> Diff: https://reviewboard.asterisk.org/r/3362/diff/
> 
> 
> Testing
> ---
> 
> Called the following extension (100@test), both letting it run all the way 
> through, as well as hanging up at various points in the middle.
> 
> [hooks]
> 
> exten => beep,1,Answer()
> same => n,Verbose(1,Channel name: ${HOOK_CHANNEL})
> same => n,Verbose(1,Hook ID: ${HOOK_ID})
> same => n,Playback(beep)
> 
> [test]
> 
> exten => 100,1,Answer()
> same => n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
> same => n,Wait(20)
> same => n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
> same => n,Wait(20)
> same => n,Hangup()
> 
> 
> Thanks,
> 
> Russell Bryant
> 
>

-- 
_
-- 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] 3415: bridging: Ensure proper locking during snapshot creation

2014-04-03 Thread rmudgett

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


You need to update the doxygen in stasis_bridges.h about bridge locking 
preconditions for many of the functions.
ast_bridge_snapshot_create() requires the bridge locked on entry.
ast_bridge_publish_merge() requires the two bridges locked on entry.
ast_bridge_publish_blind_transfer() requires the bridge locked on entry.
ast_bridge_publish_attended_transfer_xxx() requires the involved bridges to be 
locked on entry.

- rmudgett


On April 3, 2014, 2:13 p.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3415/
> ---
> 
> (Updated April 3, 2014, 2:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22904
> https://issues.asterisk.org/jira/browse/ASTERISK-22904
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> While the vast majority of bridge snapshot creation is locked properly, there 
> are currently some instances that are not. This adds the missing locking to 
> ensure bridge state is not malleable during snapshot creation.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_cel.c 411576 
>   branches/12/res/ari/resource_bridges.c 411576 
>   branches/12/main/bridge_basic.c 411576 
>   branches/12/main/bridge.c 411576 
>   branches/12/apps/app_confbridge.c 411576 
> 
> Diff: https://reviewboard.asterisk.org/r/3415/diff/
> 
> 
> Testing
> ---
> 
> Ran the testsuite to ensure that it did not cause failures.
> 
> 
> 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] 3339: Testsuite: ARI test for playback of audio to a channel in a bridge.

2014-04-03 Thread Kevin Harwell

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

Ship it!


Ship It!

- Kevin Harwell


On April 3, 2014, 3:03 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3339/
> ---
> 
> (Updated April 3, 2014, 3:03 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This was created to test the changes in /r/3338
> 
> This test is pretty simple. A channel is placed into an ARI bridge, and two 
> playbacks are queued up in quick succession. The test ensures that the files 
> play and that the first file plays and finishes before the second file plays 
> and finishes.
> 
> 
> Diffs
> -
> 
>   
> /asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/test-config.yaml 
> PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/play_file.py 
> PRE-CREATION 
>   /asterisk/trunk/tests/rest_api/playback/tests.yaml 4836 
> 
> Diff: https://reviewboard.asterisk.org/r/3339/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] 3412: testsuite: Add call setup tracking to masquerade supertest.

2014-04-03 Thread rmudgett

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

(Updated April 3, 2014, 3:45 p.m.)


Review request for Asterisk Developers.


Changes
---

Fixed PEP8 mmichelson pointed out.  Other PEP8 changes should wait for a 
separate patch unless they directly affect the changed lines in this patch.


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


Repository: testsuite


Description
---

* Changed originated calls to use AMI originate instead of CLI originate so a 
custom call timeout could be specified.  The number of calls in the chain could 
take awhile to setup which the CLI originate might timeout before all calls in 
the chain were dialed.

* Call setup is now tracked to so the test can better distinguish when calls 
hangup if they were optimizations or timeout hangups.

* Added some helpful test failure diagnosis messages when the reactor is 
stopped.

* Added needed debug routines to get "core show locks" output into the test log.

NOTE:  This is an older test that was written before PEP8 was more rigidly 
enforced in review.  Another patch will be made to clear up PEP8 formatting 
compliance.


Diffs (updated)
-

  /asterisk/trunk/tests/masquerade/run-test 4929 
  /asterisk/trunk/tests/masquerade/configs/ast1/extensions.conf 4929 

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


Testing
---

I've done many test runs looking for why the masquerade test fails and I have 
seen just about all the failure cases reported.

The core show locks code works when I uncomment and comment the right lines.  
It showed that the problem was not a deadlock on Asterisk v12.


Thanks,

rmudgett

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3412: testsuite: Add call setup tracking to masquerade supertest.

2014-04-03 Thread rmudgett


> On April 3, 2014, 1:19 p.m., Mark Michelson wrote:
> > I recommend running your changes through a PEP8 checker of some sort (such 
> > as pylint). I'll point out some of the violations I see, but a static 
> > checker will point out others.

I was wanting to do a PEP8 compliance patch separately to minimize the noise it 
will cause to this patch.


> On April 3, 2014, 1:19 p.m., Mark Michelson wrote:
> > /asterisk/trunk/tests/masquerade/run-test, line 230
> > 
> >
> > Just use
> > 
> > logger.info(cli_command.output)


- rmudgett


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


On April 1, 2014, 7:09 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3412/
> ---
> 
> (Updated April 1, 2014, 7:09 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22846
> https://issues.asterisk.org/jira/browse/ASTERISK-22846
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> * Changed originated calls to use AMI originate instead of CLI originate so a 
> custom call timeout could be specified.  The number of calls in the chain 
> could take awhile to setup which the CLI originate might timeout before all 
> calls in the chain were dialed.
> 
> * Call setup is now tracked to so the test can better distinguish when calls 
> hangup if they were optimizations or timeout hangups.
> 
> * Added some helpful test failure diagnosis messages when the reactor is 
> stopped.
> 
> * Added needed debug routines to get "core show locks" output into the test 
> log.
> 
> NOTE:  This is an older test that was written before PEP8 was more rigidly 
> enforced in review.  Another patch will be made to clear up PEP8 formatting 
> compliance.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/masquerade/run-test 4926 
>   /asterisk/trunk/tests/masquerade/configs/ast1/extensions.conf 4926 
> 
> Diff: https://reviewboard.asterisk.org/r/3412/diff/
> 
> 
> Testing
> ---
> 
> I've done many test runs looking for why the masquerade test fails and I have 
> seen just about all the failure cases reported.
> 
> The core show locks code works when I uncomment and comment the right lines.  
> It showed that the problem was not a deadlock on Asterisk v12.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3414: internal_timing: Remove the option and always make it enabled if a timing module is loaded.

2014-04-03 Thread Mark Michelson

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

Ship it!


Ship It!

- Mark Michelson


On April 3, 2014, 4:52 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3414/
> ---
> 
> (Updated April 3, 2014, 4:52 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22846
> https://issues.asterisk.org/jira/browse/ASTERISK-22846
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The masquerade supertest frequently fails because either the local channel 
> chain doesn't completely optimize out or the DTMF handshake doesn't 
> completely get accross.  Local channel optimization requires frames flowing 
> to trigger when optimization can happen.  When optimization happens the media 
> frame that triggered the optimization is dropped.  Sending DTMF requires 
> frames to flow in the other direction for timing purposes while sending 
> nothing.  If internal timing is not enabled when MOH is playing, Asterisk 
> switches to received timing when an audio frame is received.  With 
> optimization dropping media frames and MOH not sending frames unless it 
> receives frames, occasionaly there are no more frames being passed and the 
> test fails.
> 
> * The asterisk command line -I option and the asterisk.conf internal_timing 
> option are removed.  Asterisk now always uses internal timing when needed if 
> any timing module is loaded.  The issue ASTERISK-14861 did this quite awhile 
> ago in v1.4 but effectively got broken when other internal timing modules 
> besides DAHDI came along.  The ast_read_generator_actions() now only uses 
> received timing if it has no choice for frame generators like MOH, silence, 
> and playback streaming.
> 
> * Cleaned up some code dealing with frame generators in 
> ast_deactivate_generator(), generator_write_format_change(), 
> ast_activate_generator(), and ast_channel_stop_silence_generator().
> 
> * Removed ast_internal_timing_enabled(), AST_OPT_FLAG_INTERNAL_TIMING, and 
> ast_opt_internal_timing.  The v1.8 and v11 versions (and possibly v12) will 
> not have these changes as they change the API.
> 
> Question:  Should v12 get the API change above or just trunk?
> 
> 
> Diffs
> -
> 
>   /branches/12/utils/extconf.c 411684 
>   /branches/12/main/channel.c 411684 
>   /branches/12/main/asterisk.c 411684 
>   /branches/12/include/asterisk/options.h 411684 
>   /branches/12/include/asterisk/channel.h 411684 
>   /branches/12/configs/asterisk.conf.sample 411684 
>   /branches/12/channels/chan_sip.c 411684 
>   /branches/12/UPGRADE.txt 411684 
> 
> Diff: https://reviewboard.asterisk.org/r/3414/diff/
> 
> 
> Testing
> ---
> 
> With the v1.8 and v12 versions of the patch, Asterisk no longer fails the 
> masquerade supertest.  The v11 version should be similar to v1.8.
> 
> v12 takes 15-16 seconds to run on the 64 bit build agent.
> v1.8 takes 21-22 seconds to run on the 64 bit build agent.
> 
> As a side note, the counting instrumentation code I added to v12 to look for 
> this problem showed there was almost no lock contention when grabbing the 7 
> locks needed for optimization.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3414: internal_timing: Remove the option and always make it enabled if a timing module is loaded.

2014-04-03 Thread rmudgett


> On April 3, 2014, 1:10 p.m., Mark Michelson wrote:
> > /branches/12/main/channel.c, line 3664
> > 
> >
> > In chan_sip.c the replacement for ast_internal_timing_enabled() was to 
> > check if chan->timingfd was valid. Here, you check for the existence of 
> > ast_channel_timingfunc(). What's the reason for the difference?

The chan_sip.c test checks to see if internal timing is available.  The 
timingfunc test checks to see if frames are being generated by the timerfd at 
this time.


> On April 3, 2014, 1:10 p.m., Mark Michelson wrote:
> > /branches/12/main/channel.c, lines 3668-3671
> > 
> >
> > Any reason you killed the debug message that used to announce this?

It is a comment now rather than a debug message because we no longer switch 
between internal timing and loop timing modes.  We are always one or the other 
when we are generating frames.


- rmudgett


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


On April 3, 2014, 11:52 a.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3414/
> ---
> 
> (Updated April 3, 2014, 11:52 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22846
> https://issues.asterisk.org/jira/browse/ASTERISK-22846
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The masquerade supertest frequently fails because either the local channel 
> chain doesn't completely optimize out or the DTMF handshake doesn't 
> completely get accross.  Local channel optimization requires frames flowing 
> to trigger when optimization can happen.  When optimization happens the media 
> frame that triggered the optimization is dropped.  Sending DTMF requires 
> frames to flow in the other direction for timing purposes while sending 
> nothing.  If internal timing is not enabled when MOH is playing, Asterisk 
> switches to received timing when an audio frame is received.  With 
> optimization dropping media frames and MOH not sending frames unless it 
> receives frames, occasionaly there are no more frames being passed and the 
> test fails.
> 
> * The asterisk command line -I option and the asterisk.conf internal_timing 
> option are removed.  Asterisk now always uses internal timing when needed if 
> any timing module is loaded.  The issue ASTERISK-14861 did this quite awhile 
> ago in v1.4 but effectively got broken when other internal timing modules 
> besides DAHDI came along.  The ast_read_generator_actions() now only uses 
> received timing if it has no choice for frame generators like MOH, silence, 
> and playback streaming.
> 
> * Cleaned up some code dealing with frame generators in 
> ast_deactivate_generator(), generator_write_format_change(), 
> ast_activate_generator(), and ast_channel_stop_silence_generator().
> 
> * Removed ast_internal_timing_enabled(), AST_OPT_FLAG_INTERNAL_TIMING, and 
> ast_opt_internal_timing.  The v1.8 and v11 versions (and possibly v12) will 
> not have these changes as they change the API.
> 
> Question:  Should v12 get the API change above or just trunk?
> 
> 
> Diffs
> -
> 
>   /branches/12/utils/extconf.c 411684 
>   /branches/12/main/channel.c 411684 
>   /branches/12/main/asterisk.c 411684 
>   /branches/12/include/asterisk/options.h 411684 
>   /branches/12/include/asterisk/channel.h 411684 
>   /branches/12/configs/asterisk.conf.sample 411684 
>   /branches/12/channels/chan_sip.c 411684 
>   /branches/12/UPGRADE.txt 411684 
> 
> Diff: https://reviewboard.asterisk.org/r/3414/diff/
> 
> 
> Testing
> ---
> 
> With the v1.8 and v12 versions of the patch, Asterisk no longer fails the 
> masquerade supertest.  The v11 version should be similar to v1.8.
> 
> v12 takes 15-16 seconds to run on the 64 bit build agent.
> v1.8 takes 21-22 seconds to run on the 64 bit build agent.
> 
> As a side note, the counting instrumentation code I added to v12 to look for 
> this problem showed there was almost no lock contention when grabbing the 7 
> locks needed for optimization.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3363: Testsuite: Pluggable module for testing realtime

2014-04-03 Thread Mark Michelson


> On March 25, 2014, 5:42 p.m., opticron wrote:
> > /asterisk/trunk/lib/python/asterisk/realtime_test_module.py, line 322
> > 
> >
> > Just to make sure I'm clear here, we're throwing away wildcards (%) and 
> > treating them as literal matches?
> > 
> > res_config_curl doesn't appear to directly use wildcards, but if 
> > something can use them through res_config_curl, this is going to break.

Yes, agreed that this could be problematic. After looking at queries a bit 
more, especially when sorcery is used, this has the potential to fail badly.


> On March 25, 2014, 5:42 p.m., opticron wrote:
> > /asterisk/trunk/lib/python/asterisk/realtime_test_module.py, line 335
> > 
> >
> > This resource may need "LIKE" handling as well.

Welp, this hits on the concepts of "in theory" vs. "in practice". SQL certainly 
has the ability to have wildcards in any where clause, but in practice, 
Asterisk only ever does this when retrieving multiple records.

So based on what is in Asterisk, and what is likely to ever be in Asterisk, 
there is no need to add the LIKE handling here. However, given what I'm likely 
going to have to do in order to fix the wildcard handling, adding LIKE handling 
everywhere I can is probably fine.


- Mark


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


On March 15, 2014, 6:34 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3363/
> ---
> 
> (Updated March 15, 2014, 6:34 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This introduces a new pluggable module to the Asterisk testsuite intended to 
> assist in testing using realtime storage. The test module sets up an HTTP 
> server on port 8000 and services requests Asterisk makes using its 
> res_config_curl realtime backend. The actual data is stored in memory in 
> python using simple dictionaries and lists. The test module configuration 
> allows for data to be preloaded into python before Asterisk is started so 
> that Asterisk may retrieve realtime data during startup. Once AMI is 
> connected to Asterisk, the realtime test module hands over control to a 
> python module so that the test may be further controlled by the test writer.
> 
> Along with the module are six realtime tests, each designed to test an 
> operation that Asterisk can attempt. The "require" operation is not tested 
> since we currently always claim to have the proper items stored in the 
> expected way.
> 
> There are potential improvements that could be made, such as:
> * Memoization of HTTP resources served by the test module.
> * yaml-driven test development beyond the initial population of realtime data.
> * Potentially allow for the realtime test module to be run stand-alone so 
> that developers can have an easy realtime store to use for testing.
> 
> None of these are enough to prevent inclusion into the test suite though.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/tests.yaml 4836 
>   /asterisk/trunk/tests/realtime/update/update.py PRE-CREATION 
>   /asterisk/trunk/tests/realtime/update/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/realtime/update/configs/ast1/sorcery.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/update/configs/ast1/extconfig.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/realtime/store/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/realtime/store/store.py PRE-CREATION 
>   /asterisk/trunk/tests/realtime/store/configs/ast1/sorcery.conf PRE-CREATION 
>   /asterisk/trunk/tests/realtime/store/configs/ast1/extconfig.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/static/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/realtime/static/static.py PRE-CREATION 
>   /asterisk/trunk/tests/realtime/static/configs/ast1/modules.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/static/configs/ast1/extconfig.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/single/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/realtime/single/single.py PRE-CREATION 
>   /asterisk/trunk/tests/realtime/single/configs/ast1/sorcery.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/single/configs/ast1/extconfig.conf 
> PRE-CREATION 
>   /asterisk/trunk/tests/realtime/multi/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/realtime/multi/multi.py PRE-CREATION 
>   /asterisk/trunk/tests/realtime/multi/configs/ast1/

Re: [asterisk-dev] [Code Review] 3339: Testsuite: ARI test for playback of audio to a channel in a bridge.

2014-04-03 Thread Mark Michelson

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

(Updated April 3, 2014, 8:03 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Kevin's comment about removing unnecessary comments. For the time 
being, the decorator will stay here. Making the decorator part of the ARI 
library would be a change to be made at a later date potentially.


Repository: testsuite


Description
---

This was created to test the changes in /r/3338

This test is pretty simple. A channel is placed into an ARI bridge, and two 
playbacks are queued up in quick succession. The test ensures that the files 
play and that the first file plays and finishes before the second file plays 
and finishes.


Diffs (updated)
-

  /asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/test-config.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/play_file.py 
PRE-CREATION 
  /asterisk/trunk/tests/rest_api/playback/tests.yaml 4836 

Diff: https://reviewboard.asterisk.org/r/3339/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] 3403: Test for channel Pickup

2014-04-03 Thread Benjamin Keith Ford

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

(Updated April 3, 2014, 7:39 p.m.)


Review request for Asterisk Developers.


Changes
---

- Updated minversion


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


Repository: testsuite


Description
---

This test verifies that the following scenarios work for the Pickup application:

1. A channel starts to dial an IAX peer. While ringing, another channel picks 
up the first.
2. A channel starts to dial an IAX peer, and joins a pickup group. While 
ringing, another channel picks up the first from that pickup group.
3. A channel sets PICKUPMARK equal to a value and starts to dial an IAX peer. 
While ringing, another channel picks up the first using the PICKUPMARK method 
of the Pickup application.


Diffs (updated)
-

  ./asterisk/trunk/tests/apps/tests.yaml 4903 
  ./asterisk/trunk/tests/apps/directed_pickup/tests.yaml PRE-CREATION 
  ./asterisk/trunk/tests/apps/directed_pickup/test-config.yaml 4903 
  ./asterisk/trunk/tests/apps/directed_pickup/run-test 4903 
  ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/test-config.yaml 
PRE-CREATION 
  ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/run-test PRE-CREATION 
  ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/configs/ast1/iax.conf 
PRE-CREATION 
  
./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/configs/ast1/extensions.conf
 PRE-CREATION 
  ./asterisk/trunk/tests/apps/directed_pickup/pickup/test-config.yaml 
PRE-CREATION 
  ./asterisk/trunk/tests/apps/directed_pickup/pickup/configs/ast1/iax.conf 
PRE-CREATION 
  
./asterisk/trunk/tests/apps/directed_pickup/pickup/configs/ast1/extensions.conf 
PRE-CREATION 
  ./asterisk/trunk/tests/apps/directed_pickup/configs/ast1/iax.conf 4903 
  ./asterisk/trunk/tests/apps/directed_pickup/configs/ast1/extensions.conf 4903 

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


Testing
---


Thanks,

Benjamin Keith Ford

-- 
_
-- 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] 3403: Test for channel Pickup

2014-04-03 Thread Mark Michelson

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



./asterisk/trunk/tests/apps/directed_pickup/pickup/test-config.yaml


Since you're using a pre-dial handler in this test, you'll need to set the 
minversion to 11.0.0

Asterisk 1.8 did not have predial handlers.


- Mark Michelson


On April 1, 2014, 8 p.m., Benjamin Keith Ford wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3403/
> ---
> 
> (Updated April 1, 2014, 8 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23520
> https://issues.asterisk.org/jira/browse/ASTERISK-23520
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This test verifies that the following scenarios work for the Pickup 
> application:
> 
> 1. A channel starts to dial an IAX peer. While ringing, another channel picks 
> up the first.
> 2. A channel starts to dial an IAX peer, and joins a pickup group. While 
> ringing, another channel picks up the first from that pickup group.
> 3. A channel sets PICKUPMARK equal to a value and starts to dial an IAX peer. 
> While ringing, another channel picks up the first using the PICKUPMARK method 
> of the Pickup application.
> 
> 
> Diffs
> -
> 
>   ./asterisk/trunk/tests/apps/tests.yaml 4903 
>   ./asterisk/trunk/tests/apps/directed_pickup/tests.yaml PRE-CREATION 
>   ./asterisk/trunk/tests/apps/directed_pickup/test-config.yaml 4903 
>   ./asterisk/trunk/tests/apps/directed_pickup/run-test 4903 
>   ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/test-config.yaml 
> PRE-CREATION 
>   ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/run-test 
> PRE-CREATION 
>   
> ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/configs/ast1/iax.conf 
> PRE-CREATION 
>   
> ./asterisk/trunk/tests/apps/directed_pickup/pickup_chan/configs/ast1/extensions.conf
>  PRE-CREATION 
>   ./asterisk/trunk/tests/apps/directed_pickup/pickup/test-config.yaml 
> PRE-CREATION 
>   ./asterisk/trunk/tests/apps/directed_pickup/pickup/configs/ast1/iax.conf 
> PRE-CREATION 
>   
> ./asterisk/trunk/tests/apps/directed_pickup/pickup/configs/ast1/extensions.conf
>  PRE-CREATION 
>   ./asterisk/trunk/tests/apps/directed_pickup/configs/ast1/iax.conf 4903 
>   ./asterisk/trunk/tests/apps/directed_pickup/configs/ast1/extensions.conf 
> 4903 
> 
> Diff: https://reviewboard.asterisk.org/r/3403/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Keith Ford
> 
>

-- 
_
-- 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] 3407: Test Suite: Nominal caller initiated blind transfer tests using PJSIP

2014-04-03 Thread Mark Michelson

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



/asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/test-config.yaml


The comments here contradict what is said in the test description. The test 
description says that Alice transfers Bob to Charlie, but these comments say 
that Bob transfers Alice to Charlie.

Same applies for the second scenario.



/asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/transfer.py


IMO, log_call_info would be more sane if it just took in self.call.info() 
as its only parameter and then extracted what it cared about afterwards.



/asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/transfer.py


From on_transfer_status callback documentation:

"Return:
If the callback returns false then no further notification will
be sent for the transfer request for this call."

So in other words you're supposed to return something. The docs suggest 
returning cont.



/asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/test-config.yaml


Same comment here as for the earlier SIPp test: The comments above the 
scenarios contradict what the description states.


- Mark Michelson


On March 29, 2014, 5:59 a.m., jbigelow wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3407/
> ---
> 
> (Updated March 29, 2014, 5:59 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23446
> https://issues.asterisk.org/jira/browse/ASTERISK-23446
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> These tests cover nominal caller initiated blind transfer tests using PJSIP.
> 
> Note: All three tests currently fail due to the issues described in 
> ASTERISK-23501 & ASTERISK-23502.
> 
> Each test ensures the presents and values of the following:
> * channel variables SIPREFERREDBYHDR, SIPREFERTOHDR, SIPTRANSFER, and 
> SIPREFERRINGCONTEXT.
> * the BlindTransfer event.
> * the 'Referred-By' header in the INVITE sent to Charlie.
> 
> Each test also sets the TRANSFER_CONTEXT channel variable to ensure the 
> transfer still occurs properly. The 'caller_with_hold' test additionally 
> requires and checks the MusicOnHoldStart & MusicOnHoldStop events.
> 
> Tests:
> * caller_refer_only: Uses PJSua library. Basic blind transfer without a hold 
> or direct media being performed at any time. Changes were required to the 
> pjsua_mod.py library to be able to associate pjsua accounts with a specific 
> pjsua transport.
> * caller_direct_media: Uses SIPp. Blind transfer with direct media between 
> the endpoints before and after the transfer.
> * caller_with_hold: Uses SIPp. Blind transfer with putting the callee on hold 
> before the transfer.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/channels/pjsip/transfers/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/tests.yaml 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/test-config.yaml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/sipp/charlie.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/sipp/bob.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/sipp/alice.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/configs/ast1/pjsip.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/configs/ast1/extensions.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/transfer.py
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/test-config.yaml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/configs/ast1/pjsip.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/configs/ast1/extensions.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/test-config.yaml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/call

[asterisk-dev] [Code Review] 3415: bridging: Ensure proper locking during snapshot creation

2014-04-03 Thread opticron

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

While the vast majority of bridge snapshot creation is locked properly, there 
are currently some instances that are not. This adds the missing locking to 
ensure bridge state is not malleable during snapshot creation.


Diffs
-

  branches/12/tests/test_cel.c 411576 
  branches/12/res/ari/resource_bridges.c 411576 
  branches/12/main/bridge_basic.c 411576 
  branches/12/main/bridge.c 411576 
  branches/12/apps/app_confbridge.c 411576 

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


Testing
---

Ran the testsuite to ensure that it did not cause failures.


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] 3407: Test Suite: Nominal caller initiated blind transfer tests using PJSIP

2014-04-03 Thread opticron

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


The pjsip configs here could benefit heavily from use of templates.


/asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/test-config.yaml


Unless the value of SIPTRANSFER could be set to something else to indicate 
failure (it doesn't appear that is the case), this should go up with the 
conditions and the requirements section can just be dropped since it is 
optional. This also applies to the other usages of the requirements section 
here and in the other two tests.


- opticron


On March 29, 2014, 12:59 a.m., jbigelow wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3407/
> ---
> 
> (Updated March 29, 2014, 12:59 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23446
> https://issues.asterisk.org/jira/browse/ASTERISK-23446
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> These tests cover nominal caller initiated blind transfer tests using PJSIP.
> 
> Note: All three tests currently fail due to the issues described in 
> ASTERISK-23501 & ASTERISK-23502.
> 
> Each test ensures the presents and values of the following:
> * channel variables SIPREFERREDBYHDR, SIPREFERTOHDR, SIPTRANSFER, and 
> SIPREFERRINGCONTEXT.
> * the BlindTransfer event.
> * the 'Referred-By' header in the INVITE sent to Charlie.
> 
> Each test also sets the TRANSFER_CONTEXT channel variable to ensure the 
> transfer still occurs properly. The 'caller_with_hold' test additionally 
> requires and checks the MusicOnHoldStart & MusicOnHoldStop events.
> 
> Tests:
> * caller_refer_only: Uses PJSua library. Basic blind transfer without a hold 
> or direct media being performed at any time. Changes were required to the 
> pjsua_mod.py library to be able to associate pjsua accounts with a specific 
> pjsua transport.
> * caller_direct_media: Uses SIPp. Blind transfer with direct media between 
> the endpoints before and after the transfer.
> * caller_with_hold: Uses SIPp. Blind transfer with putting the callee on hold 
> before the transfer.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/channels/pjsip/transfers/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/tests.yaml 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/test-config.yaml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/sipp/charlie.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/sipp/bob.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/sipp/alice.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/configs/ast1/pjsip.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_with_hold/configs/ast1/extensions.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/transfer.py
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/test-config.yaml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/configs/ast1/pjsip.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_refer_only/configs/ast1/extensions.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/test-config.yaml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/sipp/charlie.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/sipp/bob.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/sipp/alice.xml
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/configs/ast1/pjsip.conf
>  PRE-CREATION 
>   
> /asterisk/trunk/tests/channels/pjsip/transfers/blind_transfer/caller_direct_media/configs/ast1/extensions.conf
>  PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/tests.yaml 4911 
>   /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 4911 
> 
> Diff: https://reviewboard.asterisk.org/r/3407/diff/
> 
> 
> Testing
> ---
> 
> * Added a pre-dial handler when dialing charlie to add the Referred-By
> header and commented out the header match for the S

Re: [asterisk-dev] [Code Review] 3413: media_formats: move app.c and file.c over

2014-04-03 Thread Mark Michelson

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

Ship it!



team/kharwell/media_formats_app_file/main/file.c


Since you're poking in this area, I suggest placing curly braces on these 
if statements.


- Mark Michelson


On April 2, 2014, 10:02 p.m., Kevin Harwell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3413/
> ---
> 
> (Updated April 2, 2014, 10:02 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This moves app.c and file.c over to the media formats API.
> 
> 
> Diffs
> -
> 
>   team/kharwell/media_formats_app_file/main/file.c 411583 
>   team/kharwell/media_formats_app_file/main/app.c 411583 
>   team/kharwell/media_formats_app_file/include/asterisk/format.h 411583 
> 
> Diff: https://reviewboard.asterisk.org/r/3413/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-- 
_
-- 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] 3412: testsuite: Add call setup tracking to masquerade supertest.

2014-04-03 Thread Mark Michelson

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


I recommend running your changes through a PEP8 checker of some sort (such as 
pylint). I'll point out some of the violations I see, but a static checker will 
point out others.


/asterisk/trunk/tests/masquerade/run-test


When using named parameters in calls, don't put spaces around the = sign:

Good: obj.foo(param="eggs")
Bad: obj.foo(param = "eggs")

This applies to all of the originations you've modified in this file.



/asterisk/trunk/tests/masquerade/run-test


Just use

logger.info(cli_command.output)


- Mark Michelson


On April 2, 2014, 12:09 a.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3412/
> ---
> 
> (Updated April 2, 2014, 12:09 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22846
> https://issues.asterisk.org/jira/browse/ASTERISK-22846
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> * Changed originated calls to use AMI originate instead of CLI originate so a 
> custom call timeout could be specified.  The number of calls in the chain 
> could take awhile to setup which the CLI originate might timeout before all 
> calls in the chain were dialed.
> 
> * Call setup is now tracked to so the test can better distinguish when calls 
> hangup if they were optimizations or timeout hangups.
> 
> * Added some helpful test failure diagnosis messages when the reactor is 
> stopped.
> 
> * Added needed debug routines to get "core show locks" output into the test 
> log.
> 
> NOTE:  This is an older test that was written before PEP8 was more rigidly 
> enforced in review.  Another patch will be made to clear up PEP8 formatting 
> compliance.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/masquerade/run-test 4926 
>   /asterisk/trunk/tests/masquerade/configs/ast1/extensions.conf 4926 
> 
> Diff: https://reviewboard.asterisk.org/r/3412/diff/
> 
> 
> Testing
> ---
> 
> I've done many test runs looking for why the masquerade test fails and I have 
> seen just about all the failure cases reported.
> 
> The core show locks code works when I uncomment and comment the right lines.  
> It showed that the problem was not a deadlock on Asterisk v12.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3411: Add some asserts that were handy when looking for a stasis cache problem.

2014-04-03 Thread Mark Michelson

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

Ship it!


Ship It!

- Mark Michelson


On April 1, 2014, 11:41 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3411/
> ---
> 
> (Updated April 1, 2014, 11:41 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> * Assert if a channel is destroyed but has the snapshot staging flag set.  In 
> this case the final channel destruction snapshot would never get taken.
> 
> * Assert if what we just got out of the stasis cache is not what we were 
> looking for.  This assert would have saved several days searching for a bug 
> and a lot of my hair.
> 
> * Assert if the music on hold message posts could not find the associated 
> channel.  A crash will happen later when manager tries to send the MOH AMI 
> message.  This assert catches the problem when the stasis message is posted 
> instead of by the thread processing the defective message.
> 
> * Always generate a backtrace when an ast_assert() fails.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_musiconhold.c 411651 
>   /branches/12/main/utils.c 411651 
>   /branches/12/main/stasis_cache.c 411651 
>   /branches/12/main/channel.c 411651 
> 
> Diff: https://reviewboard.asterisk.org/r/3411/diff/
> 
> 
> Testing
> ---
> 
> Inserted an assertion that failed to see that the backtrace is generated.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3414: internal_timing: Remove the option and always make it enabled if a timing module is loaded.

2014-04-03 Thread Mark Michelson

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



/branches/12/main/channel.c


In chan_sip.c the replacement for ast_internal_timing_enabled() was to 
check if chan->timingfd was valid. Here, you check for the existence of 
ast_channel_timingfunc(). What's the reason for the difference?



/branches/12/main/channel.c


Any reason you killed the debug message that used to announce this?


- Mark Michelson


On April 3, 2014, 4:52 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3414/
> ---
> 
> (Updated April 3, 2014, 4:52 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22846
> https://issues.asterisk.org/jira/browse/ASTERISK-22846
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The masquerade supertest frequently fails because either the local channel 
> chain doesn't completely optimize out or the DTMF handshake doesn't 
> completely get accross.  Local channel optimization requires frames flowing 
> to trigger when optimization can happen.  When optimization happens the media 
> frame that triggered the optimization is dropped.  Sending DTMF requires 
> frames to flow in the other direction for timing purposes while sending 
> nothing.  If internal timing is not enabled when MOH is playing, Asterisk 
> switches to received timing when an audio frame is received.  With 
> optimization dropping media frames and MOH not sending frames unless it 
> receives frames, occasionaly there are no more frames being passed and the 
> test fails.
> 
> * The asterisk command line -I option and the asterisk.conf internal_timing 
> option are removed.  Asterisk now always uses internal timing when needed if 
> any timing module is loaded.  The issue ASTERISK-14861 did this quite awhile 
> ago in v1.4 but effectively got broken when other internal timing modules 
> besides DAHDI came along.  The ast_read_generator_actions() now only uses 
> received timing if it has no choice for frame generators like MOH, silence, 
> and playback streaming.
> 
> * Cleaned up some code dealing with frame generators in 
> ast_deactivate_generator(), generator_write_format_change(), 
> ast_activate_generator(), and ast_channel_stop_silence_generator().
> 
> * Removed ast_internal_timing_enabled(), AST_OPT_FLAG_INTERNAL_TIMING, and 
> ast_opt_internal_timing.  The v1.8 and v11 versions (and possibly v12) will 
> not have these changes as they change the API.
> 
> Question:  Should v12 get the API change above or just trunk?
> 
> 
> Diffs
> -
> 
>   /branches/12/utils/extconf.c 411684 
>   /branches/12/main/channel.c 411684 
>   /branches/12/main/asterisk.c 411684 
>   /branches/12/include/asterisk/options.h 411684 
>   /branches/12/include/asterisk/channel.h 411684 
>   /branches/12/configs/asterisk.conf.sample 411684 
>   /branches/12/channels/chan_sip.c 411684 
>   /branches/12/UPGRADE.txt 411684 
> 
> Diff: https://reviewboard.asterisk.org/r/3414/diff/
> 
> 
> Testing
> ---
> 
> With the v1.8 and v12 versions of the patch, Asterisk no longer fails the 
> masquerade supertest.  The v11 version should be similar to v1.8.
> 
> v12 takes 15-16 seconds to run on the 64 bit build agent.
> v1.8 takes 21-22 seconds to run on the 64 bit build agent.
> 
> As a side note, the counting instrumentation code I added to v12 to look for 
> this problem showed there was almost no lock contention when grabbing the 7 
> locks needed for optimization.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] [Code Review] 3414: internal_timing: Remove the option and always make it enabled if a timing module is loaded.

2014-04-03 Thread rmudgett

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

The masquerade supertest frequently fails because either the local channel 
chain doesn't completely optimize out or the DTMF handshake doesn't completely 
get accross.  Local channel optimization requires frames flowing to trigger 
when optimization can happen.  When optimization happens the media frame that 
triggered the optimization is dropped.  Sending DTMF requires frames to flow in 
the other direction for timing purposes while sending nothing.  If internal 
timing is not enabled when MOH is playing, Asterisk switches to received timing 
when an audio frame is received.  With optimization dropping media frames and 
MOH not sending frames unless it receives frames, occasionaly there are no more 
frames being passed and the test fails.

* The asterisk command line -I option and the asterisk.conf internal_timing 
option are removed.  Asterisk now always uses internal timing when needed if 
any timing module is loaded.  The issue ASTERISK-14861 did this quite awhile 
ago in v1.4 but effectively got broken when other internal timing modules 
besides DAHDI came along.  The ast_read_generator_actions() now only uses 
received timing if it has no choice for frame generators like MOH, silence, and 
playback streaming.

* Cleaned up some code dealing with frame generators in 
ast_deactivate_generator(), generator_write_format_change(), 
ast_activate_generator(), and ast_channel_stop_silence_generator().

* Removed ast_internal_timing_enabled(), AST_OPT_FLAG_INTERNAL_TIMING, and 
ast_opt_internal_timing.  The v1.8 and v11 versions (and possibly v12) will not 
have these changes as they change the API.

Question:  Should v12 get the API change above or just trunk?


Diffs
-

  /branches/12/utils/extconf.c 411684 
  /branches/12/main/channel.c 411684 
  /branches/12/main/asterisk.c 411684 
  /branches/12/include/asterisk/options.h 411684 
  /branches/12/include/asterisk/channel.h 411684 
  /branches/12/configs/asterisk.conf.sample 411684 
  /branches/12/channels/chan_sip.c 411684 
  /branches/12/UPGRADE.txt 411684 

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


Testing
---

With the v1.8 and v12 versions of the patch, Asterisk no longer fails the 
masquerade supertest.  The v11 version should be similar to v1.8.

v12 takes 15-16 seconds to run on the 64 bit build agent.
v1.8 takes 21-22 seconds to run on the 64 bit build agent.

As a side note, the counting instrumentation code I added to v12 to look for 
this problem showed there was almost no lock contention when grabbing the 7 
locks needed for optimization.


Thanks,

rmudgett

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3383: TestSuite: Fix bouncing show_subscriptions test

2014-04-03 Thread opticron

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

(Updated April 3, 2014, 6:57 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 411670


Repository: testsuite


Description
---

This fixes a race condition in the tests/channels/pjsip/ami/show_subscriptions 
test where the PJSIPShowSubscriptionsInbound AMI command would be issued before 
states had finished transitioning resulting in the response containing "NULL" 
instead of "ACTIVE". This also requires the following patch that adds a test 
event when Asterisk is compiled in development mode:

Index: res/res_pjsip_pubsub.c
===
--- res/res_pjsip_pubsub.c  (revision 410980)
+++ res/res_pjsip_pubsub.c  (working copy)
@@ -42,6 +42,7 @@
 #include "asterisk/res_pjsip.h"
 #include "asterisk/callerid.h"
 #include "asterisk/manager.h"
+#include "asterisk/test.h"
 #include "res_pjsip/include/res_pjsip_private.h"

 /*** DOCUMENTATION
@@ -464,8 +465,18 @@

 int ast_sip_subscription_send_request(struct ast_sip_subscription *sub, 
pjsip_tx_data *tdata)
 {
-   return pjsip_evsub_send_request(ast_sip_subscription_get_evsub(sub),
+   struct ast_sip_endpoint *endpoint = 
ast_sip_subscription_get_endpoint(sub);
+   int res = pjsip_evsub_send_request(ast_sip_subscription_get_evsub(sub),
tdata) == PJ_SUCCESS ? 0 : -1;
+
+   ast_test_suite_event_notify("SUBSCRIPTION_STATE_SET",
+   "StateText: %s\r\n"
+   "Endpoint: %s\r\n",
+   pjsip_evsub_get_state_name(ast_sip_subscription_get_evsub(sub)),
+   ast_sorcery_object_get_id(endpoint));
+
+   ao2_cleanup(endpoint);
+   return res;
 }

 static void subscription_datastore_destroy(void *obj)


Diffs
-

  asterisk/trunk/tests/channels/pjsip/ami/show_subscriptions/AMISendTest.py 
4885 

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


Testing
---

Verified that the race was resolved by using state transition test events 
instead of the successful callback of the SIPp scenario.


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] 3398: TestSuite: Fix tests affected by 411086

2014-04-03 Thread Matt Jordan

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

Ship it!


Since we noted this behavioural correction in the UPGRADE notes for Asterisk 
12.2.0, I'd say Ship It! This does correct the tests for the behaviour in 
Asterisk post the ref leak cleanup, which is how this should behave.

- Matt Jordan


On March 26, 2014, 8 a.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3398/
> ---
> 
> (Updated March 26, 2014, 8 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This fixes the two tests affected by the fix (r411086) to the bridge 
> subscription leak. They were depending on the leaked subscription to catch 
> bridge destruction events instead of holding an explicit subscription for 
> that purpose. They now explicitly subscribe to and unsubscribe from the 
> bridge at creation and destruction as necessary.
> 
> 
> Diffs
> -
> 
>   asterisk/trunk/tests/rest_api/bridges/delete/bridge_delete.py 4885 
>   asterisk/trunk/tests/rest_api/bridges/bridge_by_id/test-config.yaml 4885 
> 
> Diff: https://reviewboard.asterisk.org/r/3398/diff/
> 
> 
> Testing
> ---
> 
> Made sure the tests got the events they were expecting.
> 
> 
> 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