Change in osmo-bts[master]: l1sap: merge MEAS IND into PRIM PH DATA / PRIM TCH

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/.

dexter gerrit-no-reply at lists.osmocom.org
Mon Nov 11 13:08:22 UTC 2019


dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/15918 )

Change subject: l1sap: merge MEAS IND into PRIM PH DATA / PRIM TCH
......................................................................


Patch Set 4:

(9 comments)

(ensure draft replies are sent)

https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/l1sap.c 
File src/common/l1sap.c:

https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/l1sap.c@67 
PS1, Line 67:  * indication for itsself that is passed up in parallel to the payload data
> itself
Done


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/l1sap.c@74 
PS1, Line 74: static bool tch_data_meas_present = true;
> ACK.
I wonder if there is a failsafe way to do this. We could have a function in l1sap.c that is called by the bts model and then does the switching. A per-bts-model API is usually a very difficult thing as in theory it would require a test with each model and I do not even have the hardware for each and every BTS, so its always blind flying.


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/l1sap.c@646 
PS1, Line 646: 	struct info_meas_ind_param *info_meas_ind = &l1sap->u.info.u.meas_ind;
> Let's rather move these assignments under each switch case.
Done


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/l1sap.c@695 
PS1, Line 695: 		 "%s MPH_INFO meas ind, ta_offs_256bits=%d, ber10k=%d, inv_rssi=%u\n",
> MPH_INFO only on some case right? maybe add a string pointing to the name in the switch case.
Done


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/l1sap.c@740 
PS1, Line 740: 		rc = 0;
> rc is already zero here, it can be dropped.
Done


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/scheduler.c 
File src/common/scheduler.c:

https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/scheduler.c@760 
PS1, Line 760: l1sap->u.tch.
> How about 'is_sub'? (still not aware of its meaning)
I am also a bit confused about the is_sub struct member. As far as I know it is used for DTX to mark special frames which must not be removed by DTX. I think this is to distinguish between real bad frames and those what are just not sent. My guess is that osmo-bts-trx seems not to support this for some reason, but phy based BTS might support it and set the flag then? I am not sure.


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/osmo-bts-trx/scheduler_trx.c 
File src/osmo-bts-trx/scheduler_trx.c:

https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/osmo-bts-trx/scheduler_trx.c@a205 
PS1, Line 205: /* FIXME: use actual values for BER etc */
> Not sure why you're removing this comment...
The problem here is that we can not use actual values for BER. This part of the code generates an artificial frame to satisfy the higher layers. We can only use artificial values. A BER value of 10000 is basically 100% Error, which matches the reality here. The rssi of -110 is fake, but realistic. I wonder about the ta_offs_256bits, maybe we should try to find some better approximation than 0 here...


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/osmo-bts-trx/scheduler_trx.c@a991 
PS1, Line 991: /* Send uplink measurement information to L2 */
> This comment does not make sense anymore.
Done


https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/osmo-bts-trx/scheduler_trx.c@a1105 
PS1, Line 1105: /* Send uplink measurement information to L2 */
> Same.
Done



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15918
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I710d0b7cf193afa8515807836ee69b8b7db84a84
Gerrit-Change-Number: 15918
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 11 Nov 2019 13:08:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Comment-In-Reply-To: fixeria <axilirator at gmail.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191111/a250fd5c/attachment.htm>


More information about the gerrit-log mailing list