Attention is currently required from: fixeria.

csaba.sipos has posted comments on this change by csaba.sipos. ( https://gerrit.osmocom.org/c/osmo-bsc/+/39416?usp=email )

Change subject: nokia_site: Add object_identity, object_state and object_identity_state attributes ......................................................................

Patch Set 3:

(3 comments)

File src/osmo-bsc/bts_nokia_site.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/39416/comment/936459d4_1b72b5c5?usp=... : PS3, Line 1823: if (find_element

Could you please explain the idea of using `if` statement without the actual body? I may be missing […]

Acknowledged

https://gerrit.osmocom.org/c/osmo-bsc/+/39416/comment/52968728_bdc62a8d?usp=... : PS3, Line 1825: get_object_identity_string(object_id_state[4]), object_id_state[5], get_object_state_string(object_id_state[10]));

Please align this line to the opening brace `(` and make sure to not exceed the line length of 120 c […]

Acknowledged

https://gerrit.osmocom.org/c/osmo-bsc/+/39416/comment/54bb03a2_f3f7e31a?usp=... : PS3, Line 1853: if (find_element

Again, what's the idea here? If the element must be present according to the protocol specification, […]

For BTS_ALARM the object id and object state and the object_id_and_state are all optional (but possible). My problem is the printout: to make the output nice and compact I am using the already available methods, and I have no idea how to do the required extra checks without an awful lot of extra if_else or case. To add to the fun: this "optional" nature likely comes from older BTS models in this family we dont support, as I have never seen an alarm message that does not have them. And making an assert check is a lot less lines 😊