Change in osmo-bsc[master]: bsc_ctrl_commands: Add GET for bts neighbor-list (local bts numbers) - gerrit-log

27 Jan 2023

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.

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/30982
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I79aeffd93089086f57c66787fe20b439a4d8b6b4
Gerrit-Change-Number: 30982
Gerrit-PatchSet: 6
Gerrit-Owner: arehbein &lt;arehbein(a)sysmocom.de&gt;
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: msuraev &lt;msuraev(a)sysmocom.de&gt;
Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Comment-Date: Fri, 27 Jan 2023 00:56:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein &lt;arehbein(a)sysmocom.de&gt;
Comment-In-Reply-To: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-MessageType: comment