Attention is currently required from: neels, msuraev.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286 )
Change subject: rlcmac: Introduce DL TBF creation through PCH ImmAss
......................................................................
Patch Set 5:
(3 comments)
File include/osmocom/gprs/rlcmac/tbf_dl.h:
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/889c0970_09976e8d
PS5, Line 9: //#include <osmocom/gprs/rlcmac/tbf_dl_fsm.h>
What's the point of commented include? Just drop
it if it's unused.
It will be added soon in a follow up commit. Having it here
it's just saving me time adding it later everywhere.
I don't see a problem with having this here since anyway this is not fully usable for
regular end users yet.
File src/rlcmac/rlcmac.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/85f0ecf9_393b37c0
PS5, Line 291: LOGRLCMAC(LOGL_NOTICE, "TODO: handle decoded dl ctrl
block!\n");
hmm, a TODO in the logs? maybe a #warning pragma
instead?
no, TODO is fine because we are actively developing this and we want to
clearly see if some part of the code is missing at runtime when doing tests.
File src/rlcmac/tbf_dl.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/398ddb6d_9f24f648
PS5, Line 39: // goto err_tbf_destruct;
i can understand that some code will come in only
later, but you do add an awful lot of these tempor […]
All of them comments are
related to the dl_tbf_fsm, they relate to the same part of the code. If this really annoys
you I can spend time dropping it here and adding it in one of next patches I have to work
on. It will simply make me or anyone else taking over this lose time finding out what
needs to be added where.
This is all the boilerplate code to alloc+initialize+free the state FSM of a dl_tbf,
similar to what we already have partially implemented for ul_tbf.
--
To view, visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I7f98e3456ef35d80becdad3481afeb771457b0ef
Gerrit-Change-Number: 31286
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: msuraev <msuraev(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Feb 2023 10:00:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment