Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GMLC responseData DialogTimeout #25

Open
loayking opened this issue May 19, 2016 · 12 comments
Open

GMLC responseData DialogTimeout #25

loayking opened this issue May 19, 2016 · 12 comments
Assignees

Comments

@loayking
Copy link
Collaborator

When deploying GMLC with Global Title 0100 with BCDEvenEncodingScheme in the Sccp Address, the last digit of the GT will be truncated, and this will lead to DialogTimeout because the initiated GT has been modified
The solution is configuring Global Title 0100 with BCDOddEncodingScheme in the Sccp Address

@deruelle
Copy link
Member

@loayking can you make a pull request with your change as described in our Open Source Playbook ?

@deruelle deruelle added the bug label May 19, 2016
@deruelle deruelle added this to the 1.0.0 milestone May 19, 2016
@vetss
Copy link
Contributor

vetss commented May 20, 2016

Hello @loayking,

thanks, good catch.

But your soulution (https://github.com/RestComm/gmlc/pull/26/files) in also wrong. You can find the proper solution for example in https://github.com/RestComm/smscgateway/blob/master/core/smsc-common-library/src/main/java/org/mobicents/smsc/library/MessageUtil.java see getSccpAddress() method. We have to reuse org.mobicents.protocols.ss7.sccp.parameter.ParameterFactory. This factory selects proper BCDEven/OddEncodingScheme.

@loayking
Copy link
Collaborator Author

Hi Sergery,
Yes you are right , I use it direct to test the concept , but yes we have to reuse org.mobicents.protocols.ss7.sccp.parameter.ParameterFactory to selects the proper BCDEven/OddEncodingScheme.

@deruelle
Copy link
Member

hi @loayking, were you able to make progress ?

@loayking
Copy link
Collaborator Author

hi @deruelle still working on it, need little more time to follow vetss way

@vetss
Copy link
Contributor

vetss commented Jun 1, 2016

In the method "SccpAddress convertAddressFieldToSCCPAddress(String address)"
we need to create SccpAddress not buy code:

GlobalTitle0100 gt = new GlobalTitle0100Impl(address, 0, BCDEvenEncodingScheme.INSTANCE,NumberingPlan.ISDN_TELEPHONY, NatureOfAddress.INTERNATIONAL);
return new SccpAddressImpl(RoutingIndicator.ROUTING_BASED_ON_GLOBAL_TITLE, gt, 0,gmlcPropertiesManagement.getHlrSsn());

but by code like:
// 1) this part must be invoked onle in "setSbbContext()" method
ParameterFactory sccpParameterFact = new ParameterFactory();
// 2) this is code for an Sccp address creating:
String address = "....";
NumberingPlan np = NumberingPlan.ISDN_TELEPHONY;
NatureOfAddress na = NatureOfAddress.INTERNATIONAL;
int ssn = gmlcPropertiesManagement.getHlrSsn();
GlobalTitle gt = sccpParameterFact.createGlobalTitle(address, translationType, np, null, na);
return sccpParameterFact.createSccpAddress(RoutingIndicator.ROUTING_BASED_ON_GLOBAL_TITLE, gt, 0, ssn);

vetss added a commit to vetss/gmlc that referenced this issue Jun 3, 2016
@vetss
Copy link
Contributor

vetss commented Jun 3, 2016

Hello @loayking

your update from #27 updates too much code and we can not accept as it is.

I have committed more simple commit:
177b575

Please check if your issue was fixed.

Thanks for bug reporting and provided code.

@vetss vetss mentioned this issue Jun 3, 2016
@loayking
Copy link
Collaborator Author

loayking commented Jun 3, 2016

Hi @vetss
Thanks for your kind feedback,
Your update is just like mine, but what I did is take more possibilities to cover all scenarios regarding GlobalTitleIndicator.
I also considered to update GMLC properties file with GlobalTitleIndicator status if this is approved.
In your commit, if the routing is not base on GT the request will not be sent, but if we make it in configuration, it will be more easy.

BR

@vetss
Copy link
Contributor

vetss commented Jun 6, 2016

Hello @loayking

Firtsly please pay attention that when you are creating your pull request you need to download the last code version (from a master branch) and make your changes as a minimum code change that is needed. Your last pull request contains many format updates and amy be some other changes that are not needed for fixing of this concrete issue. Such "big" update is not possible to be committed because we have a risk to break some logic.

If you need a deeper update (with support of different GTI) fill free to make it as compared with current code (with my last updates). Changes of GMLC properties (GmlcPropertiesManagement) is also possible. This leads also updates fo GmlcPropertiesManagementMBean and updates of CLI and manual part (that should be also provided).

@deruelle
Copy link
Member

@vetss shall we add a test case for you commit ?

@vetss
Copy link
Contributor

vetss commented Jul 24, 2016

Hi @deruelle,

such unit test is not a trivial task. I'd like to skip this update unit test if as I understand a fix fit to @loayking demands (please confirm if this, @loayking).

@loayking
Copy link
Collaborator Author

Hi @vetss

actually i did it my way, but yours should do the same.

@FerUy FerUy added enhancement and removed bug labels Aug 3, 2016
@FerUy FerUy modified the milestones: 2.0.0, 1.0.0 Aug 3, 2016
@FerUy FerUy modified the milestones: 3.0.0, 2.0.0.FINAL-Sprint1 Apr 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants