Fixed 8n-1 issue, added tests

[kurzacz]

I'm using your library for my work, where I need to send USSD codes. Some of devices require AT+CUSD command to have the USSD code encoded with 7-bit PDU, so this is where I use your project. It looks like this one: AT+CUSD=1,"AA58ACA6B28D1A",15\r In simple words, I use GSM.encode() to encode my USSD code, i put it into AT command above, then I get response encoded with PDU which I decode obviously with GSM.decode()

I had issue with sending command *115*5#, because I wasn't getting any response from GSM modems. It was strange, because all the other commands encoded with your library were all right.

After some search I found out that PDU algorithm requires special treatment for strings 8n-1 chars long (n=1,2,3...). In general, they require to be padded with char before you start encoding. More info you can find in specification, on page 17. In simple words, command *115*5# was encoded wrong because it was (8*1-1=7) chars long.

You can notice issue when you try to decode and encode the same string, like in this examples:

>>> from smspdu.codecs import GSM
>>> GSM.decode(GSM.encode('1234567'))
'1234567@'
>>> GSM.decode(GSM.encode('0123456789ABCDE'))
'0123456789ABCDE@'

All the tests strings have this thing in common: if you add 1 to their actual length, you'll get number dividable by 8.

You can compare results from your library with SMSTools3 PDU Converter. You can put your string to encode into Text field, then you will see encoded string in USSD Entry/Display field. Then you can convert it back with the appropriate Convert> button. When you press it, you'll see decoded string with annotation (Had padding which is removed). Results from function GSM.encode are almost every time equal to these generated at online converter, with except of these specific-length ones. For example:

>>> GSM.encode('*115*5#')
'AA58ACA6AA8D00'
             ||
-AA58ACA6B28D1A- <-- this one was encoded at SMSTools3 PDU Converter

When I put latter encoded USSD code, my GSM modem process the command correctly. I'd like to say that I didn't create pull request only because of my specific hardware. I suppose that there can be more devices which works with the specification I linked above. For this reason one day somebody could open an issue with the same problem that I had. I hope my solution would help those people.

[alexpirine]

Hi @kurzacz ,

Thank you for spotting this issue!

I believe we can safely use \r as padding character instead of @ when encoding 2n-1 characters.

However, I'm not so sure about the next steps for a fix.

First issue

How do we handle the case when you really need to send a <CR> as 8th character? The specification suggests to append another <CR>. So we should also strip a <CR> in this case when decoding? What happens when multiple <CR> are encoded/decoded at the end of the line? Should we have n+1 <CR> or 2n?

Second issue (related to the first one)

Also, I wonder if the implementation for SMS shouldn't be different: according to the GSM 03.40 specification, the content length is specified in the TP-UDL field (page 52), this is why this padding issue is not happening when sending and receiving SMS messages.

The excess padding character is simply stripped based on content length.

Do you believe you can make changes to take into account these two issues? What do you suggest?

[alexpirine]

Also, I believe the “2n-1 length test” should be on the chars variable, which is different from the length of data, in case of extended alphabet usage.

[kurzacz]

Hello, Well, after some time spent on your questions, I suppose that maybe my solution fix just the small issue with USSD codes, but isn't applicable for the whole your library.

[alexpirine]

Hi @kurzacz!

Been a long time… Do you think there is something to be done or should we close this issue?

@comic31 could you please take a look at this? Maybe you can think of a solution that works both for the whole library and for the specific needs mentioned by @kurzacz.

[alexpirine]

@comic31 up

[alexpirine]

@kurzacz @Asmox this issue has been fixed now.

In order to take padding into account, you can now use GSM.encode('…', with_padding=True) and GSM.decode('…', strip_padding=True).

Padding is still not used by default for backwards-compatibility and for use cases with full SMS-DELIVER / SMS-SUBMIT PDUs (because the message length is specified, so there is no need for a padding).

Hope that helps!

[kurzacz]

Thank you very much :-) I don't develop the app I used your lib with anymore. But if only I'm asked by my last employer to patch it, I'll try your updated version for sure.