Closed DJ2LS closed 7 months ago
Could we call this something similar to a received ARQ transfer data handler or FreeDATA handler? Data handler is maybe too generic? Instead of injecting this DataDispatcher inside an ARQSession, I suggest we exploit the result of calling the on_frame_received() of the ARQSession. This could return the transfered data to the ARQ Session frame handler and keep this "FreeDATA" logic outside of an ARQ transfer/session.
Could we call this something similar to a received ARQ transfer data handler or FreeDATA handler? Data handler is maybe too generic? Instead of injecting this DataDispatcher inside an ARQSession, I suggest we exploit the result of calling the on_frame_received() of the ARQSession. This could return the transfered data to the ARQ Session frame handler and keep this "FreeDATA" logic outside of an ARQ transfer/session.
@ptsmonteiro I moved the dispatcher as suggested to the on_frame_received part. I also adjusted and splitted the dispatcher naming. Please let me know if its more clear now or if it needs further adjustments
The hooking on the on_frame_received() looks good to me 👍
The decapsulate() part makes me think you were right when you wanted to have more metadata stating what kind of payload there is in the ARQ session. If you still want to go this way maybe we can add 1 byte so we can have 256 types of payloads on an ARQ session. This could be added on the session INFO frame for example. Examples of types would be: 0 - raw data 1 - gzipped compressed raw data 2 - bz2 compressed raw data 3 - bz2 compressed freedata json 4 - reserved for the future..
If we go this way I'll drop compression handling on the MessageP2P side.
What do you think?
Yes, that's what I have been initially thinking. So it's good we are in sync now. I think it's worth going this way. So I'll drop the json based approach and moving on to the 1byte info field. This brings us more flexibility and efficiency to ARQ without causing too much complexity.
@ptsmonteiro I removed the data formatter. From my side this can be reviewed again.
Work on further features like a data dispatcher should be done in a separate PR for keeping work packages small and per use case.
I think we can also leave the p2p related stuff inside this module as it's making data dispatching later easier and avoids additional header data for data types not message related. Sure, it's some kind of mixing layers, but I think this could simplify things a bit. If we don't like it, we can change it later the next weeks while gathering experience with this design approach
We should do something about the failing checks. Other than that, looks good to me 👍 :shipit:
A class which adds a "type" information to the info frame when opening an ARQ session. It allows, dispatching data by the type or using different compression algorithms.
Transmit Example:
Receive:
--> Dispatcher will be called after a successfull ARQ transmission.
dispatched_data = self.arq_data_type_handler.dispatch(type_byte, formatted_data)
corresponding endpoints can be added in arq_data_type_handler.py