Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/30982 )

Change subject: bsc_ctrl_commands: Add GET for bts neighbor-list (local bts numbers) ......................................................................

Patch Set 1: Code-Review-1

(8 comments)

File src/osmo-bsc/bts_ctrl.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/b9aa24e8_15da6ebb PS1, Line 587: OSMO_STRBUF_PRINTF(*csv, "%" PRIu8 ",", n->bts_nr); You actually need to print bts->nr, not bts->bts_nr. The later is the bts id in the specific BTS<->BSC link, which is usually 0. "nr" is the ID you see in the VTY config and which identifies the BTS within the BSC.

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/354204ec_b593d6c2 PS1, Line 607: if (csv.chars_needed >= csv.len) { /* Just to be save, although this is very unlikely */ It's impossible that this is triggered. We can only have up to 255 BTS (limited by uint8_t bts->nr). Hence, even if all of the ids would be 3 digits, the string would be 255*3 + 255 (for commas), so we can never reach 4096. You can even lower the buffer up to 1024 actually.

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/20a71361_dbfa986f PS1, Line 614: write_neighbors(&bts->neighbors, &csv); since you no longer need this function twice, just drop te function and inline it's just a loop.

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/381461b7_e6929272 PS1, Line 617: cmd->reply = "No BTS neighbors configured through BTS number"; return empty string here, it's way easier to parse by clients.

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/c481a1de_d28cdc62 PS1, Line 630: CTRL_CMD_DEFINE_RO(bts_neighbor_list, "neighbor-list"); My understanding is that we already have "bts.N.neighbor-bts.add" and "bts.N.neighbor-bts.del", so adding here a "bts.N.neighbor-bts.bts-list" cmd would make more sense.

You could also name it "bts.N.neighbor-bts.by-bts-nr"

File tests/ctrl/osmo-bsc-neighbor-list.cfg:

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/f27dc1e3_74eaee4d PS1, Line 1: network Please if possible drop all config lines which are not really needed/used by the test.

File tests/ctrl_test_runner.py:

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/b9514dfd_3eb3172a PS1, Line 529: Unrelated change, submit a new patch if needed.

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/17f884e1_66a13067 PS1, Line 580: self.assertEqual(r['value'], 'No BTS neighbors configured through BTS number') this makes it difficult for the client. Simply return an empty string in osmo-bsc in this case.