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

2014-04-05 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated April 5, 2014, 8:06 a.m.) Status -- This change has been

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

2014-04-04 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated April 4, 2014, 3:42 p.m.) Review request for Asterisk

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

2014-04-04 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11498 --- Ship it! All my concerns are addressed. - Corey Farrell On

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 https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line48 Document the (On write) hook_id. Is this the first function that has different parameters for read and

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

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

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 https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line85 These should not be uppercase. That indicates they are macros. Russell Bryant wrote: Hrm, I think of

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

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

2014-04-02 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11472 --- /trunk/funcs/func_periodic_hook.c

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

2014-04-01 Thread Russell Bryant
On March 31, 2014, 5:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 141-143 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141 Macro is deprecated, why not use Gosub? Russell Bryant wrote: I tried that. GoSub() doesn't

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

2014-04-01 Thread Russell Bryant
On March 31, 2014, 5:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 141-143 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141 Macro is deprecated, why not use Gosub? Russell Bryant wrote: I tried that. GoSub() doesn't

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

2014-04-01 Thread Russell Bryant
On March 31, 2014, 5:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 141-143 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141 Macro is deprecated, why not use Gosub? Russell Bryant wrote: I tried that. GoSub() doesn't

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

2014-04-01 Thread Russell Bryant
On March 31, 2014, 5:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 141-143 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141 Macro is deprecated, why not use Gosub? Russell Bryant wrote: I tried that. GoSub() doesn't

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

2014-04-01 Thread Russell Bryant
--- 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

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

2014-03-31 Thread Russell Bryant
On March 30, 2014, 12:12 a.m., Corey Farrell wrote: Do you have any ideas on how this will effect CPU load? I was thinking of this as an example: A system with 1 inbound call per second, each call lasting 5 minutes. Every inbound channel gets a 60 second timer for beep Playback

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

2014-03-31 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 31, 2014, 12:56 p.m.) Review request for Asterisk

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

2014-03-31 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 31, 2014, 1:14 p.m.) Review request for Asterisk

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

2014-03-31 Thread jamuel
On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 206 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206 state-last_hook is never pre-initialized to ast_tvnow(), so hooks would always trigger on first check after

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

2014-03-31 Thread Russell Bryant
On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 206 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206 state-last_hook is never pre-initialized to ast_tvnow(), so hooks would always trigger on first check after

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

2014-03-31 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 31, 2014, 4:11 p.m.) Review request for Asterisk

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

2014-03-31 Thread jamuel
On March 30, 2014, 12:12 a.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 206 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206 state-last_hook is never pre-initialized to ast_tvnow(), so hooks would always trigger on first check after

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

2014-03-31 Thread Corey Farrell
On March 29, 2014, 8:12 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, line 299 https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line299 Can we detach the audio hook here? I know we escape the hook callback very early but it still feels wasteful

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

2014-03-31 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11455 --- /trunk/funcs/func_periodic_hook.c

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

2014-03-31 Thread Russell Bryant
On March 31, 2014, 5:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 136-139 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line136 I'm unsure the purpose of beep in macro_arg. Also think chan_name + hook_id can never be close

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

2014-03-31 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 31, 2014, 8:33 p.m.) Review request for Asterisk

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

2014-03-31 Thread rmudgett
On March 31, 2014, 12:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 141-143 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141 Macro is deprecated, why not use Gosub? Russell Bryant wrote: I tried that. GoSub() doesn't

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

2014-03-31 Thread Russell Bryant
On March 31, 2014, 5:58 p.m., Corey Farrell wrote: /trunk/funcs/func_periodic_hook.c, lines 141-143 https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141 Macro is deprecated, why not use Gosub? Russell Bryant wrote: I tried that. GoSub() doesn't

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

2014-03-29 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 29, 2014, 8:06 p.m.) Review request for Asterisk

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

2014-03-29 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 29, 2014, 8:11 p.m.) Review request for Asterisk

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

2014-03-29 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 29, 2014, 8:27 p.m.) Review request for Asterisk

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

2014-03-29 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11441 --- Do you have any ideas on how this will effect CPU load? I was