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 5: -Code-Review

(5 comments)

File src/osmo-bsc/neighbor_ident_ctrl.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/60628ad1_8a67212f PS3, Line 118: llist_for_each_entry(n, &bts->neighbors, entry) {

Hmm alright. […]

I see what you mean now, "neighbor-bts" prefix means only those added by its bts_nr. Indeed then maybe filter out by neighbor->type == NEIGHBOR_TYPE_BTS_NR here. I think it may be a good idea that you reach the user and ask him for these details, since the provided example doesn't show up this kind of details. Sorry for all the discussions here, all the neighbor stuff is always a bit confusing since there are many options.

File src/osmo-bsc/neighbor_ident_ctrl.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/246e7635_7ad5c447 PS5, Line 113: cmd->reply = "bts not found."; I don't see the error lines in other functions using a dot at the end. Let's try to be consistent (I know sometimes we aren't ;)

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/f84b3447_773bd4e0 PS5, Line 119: if (resolve_local_neighbor(&neighbor_bts, bts, n)) { Better check against "< 0" here, as usual for system programming and error codes, it's clearer. Otherwise I had to look whether this returned a pointer.

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/6696b8eb_8bc42b65 PS5, Line 120: LOGP(DCTRL, LOGL_ERROR, "Error resolving neighbor entry for BTS %" PRIu8 ".\n", bts->nr); I wouldn't log an error here, it is expected that some neighbors are remote (BTS under another BSSC), and hence it is expected that some neighbor_bts may not be found. see resolve_neighbors() as an example.

File tests/ctrl_test_runner.py:

https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/37de1c71_39f67617 PS5, Line 630: self.assertEqual(r['value'], None) Shouldn't this be an empty string? Or is None simply somehow converted to an empty string?