Skip to content

Function to parse short_message from deliver_sm#90

Open
Lynesth wants to merge 2 commits into
python-smpplib:masterfrom
Lynesth:patch-4
Open

Function to parse short_message from deliver_sm#90
Lynesth wants to merge 2 commits into
python-smpplib:masterfrom
Lynesth:patch-4

Conversation

@Lynesth

@Lynesth Lynesth commented Jul 31, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Not sure if needed ot if correctly implemented but I've added a function to parse the short_message property of pdus from deliver_sm commands since not all vendors fill in optional parameters like receipted_message_id.

Example:

# Inside the message_received_handler
pdu.parse_deliver_receipt()

This returns a dict like this one:

{
    'dlvrd': b'000',
    'done_date': b'1907311208',
    'err': b'001',
    'id': b'6C7C7DE28BB3E961947700000000FE37',
    'stat': b'UNDELIV',
    'sub': b'001',
    'submit_date': b'1907311208',
    'text': b'This is a test.'
}

Since it depends on a regex and all vendor surely don't implement the receipt the same way (according to the specs: The format for this Delivery Receipt message is SMSC vendor specific but following is a typical example of Delivery Receipt report.) I've added a way to set the regex manually:

smpp.pdu.set_delivey_receipt_regex(br'myawesomeregex')

Note that the regex must be bytes.

@Lynesth

Lynesth commented Jul 31, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Oh this fails on 2.7 because of the rb in front of the regex. Not sure how to fix this right now, will look at it later.

@eigenein

eigenein commented Jul 31, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Try br instead:

Python 2.7.10 (default, Feb 22 2019, 21:55:15) 
[GCC 4.2.1 Compatible Apple LLVM 10.0.1 (clang-1001.0.37.14)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> br'\n'
'\\n'

@Lynesth

Lynesth commented Jul 31, 2019

Copy link
Copy Markdown
Contributor Author

@eigenein Thanks that did the trick.

@Lynesth

Lynesth commented Aug 5, 2019

Copy link
Copy Markdown
Contributor Author

@podshumok @eigenein Could I get some review on this when you have the time ?
I am using this in my implementation at the moment because my SMSC doesn't fill optional parameters in deliver_sm so I think it could be of use to other people as well ?

@eigenein

eigenein commented Aug 6, 2019

Copy link
Copy Markdown
Collaborator

I'm a bit 50/50, I mean, it looks good and helpful, but I'd make a utility function outside of PDU class. My motivation is that this is rather not a PDU feature but a kind of "helper" to parse a vendor-specific status short_message.

@podshumok Did you have a chance to look at this?

@Lynesth

Lynesth commented Aug 6, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@eigenein Even though it is vendor specific it seems to be pretty widely used as is and most of the providers use that format.
If I understood correctly you would prefer if it was out of the lib ?

code-of-kpp
code-of-kpp requested changes Aug 6, 2019
View reviewed changes

@code-of-kpp code-of-kpp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think that it should live in PDU class, especially the part with changing class variable in statimethod.

It is OK as external function. It would be nice to provide simple integration with Client.set_message_received_handler

Comment thread smpplib/pdu.py
command = None
status = None
_sequence = None
dlr_regex = None

@code-of-kpp code-of-kpp Aug 6, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please, avoid abbreviations

Comment thread smpplib/pdu.py

return desc

@staticmethod

@code-of-kpp code-of-kpp Aug 6, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What If I work with two SMSCs with different deliver_sm message formats?

Comment thread smpplib/pdu.py
raise ValueError("Invalid PDU command: %s", self.command)

if PDU.dlr_regex is None:
PDU.dlr_regex = re.compile(br'^id:(?P<id>\S+)\s+sub:(?P<sub>\S+)\s+dlvrd:(?P<dlvrd>\S+)\s+submit date:(?P<submit_date>\S+)\s+done date:(?P<done_date>\S+)\s+stat:(?P<stat>\S+)\s+err:(?P<err>\S+)\s+Text:(?P<text>.*)$', re.IGNORECASE)

@code-of-kpp code-of-kpp Aug 6, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please keep lines at 79 symbols (PEP8). You can split

(
    br'your'
    br' string'  # with some comments
    ' like' br'this'
)

for example

@code-of-kpp code-of-kpp Aug 6, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this default pattern can live in consts

@Lynesth

Lynesth commented Aug 6, 2019

Copy link
Copy Markdown
Contributor Author

Feel free to close it if you want. I won't have time to modify it anytime soon and wouldn't know where to put it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@code-of-kpp code-of-kpp code-of-kpp requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Lynesth @eigenein @code-of-kpp