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. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31882?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id4779f033b5eb1742462d4efc28a0398645acfe6 Gerrit-Change-Number: 31882 Gerrit-PatchSet: 8 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Tue, 05 Dec 2023 12:10:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment