pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/14611 )
Change subject: osmo-bts-trx/trx_if.c: request the newest TRXD header version ...................................................................... Patch Set 5: Code-Review-1 (6 comments) https://gerrit.osmocom.org/#/c/14611/5//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/14611/5//COMMIT_MSG@17 PS5, Line 17: version, which can be either equal or lover than VER_REQ. In love is in the air! or maybe lower. https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/l1_if.h File src/osmo-bts-trx/l1_if.h: https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/l1_if.h@11 PS5, Line 11: int setformat_sent; bool? https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/l1_if.c File src/osmo-bts-trx/l1_if.c: https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/l1_if.c@214 PS5, Line 214: l1h->config.setformat_sent = 1; = true. Also, you set trxd_hdr_ver_req inside trx_if_cmd_setformat(), but this one here. Please move them to be in the same place. https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/trx_if.c File src/osmo-bts-trx/trx_if.c: https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/trx_if.c@446 PS5, Line 446: Seems we want to add code here to match the version parameter in SETFORMAT. https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/trx_if.c@486 PS5, Line 486: if (rsp->status < 0 || rsp->status > TRX_DATA_FORMAT_VER) { You should check here if rsp->status > l1h->config.trxd_hdr_ver_req, since it should be equal or lower than the one you sent, not the one you support (which is unknown to trx). https://gerrit.osmocom.org/#/c/14611/5/src/osmo-bts-trx/trx_if.c@529 PS5, Line 529: } else if (strcmp(tcm->cmd, "SETFORMAT") == 0) { Add a comment here on why tcm->cmd is used instead of rsp->cmd (due to RSP ERR 1 being answered). -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/14611 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8afe950bd1ec2afaf3347ff848ee46e69c4f5011 Gerrit-Change-Number: 14611 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: ipse <[email protected]> Gerrit-Comment-Date: Thu, 04 Jul 2019 14:04:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
