pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/25108 )
Change subject: Move tbf ul_ack_state to osmocom FSM ...................................................................... Patch Set 4: (2 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/25108/4/src/gprs_rlcmac_sched.cpp File src/gprs_rlcmac_sched.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/25108/4/src/gprs_rlcmac_sched.cpp@176 PS4, Line 176: //msg = tbfs->ul_ack->create_ul_ack(fn, ts); > looks like it's not on purpose? Ack https://gerrit.osmocom.org/c/osmo-pcu/+/25108/4/src/tbf_ul_ack_fsm.c File src/tbf_ul_ack_fsm.c: https://gerrit.osmocom.org/c/osmo-pcu/+/25108/4/src/tbf_ul_ack_fsm.c@253 PS4, Line 253: /* FIXME: validate FN and TS match: && ctx->poll_fn = fn && ctx->poll_ts == ts */ > * indentation […] Yes, it's out of the scope since it was not done beforehand. I'm just adding it here to remember in the future to add this kind of checks in the future (I already do it for instance in NACC FSM). It's easy to do it, but doing so would change the logic or add further restrictions which would show up a bug, and I'm not willing to change behavior in this patchset. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/25108 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Icf23bf5a4b85fbcbf1542cebceb76b9ba7185d30 Gerrit-Change-Number: 25108 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: osmith <[email protected]> Gerrit-Comment-Date: Fri, 30 Jul 2021 15:04:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <[email protected]> Gerrit-MessageType: comment
