osmo-pcu[master]: Add data structure for ARQ-II in EGPRS DL

[arvind.sirsikar]

Patch Set 1:

(3 comments)

https://gerrit.osmocom.org/#/c/332/1/src/gprs_coding_scheme.cpp
File src/gprs_coding_scheme.cpp:

Line 26:  * In has 3 level indexing. 0th level is ARQ type
> It instead of In?
I will modify the line.


Line 33: /* TODO: Need to support ARQ type 1 */
> There will be more than two dimensions in that array? What is missing? What
It is already 3 dimensional array. There is nothing missing. 0th level of indexing already has 2 sub-indexing. 0th sub indexing is required to handle SPB for EGPRS DL and 1st level sub-indexing is required to support ARQ2. For this patch series only 1st sub level indexing is supported and tested. SPB will be released after this patch series gets merged to master.


https://gerrit.osmocom.org/#/c/332/1/src/rlc.h
File src/rlc.h:

Line 130: 	GprsCodingScheme cs_current_trans;
> I would prefer to cs and cs_current_trans being private here.
The cs_current_trans is made public to align with existing variables like block[RLC_MAX_LEN], len and others to have consistency. will it make sense to have cs and cs_current_trans as private?.

We can rename cs to cs_last as suggested in review of other patch. But it will have modification in other source files related to UL TBF and DL TBF to address compilation errors. In that case the meaning of this patch will be lost and will not be restricted to only EGPRS DL ARQ2 support.

Please comment.


-- 
To view, visit https://gerrit.osmocom.org/332
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I734b1024bb32f2daa43af4adf59f4a17f2294afe
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: arvind.sirsikar <arvind.sirsikar at radisys.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com>
Gerrit-HasComments: Yes