Skip to content

Reading hexadecimal ascii_pdu output in send_message and read_once#247

Open
JavierEgido wants to merge 3 commits intopython-smpplib:masterfrom
JavierEgido:ascii_pdu
Open

Reading hexadecimal ascii_pdu output in send_message and read_once#247
JavierEgido wants to merge 3 commits intopython-smpplib:masterfrom
JavierEgido:ascii_pdu

Conversation

@JavierEgido
Copy link

I had the need to read hexadecimal PDU responses from server.
As discussed in Read raw_pdu #242 I have done some modifications in client.py.
Now send_message and read_once functions return not only decoded PDU but also hexadecimal ASCII PDU.

Examples:
pdu, submit_sm_resp = client.send_message(...)
deliver_sm = client.read_once()
submit_sm_resp 00000019800000040000000000000002354532443736353100

Copy link
Collaborator

@eigenein eigenein Feb 7, 2024

Choose a reason for hiding this comment

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

There is a couple of things that I don't really like about this implementation:

  1. It breaks the public interface, read_pdu() will now return a tuple. It isn't a bad thins per se, but I believe it could be implemented in a non-breaking way (1 or 2). PS. The tests also failed because of interface change
  2. It will always perform the conversion even though vast majority of users will most likely ignore the result. The display this way can't be changed either. It would be more generic to return/forward a byte-string and leave the conversion to user

Could you adjust the implementation perhaps?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your comments. I created some new parallel functions for read_once, read_pdu and send_message to avoid affection on already implemented systems.
pdu, submit_sm_resp = client.send_message_rawpdu
deliver_pdu = client.read_once_rawpdu()

@JavierEgido JavierEgido requested a review from eigenein February 9, 2024 11:10
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.

2 participants