neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/18692 )
Change subject: msc_a: add callref as call id to ASSIGNMENT REQ. ...................................................................... Patch Set 2: Code-Review-1 (2 comments) (would be nice to also see the code that is using this new IE in the same patch, or mention in the commit log the change id of the patch that will add that later) https://gerrit.osmocom.org/c/osmo-msc/+/18692/2/include/osmocom/msc/ran_msg.h File include/osmocom/msc/ran_msg.h: https://gerrit.osmocom.org/c/osmo-msc/+/18692/2/include/osmocom/msc/ran_msg.h@89 PS2, Line 89: uint32_t call_id; each of these structs must allow indicating presence or absence of optional IEs. The earlier patch set using call_id_present was exactly the right thing to do. sorry to add another iteration, but please bring back call_id_present. (The idea of these structs is to represent 1:1 what the encoded message contained/should contain, and not make any assumptions on how individual IEs interact.) https://gerrit.osmocom.org/c/osmo-msc/+/18692/2/src/libmsc/ran_msg_a.c File src/libmsc/ran_msg_a.c: https://gerrit.osmocom.org/c/osmo-msc/+/18692/2/src/libmsc/ran_msg_a.c@998 PS2, Line 998: call_id = &ac->call_id; I don't understand why the call_id is related to RTP address. I prefer the earlier patch set handling the call_id separately by call_id_present. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/18692 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I4288f47e4a6d61ec672f431723f6e72c7c6b0799 Gerrit-Change-Number: 18692 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pma...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-CC: laforge <lafo...@osmocom.org> Gerrit-Comment-Date: Sun, 14 Jun 2020 12:10:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment