Navigation Menu

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

SmppNLSTSplitter for sending messages using National Language Shift Table #1323

Closed
wants to merge 2 commits into from

Conversation

roadrunner
Copy link
Contributor

@roadrunner roadrunner commented Nov 30, 2016

to support sending messages with National Language Shift Tables, there are two thing needs to be taken care of:
even if the short message is only one part, we need to set UDHI; we are using esm_class.
splitting messages for locking tables:
155 characters allowed when submitting single short message
149 characters allowed when submitting multipart short messages

i've added test cases for splitting

@roadrunner
Copy link
Contributor Author

i wonder if you can apply this patch 2.18.x branch as well?

@oscerd
Copy link
Contributor

oscerd commented Nov 30, 2016

Can you please Change the commit message? I think there is a typo :-)

@roadrunner roadrunner changed the title SmppNLSTSplitter for sending messages using National Language Shit Table SmppNLSTSplitter for sending messages using National Language Shift Table Nov 30, 2016
@roadrunner
Copy link
Contributor Author

Sorry, it was a typo :)

@oscerd
Copy link
Contributor

oscerd commented Nov 30, 2016

Me or @davsclaus will review soon :-) thanks for your contribution!

@oscerd
Copy link
Contributor

oscerd commented Dec 1, 2016

@davsclaus this looks good to me. What do you think?

@roadrunner
Copy link
Contributor Author

i'm using this splitter in production without any issue. i can confirm that it's working for Turkish NLST is working.
but i've to compile and install camel locally. i thought if i publish it others might contribute..

import org.slf4j.LoggerFactory;

/**
* Created by engin on 30/11/2016.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this kind of javadoc comments

@@ -0,0 +1,100 @@
package org.apache.camel.component.smpp;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ASF license header



protected static final int UDHIE_SAR_REF_NUM_LENGTH = 1;
// protected static final byte UDHIE_IDENTIFIER_SAR = 0x00;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented out code, or explain why its commented out

} else {
lengthOfData = segmentLength;
}
logger.info("Length of data = {}", lengthOfData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log at DEBUG level

logger.info("Length of data = {}", lengthOfData);

segments[i] = new byte[UDHIE_NLI_MULTI_MSG_HEADER_REAL_LENGTH + lengthOfData];
logger.info("segments[{}].length = {}", i, segments[i].length);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

if (null != splitter){
return splitter;
}
logger.warn("Invalid splitter given. Must be instance of SmppSplitter");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not throw an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 better to throw exception. thanks for the tip

@@ -0,0 +1,63 @@
package org.apache.camel.component.smpp;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASF license header

import static org.junit.Assert.assertEquals;

/**
* Created by engin on 30/11/2016.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

@davsclaus
Copy link
Contributor

And there is likely also some checkstyle issues to fix

@oscerd
Copy link
Contributor

oscerd commented Dec 1, 2016

My comment was only about the feature. I didn't review the code :-)

@roadrunner
Copy link
Contributor Author

okay, i'm dealing with the mess..

added ASF license headers
invalid splitter throws exception
@davsclaus
Copy link
Contributor

Looks better. @oscerd can you take a look as well and merge if okay.

@oscerd
Copy link
Contributor

oscerd commented Dec 2, 2016

Sure :-)

@oscerd
Copy link
Contributor

oscerd commented Dec 2, 2016

Thanks the PR has been merged. Can you close this?

@roadrunner
Copy link
Contributor Author

Great, thank you guys..

@roadrunner roadrunner closed this Dec 2, 2016
@oscerd
Copy link
Contributor

oscerd commented Dec 2, 2016

Thanks a lot for the contrib @roadrunner :-)

@roadrunner
Copy link
Contributor Author

i hope, i'll be doing more soon 👍

zregvart pushed a commit to zregvart/camel that referenced this pull request Jan 13, 2021
[ENTESB-11189]make  camel-elasticsearch-rest feature self-contained
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants