Attention is currently required from: pespin, dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34060 ) Change subject: pcu_sock: use PCU_IF_SAPI_AGCH_2 instead PCU_IF_SAPI_AGCH ...................................................................... Patch Set 5: Code-Review-1 (4 comments) File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/70221348_392c3cd9 PS5, Line 445: confirmation Same as in the related osmo-bts patch: "Sending confirmation" may be confusing, let's make this cleaner by saying "Sending DATA.cnf" or "Sending data confirmation". https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/8854a18c_2a8ff1d0 PS5, Line 533: struct gsm_pcu_if_agch *agch; const https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/5e4d6202_3507948f PS5, Line 551: rc = -EIO; So if `agch->confirm == true`, but `rsl_imm_assign_cmd()` fails, we still send the confirmation. This looks wrong to me. Am I missing something? https://gerrit.osmocom.org/c/osmo-bsc/+/34060/comment/244a48d5_1814d863 PS5, Line 555: gsm48_imm_ass = (struct gsm48_imm_ass *)agch->data; You're setting this pointer, but not using it within this `case`. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34060 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I709c27adaf09a6766cfde4d76d878626d30ebb3c Gerrit-Change-Number: 34060 Gerrit-PatchSet: 5 Gerrit-Owner: dexter <pmaier@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: osmith <osmith@sysmocom.de> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-Attention: pespin <pespin@sysmocom.de> Gerrit-Attention: dexter <pmaier@sysmocom.de> Gerrit-Comment-Date: Wed, 30 Aug 2023 22:51:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment