dgorski / app_tdd

TDD Module for Asterisk
3 stars 2 forks source link

app_tdd: Add TddWait and TddStop applications. #19

Closed InterLinked1 closed 1 year ago

InterLinked1 commented 1 year ago

Fixes: #18

dgorski commented 1 year ago

There are WAY too many unnecessary changes in this to review.  I cannot merge this.

Never mix code changes/additions with adjustments for coding standards or trivial uptates to comments. It is way to hard to review this way.

InterLinked1 commented 1 year ago

There are WAY too many unnecessary changes in this to review.  I cannot merge this.

Never mix code changes/additions with adjustments for coding standards or trivial uptates to comments. It is way to hard to review this way.

Yeah, fair enough. I split it apart to just include the additions.

InterLinked1 commented 1 year ago

@dgorski Any outstanding issues with this, or can it be merged? We need this for production, and it'd be ideal to pull directly from the repo rather than using patches that might break later.

InterLinked1 commented 1 year ago

@dgorski Just wanted to get your attention on this again, are there any outstanding issues or is this good to go?

dgorski commented 1 year ago

Sorry about the delay.

InterLinked1 commented 1 year ago

Sorry about the delay.

No worries at all, thanks for taking a look. I also just submitted a separate PR for those coding guidelines fixes if that helps.

nicchap commented 1 year ago

I'm really trying to wrap my head around TddWait. Do you actually call TddWait(Text To Play) and it will only return once the tones have played? Do you still need to play an audio source ?

InterLinked1 commented 1 year ago

I'm really trying to wrap my head around TddWait. Do you actually call TddWait(Text To Play) and it will only return once the tones have played? Do you still need to play an audio source ?

No, TddWait takes no arguments. You call it to wait for the TDD buffer to drain before dialplan execution continues. As an example:

same => n,TddTx(This is some text to send) same => n,TddWait()

If you add to the buffer using an AMI action, TddWait will wait for the entire buffer to drain.

This is because TddTx itself is nonblocking. You need to call an application like StreamSilence() or something else that will transmit silent audio, normally, to make it work. You also need to calculate manually, which is tedious and error prone, hence TddWait() will do all of this for you.

nicchap commented 1 year ago

Awesome! I think I get it now. Thanks.