Change in osmo-bsc[master]: WIP: Support set up of C/I parameters for osmo-bts MS Power Control Loop

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

pespin gerrit-no-reply at lists.osmocom.org
Wed Sep 1 11:28:40 UTC 2021


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/25298 )

Change subject: WIP: Support set up of C/I parameters for osmo-bts MS Power Control Loop
......................................................................


Patch Set 2:

(12 comments)

https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/include/osmocom/bsc/gsm_data.h 
File include/osmocom/bsc/gsm_data.h:

https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/include/osmocom/bsc/gsm_data.h@1346 
PS2, Line 1346: 	/* Measurement averaging parameters for C/I: */
> I think using an array with pre-defined indexes would simplify the code: […]
I also though about it, but I don't think it really helps, because anyway the RSL messages are not using arrays, so you still need to copy to/from specific offsets, which means you cannot easily use loops. And I want to avoid using arrays with enums in the RSL protocol because that would also require speechifying enums for the protocol, adding even more non-standard stuff.

So I agree some code can look a bit verbose, but it's manily due to having lots of parameters added. I can move some big macros to a helper function if it really makes it less hard to read.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_ipaccess_nanobts.c 
File src/osmo-bsc/bts_ipaccess_nanobts.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_ipaccess_nanobts.c@870 
PS2, Line 870: enc_osmo_meas_proc_params
> Why do we add osmo-bts specific code in bts_ipaccess_nanobts. […]
Ok I can move it there.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_ipaccess_nanobts.c@889 
PS2, Line 889: #define ENC_PROC(PARAMS, TO, TYPE) do { \
I could move this to a helper function, but not sure if we'd really win a lot, opinions/preferences welcome.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_ipaccess_nanobts.c@994 
PS2, Line 994: 		#define ENC_THRESH_CI(TYPE) \
I think it's fine keeping this one and the one below as macros.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c 
File src/osmo-bsc/bts_vty.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3063 
PS2, Line 3063: Channel Type
> FR/EFR is not really a 'Channel type', but rather 'Codec type'.
I'm just following naming used in "GSM/EDGE: Evolution and Performance" Table 10.3.
It's a channel type in the generic sense, not in "GSM logical channel type". I don't care about changing it to Codec Type if you prefer though.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3068 
PS2, Line 3068: Channel Type (E)GPRS
> Can you please explain how is this related to GPRS? Currently all parameters in 'config-{ms,bs}-powe […]
So far, I'm just adding it for completeness when doing all the monkey-type work, since we got values for it too.
It's still not clear to me how are we going to use it, I never had a look at MS power control on PDCH but I guess there may be some way to tell the MS to reduce the power from the PCU?
So yes, we'd pass that to the PCU at some point from the BTS.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3137 
PS2, Line 3137: ,
> Hmm, maybe worth adding a comment like: , /* nope */, because two comas look odd.
ACK good idea.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3190 
PS2, Line 3190: lower_cmp_p, lower_cmp_n
> Ha! I also copy-pasted it here :P
ACK I'll send a separate patch.


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3209 
PS2, Line 3209: Upper
> Lower?
Ack


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3227 
PS2, Line 3227: lower_cmp_p, lower_cmp_n
> Copy-pasted, should be upper (also see comment above).
Ack


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/src/osmo-bsc/bts_vty.c@3354 
PS2, Line 3354: -toI
> -to-
Ack


https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/tests/power_ctrl.vty 
File tests/power_ctrl.vty:

https://gerrit.osmocom.org/c/osmo-bsc/+/25298/2/tests/power_ctrl.vty@78 
PS2, Line 78:    ci-thresh fr lower 13 upper 17
> Why do we print C/I parameters for *BS* power control if they only apply to MS power control?
Indeed my fault, thanks for noticing.



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I7e76ec47b323d469f777624b74b08752d1f5584f
Gerrit-Change-Number: 25298
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Sep 2021 11:28:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210901/f555151b/attachment.htm>


More information about the gerrit-log mailing list