Hello,

thanks for provided updates, I have just checked them.

Here are my remarks:

1. I have found interface updates for setReportingState-73, istCommand - 88 and releaseResources - 20. May I ask which MAP operations are you going to cover ? (I just want to mark them as in implementation)

2. you have added only MAPMessageType.istCommand_Request but we need to have also istCommand_Response diffwerently for resquest and response. Also response's getMessageType() must refer to a proper value

3. Interface definition files like IstCommandRequest have some header info from MAP specification. Their formatting was broken for some reasons and we fix of formatting when we implement a primitive. An example for a proper header you can find in https://github.com/RestComm/jss7/blob/master/map/map-api/src/main/java/org/mobicents/protocols/ss7/map/api/service/callhandling/SendRoutingInformationRequest.java - see a section between and ("" is needed for the format will not broken again in the future). May I ask you to fix it ?

4. "imsi [0] IMSI, extensionContainer [1] ExtensionContainer OPTIONAL" parameters in IstCommandRequest (unlike IstCommandResponse) are context-specific. Also imsi parameter is NOT optional so we need to check at decoding / encoding steps (if it is not provided we need to throw MAPException). See as an example SendRoutingInformationRequestImpl (msisdn parameter)
   IstCommandResponseImpl encoding / descoding is correct

5. we need for each implemented method a unit test for testing on encoding / decoding. You can find examples there:
   https://github.com/RestComm/jss7/blob/master/map/map-impl/src/test/java/org/mobicents/protocols/ss7/map/service/callhandling/SendRoutingInformationRequestTest.java
   https://github.com/RestComm/jss7/blob/master/map/map-impl/src/test/java/org/mobicents/protocols/ss7/map/service/callhandling/SendRoutingInformationResponseTest.java

6. MAPDialogCallHandlingImpl.addIstCommandRequest() - this operation can be used only in MAP version 3. So you need to leave only version 3 in the code block:
   if ((this.appCntx.getApplicationContextName() != MAPApplicationContextName.ServiceTerminationContext)
   || (vers != MAPApplicationContextVersion.version1 && vers != MAPApplicationContextVersion.version2 && vers != MAPApplicationContextVersion.version3))
   throw new MAPException(
   "Bad application context name for addIstCommandRequest: must be ServiceTerminationContext _V1, V2 or V3");

The same this is also in addIstCommandResponse()

7. MAPDialogCallHandlingImpl.addIstCommandResponse() - the parameter is optional. This means that if no extensionContainer is provided by a sender we need to skip a parameter body. See an example for it in https://github.com/RestComm/jss7/blob/master/map/map-impl/src/main/java/org/mobicents/protocols/ss7/map/service/sms/MAPDialogSmsImpl.java - addMoForwardShortMessageResponse()

8. MAPServiceCallHandlingImpl.isServingService() -
   case ServiceTerminationContext: - we need to accept only version 3 here

9. MAPServiceCallHandlingImpl.getMAPv1ApplicationContext() - we need to remove MAPOperationCode.istCommand from here. This section is only for MAP operations that supports MAP version 1 (this one supports only version 3)

10. MAPServiceCallHandlingImpl.istCommandResponse() must take into account that a parameter is optional. See as an example MAPServiceSmsImpl.moForwardShortMessageResponse().

11. for MAPFunctionalTest we have one hint - you can add (temporary for testing locally !!) the folloing code in the begin of your test:
    saveTrafficInFile();
    This leads that MAPFunctionalTest writes a pcap file with the content which you are transferring. You can open this file in WireShark and check if it decodes what you are going to send.

Please fix mentioned by me remarks and then provide a new pull request for ONE operation, I will check and commit and only then we will be able to go forward.

Hello,

I am newbie using GIT, so first of all, sorry for the inconveniences.
I created the PR just after the first commit, and I supposed that this timing should be respected when committing it. I have not asked yet for a new PR of the second commit and I thought that you could commit first PR with its previous commits, and second PR with the remaining commits. As I read you, I understand that is not possible, isn't it? You have to commit all the changes in the fork when committing a PR.

In order to make several PRs and avoid overlapping my PRs with the commits, should be better to create a branch in our fork for each PR?

Thanks in advance.

In order to make several PRs and avoid overlapping my PRs with the commits, should be better to create a branch in our fork for each PR?

Yes please.

Hi,
I have committed all changes requested. I only have a doubt about this point:

4. "imsi [0] IMSI, extensionContainer [1] ExtensionContainer OPTIONAL" parameters in IstCommandRequest (unlike IstCommandResponse) are context-specific.

How do I know that the parameters are context-specific or another class? I could not find this information in the documentation...

Thank you.

How do I know that the parameters are context-specific or another class?
I could not find this information in the documentation...

1. The record
   imsi IMSI
   means that ASN Tag class is CLASS_UNIVERSAL (0) and Tag depends on IMSI element. You can find in spec that "IMSI ::= TBCD-STRING (SIZE (3..8))" and "TBCD-STRING ::= OCTET STRING". So tag in this case is "STRING_OCTET" (4).

2. THE RECORD
   imsi [0] IMSI
   means that ASN Tag class is CLASS_CONTEXT_SPECIFIC (2) and Tag==0 (a value from "[0]")

I have committed your update into both master and netty-2 branches. Thans for your work.

Also I have committed some little mostly tuning update "Minor updates after IstCommand MAP operation introducing". Before you continue please synch with master branch and check it.

I am closing this pull request now.