- sipt_has_isup_body() - $sipt(redirection_information.reason) - $sipt(redirection_information.original_reason) - $sipt(redirecting_number.nai) - $sipt(redirecting_number) - $sipt(original_called_number.nai) - $sipt(original_called_number) - $sipt(generic_number.nai) - $sipt(generic_number)
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [x] PR should be backported to stable branches - [x] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description <!-- Describe your changes in detail -->
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2841
-- Commit Summary --
* sipt: added one function and 8 pseudo-variables
-- File Changes --
M src/modules/sipt/doc/sipt_admin.xml (95) M src/modules/sipt/sipt.c (358) M src/modules/sipt/ss7.h (14) M src/modules/sipt/ss7_parser.c (208)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2841.patch https://github.com/kamailio/kamailio/pull/2841.diff
@tsearle - would you like to check this PR for sipt module?
Thanks for the pull request. I did a review and generally things are looking good. One question, you changed the implementation and (it seems also documentation) for the sipt_get_redirection_info(..) function. Was this a bug fix or a refactoring motivated from the added functionality from your PR?
@henningw I did not change sipt_get_redirection_info behavior. It returns the same result. I only fixed documentation to describe actual behavior.
The old documentation says that redirection_info `Returns redirection info header...` But in fact it does not get information from redirection info header ([ISUP_PARM_REDIRECTION_INFO](https://github.com/kamailio/kamailio/blob/master/src/modules/sipt/ss7.h#L121)), but gets from diversion info ([ISUP_PARM_DIVERSION_INFORMATION](https://github.com/kamailio/kamailio/blob/master/src/modules/sipt/ss7.h#L138)) You can check this link to see that another ISUP field was used https://github.com/kamailio/kamailio/blob/master/src/modules/sipt/ss7_parser...
I was referring to line 374/376 of the code:
``` if((body.s[0] != ISUP_ACM) && (body.s[0] != ISUP_CPG) && (body.s[0] != ISUP_IAM)) { LM_DBG("message not an ACM or CPG or IAM\n"); return -1; }
```
@MikhailKalashnikov pushed 1 commit.
2332da7ea522dcdc5637019f1b91135d4c529ce7 rollback sipt_get_redirection_info
@henningw Thank you for this point, It was my mistake, because the old documentation of sipt_get_redirection_info provided incorrect information, I first made it for IAM, and then realized that it returns different field, but did not rollback this change. I fixed it now
@MikhailKalashnikov thanks. Two more things ;-)
- please check also line 904 in the patch, here is another change probably related to this - please document the new added PVs in the wiki as well, there is already a sipt section: [link](https://www.kamailio.org/wiki/cookbooks/devel/pseudovariables)
Thank you.
@henningw - In this line 904. Behavior remains the same, because I also corrected mapping for the old [REDIRECTION_INFO](https://github.com/SIfoxDevTeam/kamailio/commit/4e6cdcfd54fc15dd9db10a3c08cb...) PV to make it clear that it returns ISUP_PARM_DIVERSION_INFORMATION.
- Could you give me (username: mish_gun) a write permission to edit wiki?
@MikhailKalashnikov - i see you refactored it on two places so the behaviour stays the same. Regarding the wiki, do you already tried to [register](https://www.kamailio.org/wiki/cookbooks/devel/pseudovariables?do=register)?
@henningw yes. I've registered with username `mish_gun`. and waiting for write permissions
@MikhailKalashnikov thanks, your user should have now write permissions (before it was open for everybody, but we just got to much spam in the past on the wiki)
@tsearle commented on this pull request.
@@ -102,10 +115,30 @@ static sipt_header_map_t sipt_header_mapping[] =
{{"CHARGE_INDICATOR", 1}, {NULL, 0} }}, - {"REDIRECTION_INFO", ISUP_PARM_REDIRECTION_INFO, - {{NULL, 0}} }, - {"REDIRECTION_NUMBER", ISUP_PARM_REDIRECTION_NUMBER, - {{"NATURE_OF_ADDRESS", 1}, + {"REDIRECTION_INFO", ISUP_PARM_DIVERSION_INFORMATION,
wouldn't it be better to keep "REDIRECTION_INFO" as is and add "DIVERSION_INFO" as an extra parameter?
@henningw commented on this pull request.
@@ -102,10 +115,30 @@ static sipt_header_map_t sipt_header_mapping[] =
{{"CHARGE_INDICATOR", 1}, {NULL, 0} }}, - {"REDIRECTION_INFO", ISUP_PARM_REDIRECTION_INFO, - {{NULL, 0}} }, - {"REDIRECTION_NUMBER", ISUP_PARM_REDIRECTION_NUMBER, - {{"NATURE_OF_ADDRESS", 1}, + {"REDIRECTION_INFO", ISUP_PARM_DIVERSION_INFORMATION,
@tsearle It seems to be just a refactoring, have a look to the line 904 of the sipt.c diff/patch. It still returns the same function.
@MikhailKalashnikov commented on this pull request.
@@ -102,10 +115,30 @@ static sipt_header_map_t sipt_header_mapping[] =
{{"CHARGE_INDICATOR", 1}, {NULL, 0} }}, - {"REDIRECTION_INFO", ISUP_PARM_REDIRECTION_INFO, - {{NULL, 0}} }, - {"REDIRECTION_NUMBER", ISUP_PARM_REDIRECTION_NUMBER, - {{"NATURE_OF_ADDRESS", 1}, + {"REDIRECTION_INFO", ISUP_PARM_DIVERSION_INFORMATION,
This existing "REDIRECTION_INFO" actually gets information from diversion info (ISUP_PARM_DIVERSION_INFORMATION) and not from redirection info header (ISUP_PARM_REDIRECTION_INFO). So this old name is incorrect. It would be better to rename the old "REDIRECTION_INFO" to "DIVERSION_INFO". But it will break existing behavior. If someone uses this old "REDIRECTION_INFO" he will get wrong results. So I did not change existing behavior, only corrected documentation of "REDIRECTION_INFO"
seems ok for me
I updated wiki page with PVs
Thank you, merged
Merged #2841 into master.
@henningw Is it possible to backport this PR to a stable branch (to 5.5)?
Its not possible, sorry, as this is a new functionality which should not be introduced into the stable branch. You could compile a 5.5.x version and apply the patch locally.
-
Daniel-Constantin Mierla -
Henning Westerholt
-
MikhailKalashnikov
-
tsearle