-
Notifications
You must be signed in to change notification settings - Fork 3
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 binary payload handling to PyLECO #82
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
+ Coverage 89.04% 89.52% +0.48%
==========================================
Files 36 36
Lines 2902 2931 +29
Branches 355 361 +6
==========================================
+ Hits 2584 2624 +40
+ Misses 267 256 -11
Partials 51 51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@seb5g, in this PR I experiment with the transport of binary data via PyLECO. |
Idea for:
Just add a parameter, whether to return a binary value. |
@untzag, @ksunden, @VigneshVSV this could be also of interest for your own projects. |
I ask for feedback regarding the current implementation: This is how you call the method # on the remote side (on MessageHandler):
def some_funny_method(self):
additional_payload = self.current_message.payload[1:]
# do something with additional_payload
self.additional_response_payload = [b"whatever", b"to return", b"binary"]
return None
# on the requester side:
ask("receiver", method="some_funny_method", additional_payload=[b"123", b"456], extract_additional_payload=True)
# returns: [b"whatever", b"to return", b"binary"] The parameter |
I think its a tricky question in general as seen by some questions you raised in the issue. Currently I allow either returning either of non-serialized data or serialized data only. Not both. Technically a user could do def my_server_side_method_returning_bytes(self):
return memoryview(self.my_large_numpy_array)
def my_server_side_method_normal(self):
return 5, 'my name', self.foo_list i.e. for the user, the usage of methods is the same and there is no need to access an additional request or response object. Although I dont see what is wrong with that either. Some HTTP servers which work based on decorating functions, like flask, allow to access the request object through some global variable. Same can be then done for response. Nevertheless, since I allow only one type (either of bytes, bytearray or memoryview) or of a other python types which needs to be serialized, its a little easier for me to place them in different indices of the multipart message to know what was the reply. In this way, the client just checks which part of the multipart message containing return values has size greater than 0. The user given pre serialized part or the RPC server serialized part? Of course, the ideal and best case is to have both so that the user can return any damn return value. This is the right approach. May be its best to reserve a tuple of size 2 as a fixed internal return value scheme. Just before serializing, the type can be checked to be tuple and if its of length 2, then one can proceed to inspect the types each element of the element. If one turns out to be byte type, then dont serialize it again as that will lead to serilization error: def my_mixed_returning_server_method(self):
return { 1: self.some_foo_list, 3 : 42 }, memoryview(self.my_large_numpy_array) Within the RPC server: ret = func(*args, **kwargs)
if isinstance(ret, tuple) and len(ret) == 2: # return value containing one normal part and one byte part
if isinstance(ret[1], (bytes, bytearray, memoryview)): # check if its really a byte part
self.serializer.dumps(ret[0]) # ret[1] is already serialitzed
else:
self.serializer.dumps(ret) # serialize normal data given in tuple of length 2
# as the return value was indeed a normal tuple
elif not isinstance(ret, (bytes, bytearray, memoryview)): # covers the case of prevent serialization of single
# return value which is of bytes type
self.serializer.dumps(ret) # some other data including tuples of lower or higher length Of course the dumps return value must be assign to a place within the multipart message. This is the way I would do it. |
Thanks @VigneshVSV for your extensive answer. |
I have an idea (inspired by you): If the json frame is empty, the other frames are the return value. |
Newest design:
|
a8c0b08
to
d2d5ad8
Compare
Regarding the |
@VigneshVSV , thanks for your comments, they are helpful. |
fa95894
to
938e3b0
Compare
938e3b0
to
6ba7917
Compare
A small change:
Currently the you have to tell the
|
I'm not using really pyleco at the moment as you are the one who implemented it in pymodaq... so I still have difficulties to really understand how all this works. But for sure, everything within pymodaq can be binary serialized so direct emission of such data (frame as you call it?) would be better. I'm not sure how i could really help in the design... |
Thanks for your comment @seb5g. I appreciate it. |
f1967fe
to
bbd1c53
Compare
Summary:
|
Implements the transport of binary data in additional frames, related to How to transmit binary data (pymeasure/leco-protocol#65).
Idea:
Implementation ideas.
MessageHandler
offers the whole message for the rpc handler and adds the additional frames afterwardsOpen questions:
A return value of
None
with additional payload frames is a strong indicator, but not exclusive to this scenario. Is it sufficient?additional_payload
of the Message: what to do, if there is no data defined: should there be an empty first data frame before the additional, or is the additional_payload the whole payload?