osmo-pcu[master]: Add missing multislot classes

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Holger Freyther gerrit-no-reply at lists.osmocom.org
Thu Oct 5 09:23:29 UTC 2017


Patch Set 1:

(6 comments)

Assuming these tables are correctly copy and pasted. What is the minimal dependency to get this in? Which patch removes the <= 32 assumption?

tests/alloc/AllocTest.cpp requires updates to use/test the new classes as well.

The commit message needs some work. You are not "adding missing" classes but you extend from one version of the specification to the other. Also indicate which devices use the classes and if you had a change to verify it.. Also how you intend to address the 35-45 TA issue.

The comment that old code had it too might be misleading as the requirement didn't exist for the other classes or if they did we have more work to do on the tables.

https://gerrit.osmocom.org/#/c/4072/1//COMMIT_MSG
Commit Message:

Line 10: Emacs macros into C struct to avoid typos.
We don't do reformat and extend in one go. We rarely do. As a reviewer I still have to see if things change. :(


Line 13: yet (this was the case with the old code too).
Good to describe it.


https://gerrit.osmocom.org/#/c/4072/1/src/gprs_rlcmac_ts_alloc.cpp
File src/gprs_rlcmac_ts_alloc.cpp:

Line 41: /* FIXME: use actual TA offset for computation - make sure to adjust "1 + MS_TO" accordingly */
Good to communicate it.


Line 52: 	/*           | Rx     Tx   Sum |  Tta    Ttb    Tra    Trb |      */
this header is nice


Line 54: 	/*  1 */  {   1,     1,     2,     3,     2,     4,     2,     1 },
To be more interesting we can even do [0] = {}, [1].. but that is not a call to action just to illustrate that there are always things to do better but the cost vs. value vs. danger might not be big enough to warrant making a change.


Line 83: 	/* 30 */  {   5,     1,     6,     2,     1,     1,     1,     1 },
Any idea why we padded the structs before? Were they not assigned? Why did we extend it?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ef2eb99c517f25e7d1e71b985a3e0eb3879eb2c
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list