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

Adding diameter S13 extension to close issue #24 #32

Merged
merged 6 commits into from May 3, 2016

Conversation

Projects
None yet
4 participants
@mnowa
Collaborator

mnowa commented Apr 1, 2016

Support for ME Identity Check Procedure for S13/S13' interface to resolve issue #24

@deruelle

This comment has been minimized.

Member

deruelle commented Apr 2, 2016

@brainslog please review.

@brainslog

This comment has been minimized.

Collaborator

brainslog commented Apr 22, 2016

@mnowa, thanks for the PR.

It looks very good functionality wise, just a few general remarks:

  • Please replace (add where missing) the license header with the correct one:
 /*
  * TeleStax, Open Source Cloud Communications
  * Copyright 2011-2016, TeleStax Inc. and individual contributors
  * by the @authors tag.
  *
  * This program is free software: you can redistribute it and/or modify
  * under the terms of the GNU Affero General Public License as
  * published by the Free Software Foundation; either version 3 of
  * the License, or (at your option) any later version.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU Affero General Public License for more details.
  *
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  */
  • Please use indentation of 2 spaces for java files (it's ok to leave the dictionary using tabs). We will soon move everything to 2-space.
  • Fix a few typos (commented on the exact locations)

Other than this, thanks for the contribution and great work!

throw new InternalException( "Cant receive message in state TIMEDOUT. Command: " + event.getData());
default:
logger.error("S6a Client FSM in wrong state: {}", state);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Should be S13 instead of S6a

try {
return terminalInfoAvp.getGrouped().getAvp(Avp.TGPP_IMEI) != null;
}catch(AvpDataException ex){
logger.debug("Failure trying to obtain (Terminal-Infonation) IMEI AVP value", ex);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Terminal-Information instead of Terminal-Infonation

return imei.getUTF8String();
}
}catch(AvpDataException ex){
logger.debug("Failure trying to obtain (Terminal-Infonation) IMEI AVP value", ex);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Terminal-Information instead of Terminal-Infonation

try {
return terminalInfoAvp.getGrouped().getAvp(Avp.TGPP2_MEID) != null;
}catch(AvpDataException ex){
logger.debug("Failure trying to obtain (Terminal-Infonation) MEID AVP value", ex);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Terminal-Information instead of Terminal-Infonation

return meid.getOctetString();
}
}catch(AvpDataException ex){
logger.debug("Failure trying to obtain (Terminal-Infonation) MEID AVP value", ex);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Terminal-Information instead of Terminal-Infonation

try {
return terminalInfoAvp.getGrouped().getAvp(Avp.SOFTWARE_VERSION) != null;
}catch(AvpDataException ex){
logger.debug("Failure trying to obtain (Terminal-Infonation) Software-Version AVP value", ex);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Terminal-Information instead of Terminal-Infonation

return softwareVersion.getUTF8String();
}
}catch(AvpDataException ex){
logger.debug("Failure trying to obtain (Terminal-Infonation) Software-Version AVP value", ex);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

Terminal-Information instead of Terminal-Infonation

break;
default:
logger.error("Wrong action in s13 Server FSM. State: IDLE, Event Type: {}", eventType);

This comment has been minimized.

@brainslog

brainslog Apr 22, 2016

Collaborator

S13 instead of s13

Added missing ApplicationId setting on new session data created by S1…
…3SessionFactoryImpl, corrected typos, replaced (added where missing) the license header with TeleStax license, replaced tabs with double quotes in S13 classes
@mnowa

This comment has been minimized.

Collaborator

mnowa commented May 2, 2016

@brainslog, I've modified the code according to you comment. Could you please review?

@brainslog brainslog merged commit 8ca3f1b into RestComm:master May 3, 2016

@brainslog

This comment has been minimized.

Collaborator

brainslog commented May 3, 2016

@mnowa it's looking good. Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment