Dear Saurabh,
it would be nice if you could address the review comment of Harald for the padding patch and send it again. In terms of the commit message it would be interesting to know how you found it, if something broke because of it (e.g. if a specific Handset+Chipset+Firmware release broke). Such information is nicely captured in a commit message and might help future developers to prevent doing a revert or similar.
In terms of the EDGE support it is custom in Free Software projects to build on top of what is already present. I don't have a specific approach and there are multiple options but from what I see the situation is:
* You seem to have added additional encoding regression tests. It would be nice to move them to the master version and see if the result is similar/correct as well.
* Jacob has not implemented compressed bitmaps in the DL TBF handling. The reason is because we want to avoid bufferbloat our windows don't really get that big and it seems we didn't need it yet. Now that you have built a tree based encoder, it might make sense to include the encoder, add tests for the encoding, reason/explain the performance and then hook the encoding in the DL ACK handling.
* Any other corrections/fixes you have.
* Identify other areas where your support is behaving better than the code in master or features missing and implemented in your branch.
Do you already have an idea yourself how you intend to move forward and get your improvements included in the main repository?
kind regards holger
Hi,
On 02.03.2016 20:14, Holger Freyther wrote:
- Jacob has not implemented compressed bitmaps in the DL TBF handling.
More precisely, compressed bitmaps (decoding) are supported for DL TBFs only, since it is required to decode them DL Ack/Nack if the window size is increased which makes sense with multiple PDCH.
The reason is because we want to avoid bufferbloat our windows don't really get
that big and
it seems we didn't need it yet.
The reason for not implementing them for UL TBF ACK yet was the fact that we only support single slot only in that direction and that the imposed restriction won't probably hurt in most cases. We can still send URBB even if the window size were larger than 96, so there are only some cases with long runs of NACKs and very few ACKs which might suffer from a slightly increased latency.
Now that you have built a tree based encoder, it might make sense to include the encoder, add tests for the encoding,
reason/explain
the performance and then hook the encoding in the DL ACK handling.
-> UL TBF ACK handling
Jacob
On 03 Mar 2016, at 14:40, Jacob Erlbeck jacob01@gmx.net wrote:
Hi,
On 02.03.2016 20:14, Holger Freyther wrote:
- Jacob has not implemented compressed bitmaps in the DL TBF handling.
More precisely, compressed bitmaps (decoding) are supported for DL TBFs only, since it is required to decode them DL Ack/Nack if the window size is increased which makes sense with multiple PDCH.
The reason is because we want to avoid bufferbloat our windows don't really get
that big and
it seems we didn't need it yet.
The reason for not implementing them for UL TBF ACK yet was the fact that we only support single slot only in that direction and that the imposed restriction won't probably hurt in most cases. We can still send URBB even if the window size were larger than 96, so there are only some cases with long runs of NACKs and very few ACKs which might suffer from a slightly increased latency.
Now that you have built a tree based encoder, it might make sense to include the encoder, add tests for the encoding,
reason/explain
the performance and then hook the encoding in the DL ACK handling.
-> UL TBF ACK handling
thanks for the correction.
Hello All, Based on your inputs and our analysis of the code bases( current master versus Radisys EGPRS) we propose following elements, each to be submitted in incremental patches for merge to master branch 1. Compression algorithm based on code tree 2. EPDAN decoding based on the code tree 3. PUAN encoding based on code tree 4. MCS 5-9 support in UL 5. Split block handling in UL 6. Split block generation in DL 7. Retransmission with Incremental redundancy 8. Relevant test suite updates as part of each of the item above
Please let us know your feedback/suggestions for the same.
Regards Saurabh
-----Original Message----- From: Holger Freyther [mailto:holger@freyther.de] Sent: Friday, March 04, 2016 10:35 PM To: Jacob Erlbeck jacob01@gmx.net Cc: osmocom-net-gprs@lists.osmocom.org Subject: Re: Padding patch and moving forward with EGPRS support/merge
On 03 Mar 2016, at 14:40, Jacob Erlbeck <jacob01@gmx.netmailto:jacob01@gmx.net> wrote:
Hi,
On 02.03.2016 20:14, Holger Freyther wrote:
- Jacob has not implemented compressed bitmaps in the DL TBF handling.
More precisely, compressed bitmaps (decoding) are supported for DL TBFs only, since it is required to decode them DL Ack/Nack if the window size is increased which makes sense with multiple PDCH.
The reason is because we want to avoid bufferbloat our windows don't really get
that big and
it seems we didn't need it yet.
The reason for not implementing them for UL TBF ACK yet was the fact that we only support single slot only in that direction and that the imposed restriction won't probably hurt in most cases. We can still send URBB even if the window size were larger than 96, so there are only some cases with long runs of NACKs and very few ACKs which might suffer from a slightly increased latency.
Now that you have built a tree based encoder, it might make sense to include the encoder, add tests for the encoding,
reason/explain
the performance and then hook the encoding in the DL ACK handling.
-> UL TBF ACK handling
thanks for the correction.
On 11 Mar 2016, at 14:34, Saurabh Sharan Saurabh.Sharan@radisys.com wrote:
Hello All,
Dear Saurabh,
Based on your inputs and our analysis of the code bases( current master versus Radisys EGPRS) we propose following elements, each to be submitted in incremental patches for merge to master branch • Compression algorithm based on code tree • EPDAN decoding based on the code tree • PUAN encoding based on code tree • MCS 5-9 support in UL • Split block handling in UL • Split block generation in DL • Retransmission with Incremental redundancy • Relevant test suite updates as part of each of the item above
Please let us know your feedback/suggestions for the same.
looks good in general. Please follow the general guideline, try to make small commits, update test results, add new tests. After each of the elements we should not only have a new feature, but better structure, better test coverage and throughput.
kind regards holger
Hello,
Following patches are identified for the activity “compression algorithm based on code tree”.
• Structure definition for Tree node, declaration of zero run length code list and one run length code list • Building a tree of code words to save run length during initialization and addition of Test cases in Unit test framework • Utility to Search the run length according to the received code word from the tree, when CRBB_STARTING_COLOR_CODE is One during decoding of EPDAN and addition of test case in Unit test framework • Utility to Search the run length according to the received code word from the tree, when CRBB_STARTING_COLOR_CODE is Zero during decoding of EPDAN and addition of test case in Unit test framework • Utility to find runlength of one’s or zero’s and addition of Test cases in Unit test framework • Utility to find code word of runlength during encoding of PUAN and addition of Test cases in Unit test framework
Regards, Sangamesh
-----Original Message----- From: Holger Freyther [mailto:holger@freyther.de] Sent: Sunday, March 13, 2016 1:04 AM To: Saurabh Sharan Saurabh.Sharan@radisys.com Cc: osmocom-net-gprs@lists.osmocom.org Subject: Re: Padding patch and moving forward with EGPRS support/merge
On 11 Mar 2016, at 14:34, Saurabh Sharan Saurabh.Sharan@radisys.com wrote:
Hello All,
Dear Saurabh,
Based on your inputs and our analysis of the code bases( current master versus Radisys EGPRS) we propose following elements, each to be submitted in incremental patches for merge to master branch • Compression algorithm based on code tree • EPDAN decoding based on the code tree • PUAN encoding based on code tree • MCS 5-9 support in UL • Split block handling in UL • Split block generation in DL • Retransmission with Incremental redundancy • Relevant test suite updates as part of each of the item above
Please let us know your feedback/suggestions for the same.
looks good in general. Please follow the general guideline, try to make small commits, update test results, add new tests. After each of the elements we should not only have a new feature, but better structure, better test coverage and throughput.
kind regards holger
That's already implemented in bitvec_rl(const struct bitvec *bv, bool b); from bitvec.h in libosmocore. See also bitcomp.h for usage details.
On 03/16/2016 01:48 PM, Sangamesh Sajjan wrote:
• Utility to find runlength of one’s or zero’s and addition of Test cases in Unit test framework
Hi
Below are the patches we have identified for the MCS5 - MCS9 in UL activity for EGPRS.
1. Structure definition for EGPRS Header type 1 and 2 in UL. 2. Removing GMSK only check from UL TBF path present in gprs_rlcmac_pdch::rcv_data_block function. 3. Moving "LOGP(DRLCMACUL, LOGL_DEBUG, " UL data: %s\n", osmo_hexdump(data, len));" from function gprs_rlcmac_pdch::rcv_data_block to entry of gprs_rlcmac_pdch::rcv_block function for ease in debugging and the TbfTest.err file modification. 4. Replacing Unnecessary copy constructor for GprsCodingScheme class objects by modifying the function prototype using reference variables. Ex: int gprs_rlcmac_pdch::rcv_block_gprs(uint8_t *data, uint32_t fn, struct pcu_l1_meas *meas, GprsCodingScheme cs); Will be changed to Int gprs_rlcmac_pdch::rcv_block_gprs(uint8_t *data, uint32_t fn, struct pcu_l1_meas *meas, GprsCodingScheme &cs); 5. Decoding::rlc_parse_ul_data_header will be modified with calling separate parser function for GprsCodingScheme::HEADER_GPRS_DATA AND GprsCodingScheme::HEADER_EGPRS_DATA_TYPE_3 cases. 6. Member variable enum Mode m_scheme_type; will be added in class GprsCodingScheme and constructor GprsCodingScheme(Scheme s = UNKNOWN); will be modified to GprsCodingScheme(Scheme s = UNKNOWN,Mode scheme_type = GPRS); and object creation will be modified accordingly along with changes in isGprs() and isEgprs() member functions. 7. New parser function for GprsCodingScheme::HEADER_GPRS_DATA_TYPE_2 case with addition of Test cases in Unit test framework excluding padding case. 8. New parser function for GprsCodingScheme::HEADER_GPRS_DATA_TYPE_1 case with addition of new Test cases Unit test framework. 9. Handling of padding case in Header type 2 including Test case addition in Unit Test framework.
Thanks, Aravind Sirsikar
-----Original Message----- From: Holger Freyther [mailto:holger@freyther.de] Sent: Sunday, March 13, 2016 1:04 AM To: Saurabh Sharan Saurabh.Sharan@radisys.com Cc: osmocom-net-gprs@lists.osmocom.org Subject: Re: Padding patch and moving forward with EGPRS support/merge
On 11 Mar 2016, at 14:34, Saurabh Sharan Saurabh.Sharan@radisys.com wrote:
Hello All,
Dear Saurabh,
Based on your inputs and our analysis of the code bases( current master versus Radisys EGPRS) we propose following elements, each to be submitted in incremental patches for merge to master branch • Compression algorithm based on code tree • EPDAN decoding based on the code tree • PUAN encoding based on code tree • MCS 5-9 support in UL • Split block handling in UL • Split block generation in DL • Retransmission with Incremental redundancy • Relevant test suite updates as part of each of the item above
Please let us know your feedback/suggestions for the same.
looks good in general. Please follow the general guideline, try to make small commits, update test results, add new tests. After each of the elements we should not only have a new feature, but better structure, better test coverage and throughput.
kind regards holger
osmocom-net-gprs@lists.osmocom.org
-
Aravind Sirsikar -
Holger Freyther -
Jacob Erlbeck -
Max -
Sangamesh Sajjan -
Saurabh Sharan