Attention is currently required from: pespin. arehbein 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 6:
(6 comments)
Patchset:
PS3:
You went the wrong direction by moving that into "neighbor-bts", you need to revert that and go back […]
I'm assuming that was correct after all, since someone (not me) marked this as resolved. To me it looks like it's in the right place (where "neighbor-bts add/del" also are)
File src/osmo-bsc/neighbor_ident_ctrl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/a874b02d_0d1fc46b PS3, Line 118: llist_for_each_entry(n, &bts->neighbors, entry) {
I see what you mean now, "neighbor-bts" prefix means only those added by its bts_nr. […]
I asked and it's how you understood it
File src/osmo-bsc/neighbor_ident_ctrl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/11bd17da_3d51c7ca PS5, Line 113: cmd->reply = "bts not found.";
I don't see the error lines in other functions using a dot at the end. […]
I actually copied this from somewhere else in the code, but I'll keep it in mind
https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/2d3def39_f1b42ef6 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. […]
Ah okay, is it a general Osmocom principle to return error codes as negative numbers always?
https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/72e9ed49_f812615a 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) […]
Done
File tests/ctrl_test_runner.py:
https://gerrit.osmocom.org/c/osmo-bsc/+/30982/comment/66ed685e_2d18b1df 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?
Yes it's converted, not sure if in the Python scripts or if the CTRL reply automatically omits message parts that would be empty strings. I initially put `two single quotes there, but then the assert would fail due to the conversion, because `None` was parsed.