[Asterisk-code-review] res_pjsip_aoc: New module for sending advice-of-charge with chan_pjsip (asterisk[master])

Joshua Colp asteriskteam at digium.com
Sat Oct 29 13:37:21 CDT 2022


Attention is currently required from: Michael Kuron.

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/19460 )

Change subject: res_pjsip_aoc: New module for sending advice-of-charge with chan_pjsip
......................................................................


Patch Set 4:

(3 comments)

Patchset:

PS2: 
> Is there a way to register new endpoint settings without having to modify chan_pjsip? […]
There is not. It has to be modified, and an Alembic script created[1] to cover the realtime aspect. A sample config file is in the tree in contrib/ast-db-manage which can be provided to alembic using the -c option:

alembic -c config.ini.sample revision -m "add aoc option"

As for test coverage, the testsuite is a separate repo[2] and there are tests in tests/channels/pjsip for PJSIP. Finding one that is closest to what you want is the easiest option and then copying it. Instructions for setting up the testsuite and running it are on the wiki[3].

[1] https://alembic.sqlalchemy.org/en/latest/tutorial.html#create-a-migration-script
[2] https://github.com/asterisk/testsuite
[3] https://wiki.asterisk.org/wiki/display/AST/Using+Python3


File channels/chan_pjsip.c:

https://gerrit.asterisk.org/c/asterisk/+/19460/comment/bbf431ad_2a43f0ad 
PS2, Line 1834: 	case AST_CONTROL_AOC:
> I am using a framehook now and it works fine, but I get these warning messages: […]
Can the framehook return an &ast_null_frame instead as the frame has been handled by it?


File res/res_pjsip_aoc.c:

https://gerrit.asterisk.org/c/asterisk/+/19460/comment/6767ff6b_f3ba1a70 
PS2, Line 491: static pj_status_t aoc_outgoing_response(pjsip_tx_data *tdata)
> In my testing, aoc_bye_supplement.outgoing_response never got triggered. […]
There's a limited number of pjsip_modules that can exist at any time, so trying to avoid them is best. I think investigating why in the particular use case it wasn't called would be good.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19460
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Iebb7ad0d5f88526bc6629d3a1f9f11665434d333
Gerrit-Change-Number: 19460
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Kuron <m.kuron at gmx.de>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Michael Kuron <m.kuron at gmx.de>
Gerrit-Comment-Date: Sat, 29 Oct 2022 18:37:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joshua Colp <jcolp at sangoma.com>
Comment-In-Reply-To: Michael Kuron <m.kuron at gmx.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221029/15e68332/attachment.html>


More information about the asterisk-code-review mailing list