osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/22436 )
Change subject: sched: Avoid picking TBF with nacked dl blocks when GMSK is required ...................................................................... Patch Set 1: (3 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/22436/1//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/c/osmo-pcu/+/22436/1//COMMIT_MSG@19 PS1, Line 19: later on priority (can you avoid using 'later' twice with different meaning to make it more readable?) https://gerrit.osmocom.org/c/osmo-pcu/+/22436/1/src/gprs_rlcmac_sched.cpp File src/gprs_rlcmac_sched.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/22436/1/src/gprs_rlcmac_sched.cpp@325 PS1, Line 325: (prio !=DL_PRIO_CONTROL && !(prio == DL_PRIO_NEW_DATA && w->resend_needed() < 0))) { The if gets very complex. How about refactoring this into a function instead? Something like this (if using it, make sure that all the true / false are right, it's just a draft): bool can_downgrade(tbf, req_msc_kind, prio) { if (req_msc_kind == ...) return true; if (!tbf->is_egprs_enabled()) return true; switch (prio) { case DL_PRIO_CONTROL: return false; case DL_PRIO_NEW_DATA: return w->resend_needed(); } return false; } https://gerrit.osmocom.org/c/osmo-pcu/+/22436/1/src/gprs_rlcmac_sched.cpp@325 PS1, Line 325: =D (missing space, fix it while at it?) -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/22436 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I253de8e1a190a9adb56160f38892c9e43e2c0272 Gerrit-Change-Number: 22436 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-Comment-Date: Tue, 26 Jan 2021 12:40:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
