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

Add support for 8bit encoding #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pchethan
Copy link

@pchethan pchethan commented Jul 4, 2019

No description provided.

@pchethan
Copy link
Author

pchethan commented Jul 4, 2019

@megadix Could you review addition of 8-bit encoding support?

@megadix
Copy link
Contributor

megadix commented Jul 5, 2019

First of all, thank you for your trust in my competencies :) It's been a long time since I don't touch SMS-related stuff, so please forgive me if I'm missing something important...

I've quickly read through the docs as stated in the comment (i.e. SMPP 3.4, GSM 03.38). I see that using a 1111xxxx value is used to send GSM Class Control messages, but I don't understand why enforcing ISO 8859-1. Shouldn't it be encoded in the lower bits? Am I missing something?

Dimitri

@pchethan
Copy link
Author

pchethan commented Jul 5, 2019

You are the author of DataCodingCharsetHandler, you know best :)

GSM 03.38 Section 4 details 1111xxxx and says that 1111x1xx represents 8-bit data. ISO 8859-1 is the encoding that can represent 8-bit binary data, hence the mapping. Did that answer your question?

Screenshot 2019-07-05 at 10 01 01 PM

@paoloc0
Copy link
Contributor

paoloc0 commented Jul 12, 2019

ISO 8859-1 is the encoding that can represent 8-bit binary data, hence the mapping.

@pchethan 8-bit binary data has no inherent character set. It's just arbitrary binary data. Nonetheless, you no doubt have a use case where doing this makes your code work, for the right or wrong reasons. I'm sure it will help @megadix if you also provide an example of the code that uses this new code you're introducing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants