Attention is currently required from: arehbein.

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31882?usp=email )

Change subject: Make BSSGP timing data configurable ......................................................................

Patch Set 8:

(5 comments)

File include/osmocom/bsc/vty.h:

https://gerrit.osmocom.org/c/osmo-bsc/+/31882/comment/c7ac3904_99c10ccd PS8, Line 106: /* Add additional group strings for vty command generation here */ IMHOyou should in general avoid filling your commits in a patchset with this type of comments which only make them more complex to review. This kind of stuff is for you keep private during dev imho.

File src/osmo-bsc/bsc_vty.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31882/comment/59055467_74eb3aaf PS8, Line 323: /* TODO: Add other options for group argument bssgp '[(rlc|bssgp|ns)]' when adding additional timer groups. Same with this one.

File src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31882/comment/6814fd72_fbcff715 PS8, Line 212: .t1_s = osmo_tdef_get(g->tdefs, 1, OSMO_TDEF_S, -1), so you added defines for timers 1..4 but you are not using them?

File src/osmo-bsc/bts_vty.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31882/comment/00deb420_fbbce117 PS8, Line 1704: struct tdef_data gprs_bssgp_timer_tdef_data[11] = { I still find this is most probably not needed.

You can probably change the indices in gprs_bssgp_cfg_strs to directly be the GSM_BTS_TDEF_ID_BSSGP_* values?

https://gerrit.osmocom.org/c/osmo-bsc/+/31882/comment/6f2ae417_4db4b14d PS8, Line 1741: int idx = get_string_value(gprs_bssgp_cfg_strs, argv[0]), val = atoi(argv[1]); definetly not in a single line.