Attention is currently required from: neels, nt2mku. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36784?usp=email )
Change subject: Omit octet 3a in mobile-terminating setup requests if speech version is GSM-FR only ...................................................................... Patch Set 2: (5 comments) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/36784/comment/04d19fe5_db78a8f1 PS2, Line 7: mobile-terminating setup requests Please note that `gsm48_encode_bearer_cap()` is also used by osmocom-bb, so this change also affects mobile originating CC SETUP messages. I suggest to change commit name to `gsm48_encode_bearer_cap(): omit octet 3a if only GSM-FR is supported`. https://gerrit.osmocom.org/c/libosmocore/+/36784/comment/ff0a3a8a_0adcbf0e PS2, Line 9: Some early GSM phones (like the Siemens P1 Porty) do not accept a mobile-terminating call setup if octet 3a is present. If speech version is GSM-FR (v1, 0x00) only, omit octet 3a. cosmetic: the usual line length limit for COMMIT_MSG is 72 chars, so please add line break(s). Patchset: PS2: As can be seen, the build verification is failing because the output of `tests/gsm0408` has changed: ``` --- experr 2024-05-12 19:45:16.719230477 +0000 +++ /build/builddir/tests/testsuite.dir/at-groups/28/stderr 2024-05-12 19:45:16.727230501 +0000 @@ -1,6 +1,3 @@ Incorrect encoded result of CSD 2400/V.22bis/transparent: should: 07 a2 b8 81 21 13 43 83 is: 07 a2 88 81 21 13 43 83 -Incorrect encoded result of Speech, without octet 3a: - should: 01 a0 - is: 02 20 80 --- expout 2024-05-12 19:45:16.719230477 +0000 +++ /build/builddir/tests/testsuite.dir/at-groups/28/stdout 2024-05-12 19:45:17.471232722 +0000 @@ -2,7 +2,7 @@ Test `CSD 4800/RLP/non-transparent' passed Test `CSD 2400/V.22bis/transparent' failed Test `Speech, all codecs' passed -Test `Speech, without octet 3a' failed +Test `Speech, without octet 3a' passed Simple TMSI encoding test....passed Simple IMSI encoding test....passed: [10] 17 08 99 10 07 00 00 00 64 02 ``` You need to amend the expected output in order to make the build verification pass, which can be done by running `make -C tests/ update_exp`. File src/gsm/gsm48_ie.c: https://gerrit.osmocom.org/c/libosmocore/+/36784/comment/1c7b7bea_045d8bdf PS2, Line 330: /* if we have only one speech version, which is FR, skip octet 3a */ Let's add a spec. reference here: ``` /* According to 3GPP TS 24.008, section 10.5.4.5.1, octet 3a etc. shall be included only if the mobile station supports CTM text telephony or if it supports at least one speech version for GERAN other than V1 (FR or HR). */ ``` https://gerrit.osmocom.org/c/libosmocore/+/36784/comment/fea112e0_80f06b9f PS2, Line 331: bcap->speech_ver[1] >= 0 || (bcap->speech_ver[0] & 0x0f) != 0x00 Consider the following input: ``` bcap->speech_ver[] = { GSM48_BCAP_SV_HR /* 0x01 */ GSM48_BCAP_SV_FR /* 0x00 */ }; ``` In this case it's still only the speech version 1, but octets 3[ab] will be encoded. We should also still include octet(s) 3a etc. if `bcap->speech_ctm != 0`. I suggest adding unit tests for that too. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36784?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ia09abb32a8458384151a6ae28744835ea440fc5b Gerrit-Change-Number: 36784 Gerrit-PatchSet: 2 Gerrit-Owner: nt2mku <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Attention: nt2mku <[email protected]> Gerrit-Comment-Date: Mon, 13 May 2024 14:36:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
