fixeria 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: (5 comments) 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? Other *_sent members are using int. Feel free to fix this in a separate change, I don't think it's critical and deserves that much attention. 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; > move them to be in the same place ACK. 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. Huh, I thought we're doing this already for all commands. And that's why we decided to keep the requested version in the response... As it turns out, we do match the command only. For sure, I can add SETFORMAT here, but what about other commands? What if we send 'CMD TXTUNE 890000', but receive 'RSP TXTUNE 000000'? Why this matching should be implemented for every separate command individually? 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. […] ACK. 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). ACK! -- 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 <axilira...@gmail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <axilira...@gmail.com> Gerrit-Reviewer: laforge <lafo...@gnumonks.org> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-CC: ipse <alexander.cheme...@gmail.com> Gerrit-Comment-Date: Thu, 04 Jul 2019 15:41:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pes...@sysmocom.de> Gerrit-MessageType: comment
