-
Notifications
You must be signed in to change notification settings - Fork 118
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
COctetString update #33
base: master
Are you sure you want to change the base?
Conversation
Binary stores the string plus the NULL character.
Fixed a typo
Adding StringIndexOutOfBoundsException in Vendor_specific_msc_addr. Happens when address length is 0 or 1.
super(tag, new byte[value.getBytes(charsetName).length + 1]); | ||
System.arraycopy(value.getBytes(charsetName), 0, this.value, 0, | ||
value.getBytes(charsetName).length); | ||
this.value[value.getBytes().length] = (byte) 0x00; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not passing the charsetName argument to getBytes() on that line
You call value.getBytes(charsetName) many times, can you revise the code to call it just once?
Hi Pim, Thanks for sharing this. I've pulled you branch here: https://github.com/opentelecoms-org/jsmpp/commits/pmoerenhout-master to see it build in travis Could you please review against the latest master and see my comment about repeated calls to getBytes(charsetName) and submit the pull request against https://github.com/opentelecoms-org/jsmpp ? Regards, Daniel |
Ha Daniel, I pull a request against dpocock/jsmpp, because the https://github.com/opentelecoms-org/jsmpp https://github.com/opentelecoms-org/jsmpp was not forked from the uudahr repo. Regards, Pim
|
Hi Pim, The opentelecoms-org/jsmpp repository does have the same history as the uudashr/jsmpp - Github just doesn't show it is a fork. In your own repository, you should be able to update your reference to it using Do you use jsmpp as part of Apache Camel or with another application? I've also been updating the camel-smpp component. Regards, Daniel |
Hi Daniel, Yes, I just used it to use SSH to point to my own repository, but indeed pointing to the opentelecoms-org should also do the job. I forked now from the opentelecoms, making it a little bit easier. I use it in my own Java Spring environment to send SMPP messages to a Jinny SMSC. This works, with some quirks from the Jinny SMSC (charset in delivery is mixed GSM and Latin1). In the past I used to connect to CMG (Acision) SMSC. In what environment you are using it ? I saw the DateFormatter test failed. I will look into that... Regards, Pim
|
Hi Pim, I didn't have time to check the DateFormatter issue more thoroughly but I opened a bug report here with my initial observations: opentelecoms-org/jsmpp#1 - please look at that before doing anything with it. I use jSMPP with Apache Camel's camel-smpp component. Camel makes it very easy to integrate with message queues and other technologies. Regards, Daniel |
…rown. Added tests.
This version implements correctly the COctetString. Also extended the test to test for the NULL character in the binary value.