Include ARQ scheme #10 #37
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
an Acknowledgement mode flag.
- include packet inspection for UEs - copy messages after adding them to the sender window. Otherwise messages get lost in window - handle case in which there is data to send, but no fragment can be generated since the sender window is full
ethertype byte order was wrong more log outputs to identify which ARQ scheme is currently used
fragmenter_has_fragment returned true in some cases were no fragment could be sent anymore reassembler had a memory issue when reassembling a frame
we use 4 bits instead of 3 for seqNr
BS will automatically assign UL slots when it transmits a frame that has to be acked
dl1com
reviewed
Oct 1, 2020
There was a problem hiding this comment.
I guess it would be nice to have some more verbose documentation on a per-function level, especially the input and output parameters. I'd propose to use doxygen format or something similar.
No need to do everything at once, but all functions which where touched by this PR may be a good start :)
dl1com
reviewed
Oct 1, 2020
dl1com
reviewed
Oct 1, 2020
src/mac/mac_messages.c
Outdated
Comment on lines
411
to
414
| msg->hdr.DLdataAck.ctrl_id = msg->type & 0b111; | ||
| msg->hdr.DLdataAck.ack_type = (msg->hdr_bin[0] >> 3) & 0b11; | ||
| msg->hdr.DLdataAck.seqNr = msg->hdr_bin[0] & 0b111; | ||
| msg->hdr.DLdataAck.ack_type = (msg->hdr_bin[0] >> 4) & 0b1; | ||
| msg->hdr.DLdataAck.seqNr = msg->hdr_bin[0] & 0b1111; | ||
| msg->hdr.DLdataAck.fragNr = (msg->hdr_bin[1] >> 3) & 0b11111; |
There was a problem hiding this comment.
Is it possible to add a unit test for this?
dl1com
reviewed
Oct 1, 2020
dl1com
reviewed
Oct 1, 2020
dl1com
reviewed
Oct 1, 2020
- include a maximum number of retransmits for the ARQ scheme - check whether a fragment is available inside get_fragment - Some doxygen documentation - Verbose assignment of MacMessage types enum
dl1com
reviewed
Oct 4, 2020
There was a problem hiding this comment.
Nice work!
Just two more comments:
- Could you please add an unit test for
packet_inspect_is_tcpip? - Function
mac_frag_get_fragmentis of pretty high complexity (with over 100 lines) with a high level of indentations - I've opened an issue for refactoring: Refactoring of mac_frag_get_fragment #42
dl1com
approved these changes
Oct 11, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enhances the system with an additional MAC transmission mode.
Currently all MAC frames are sent in Unacknowledged mode, this PR allows to use a selective repeat ARQ in parallel to the existing Unacknowledged mode.
Fixes #10