Max has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/13216 )

Change subject: Fix Channel Coding Command for MCS
......................................................................

Fix Channel Coding Command for MCS

Previously result of ".to_num() - 1" was used without any checks which
means that in case of to_num() returning zero we would effectively try
to encode (uint8_t)(-1).

Let's fix this by using proper mcs_chan_code() function which returns
Channel Coding Command for MCS without the need to further correct it
and adjust expected tests output accordingly.

Change-Id: I868062a81fffe6714a811c032215f25a79259905
---
M src/encoding.cpp
M tests/tbf/TbfTest.cpp
M tests/types/TypesTest.cpp
M tests/types/TypesTest.ok
4 files changed, 12 insertions(+), 12 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  osmith: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/src/encoding.cpp b/src/encoding.cpp
index 9920b46..b0d0467 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -268,7 +268,7 @@
        bitvec_write_field(dest, &wp, usf, 3);    // USF
        bitvec_write_field(dest, &wp, 0, 1);    // USF_GRANULARITY
        bitvec_write_field(dest, &wp, 0, 1);   // "0" power control: Not Present
-       bitvec_write_field(dest, &wp, tbf->current_cs().to_num() - 1, 2);    // 
CHANNEL_CODING_COMMAND
+       bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 2);    
// CHANNEL_CODING_COMMAND
        bitvec_write_field(dest, &wp, 1, 1);    // TLLI_BLOCK_CHANNEL_CODING
        if (alpha) {
                bitvec_write_field(dest, &wp, 0x1, 1);   // ALPHA = present
@@ -315,7 +315,7 @@
        CHECK(rc);

        /* 3GPP TS 44.060 §12.10d EGPRS Modulation and coding Scheme 
description: */
-       rc = bitvec_set_u64(dest, tbf->current_cs().to_num() - 1, 4, false); /* 
EGPRS CHANNEL_CODING_COMMAND */
+       rc = bitvec_set_u64(dest, mcs_chan_code(tbf->current_cs()), 4, false); 
/* EGPRS CHANNEL_CODING_COMMAND */
        CHECK(rc);

        /* TLLI_BLOCK_CHANNEL_CODING */
@@ -571,7 +571,7 @@

        if (!use_egprs) {
                bitvec_write_field(dest, &wp,0x0,1); // Message escape
-               bitvec_write_field(dest, &wp,tbf->current_cs().to_num()-1, 2); 
// CHANNEL_CODING_COMMAND
+               bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 
2); // CHANNEL_CODING_COMMAND
                bitvec_write_field(dest, &wp,0x1,1); // 
TLLI_BLOCK_CHANNEL_CODING
                write_ta_ie(dest, wp,tbf->ta(), ta_idx, ta_ts);
        } else { /* EPGRS */
@@ -579,7 +579,7 @@
                bitvec_write_field(dest, &wp,0x0,2); // EGPRS message contents
                bitvec_write_field(dest, &wp,0x0,1); // No 
CONTENTION_RESOLUTION_TLLI
                bitvec_write_field(dest, &wp,0x0,1); // No COMPACT reduced MA
-               bitvec_write_field(dest, &wp,tbf->current_cs().to_num()-1, 4); 
// EGPRS Modulation and Coding IE
+               bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 
4); // EGPRS Modulation and Coding IE
                /* 0: no RESEGMENT, 1: Segmentation*/
                bitvec_write_field(dest, &wp, 0x1, 1);
                write_ws(dest, &wp, tbf->window_size()); // EGPRS Window Size
@@ -809,7 +809,7 @@
        struct gprs_rlcmac_ul_tbf *tbf, bool is_final)
 {

-       bitvec_write_field(dest, &wp, tbf->current_cs().to_num() - 1, 2); // 
CHANNEL_CODING_COMMAND
+       bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 2); // 
CHANNEL_CODING_COMMAND
        write_packet_ack_nack_desc_gprs(bts, dest, wp, tbf->window(), is_final);

        bitvec_write_field(dest, &wp, 1, 1); // 1: have 
CONTENTION_RESOLUTION_TLLI
@@ -991,7 +991,7 @@
        bitvec_write_field(dest, &wp, 0, 2); // fixed 00
        /* CHANNEL_CODING_COMMAND */
        bitvec_write_field(dest, &wp,
-               tbf->current_cs().to_num() - 1, 4);
+               mcs_chan_code(tbf->current_cs()), 4);
        /* 0: no RESEGMENT, 1: Segmentation*/
        bitvec_write_field(dest, &wp, 1, 1);
        bitvec_write_field(dest, &wp, 1, 1); // PRE_EMPTIVE_TRANSMISSION, TODO: 
This resembles GPRS, change it?
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 6dc802c..151e7fa 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -739,7 +739,7 @@

        struct msgb *msg1 = ul_tbf->create_ul_ack(*fn, ts_no);

-       static uint8_t exp1[] = { 0x40, 0x24, 0x01, 0x3f, 0x3e, 0x24, 0x46, 
0x68, 0x90, 0x87, 0xb0, 0x06,
+       static uint8_t exp1[] = { 0x40, 0x24, 0x01, 0x03, 0x3e, 0x24, 0x46, 
0x68, 0x90, 0x87, 0xb0, 0x06,
                                  0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 
0x2b, 0x2b, 0x2b, 0x2b
        };

@@ -766,7 +766,7 @@

        msg1 = ul_tbf->create_ul_ack(*fn, ts_no);

-       static uint8_t exp2[] = { 0x40, 0x24, 0x01, 0x3f, 0x3e, 0x24, 0x46, 
0x68, 0x90, 0x88, 0xb0, 0x06, 0x8b,
+       static uint8_t exp2[] = { 0x40, 0x24, 0x01, 0x03, 0x3e, 0x24, 0x46, 
0x68, 0x90, 0x88, 0xb0, 0x06, 0x8b,
                                  0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 
0x2b, 0x2b, 0x2b
        };

diff --git a/tests/types/TypesTest.cpp b/tests/types/TypesTest.cpp
index 9c24ffe..4879424 100644
--- a/tests/types/TypesTest.cpp
+++ b/tests/types/TypesTest.cpp
@@ -497,7 +497,7 @@
                                 0x23, /* TA */
                                 0x00, /* 0-length §10.5.2.21 Mobile Allocation 
*/
                                 /* ETSI TS 44.018 §10.5.2.16 IA Rest Octets */
-                                0xc8, 0x02, 0x7b, 0xa0, 0x2b, 0x2b, 0x2b, 
0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };
+                                0xc8, 0x02, 0x1b, 0xa0, 0x2b, 0x2b, 0x2b, 
0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };

        check_imm_ass(tbf, false, GSM_L1_BURST_TYPE_ACCESS_0, res, sizeof(res), 
"ia_rest_uplink(MBA)");
 }
@@ -537,7 +537,7 @@
                                 0x23, /* TA */
                                 0x00, /* 0-length §10.5.2.21 Mobile Allocation 
*/
                                 /* ETSI TS 44.018 §10.5.2.16 IA Rest Octets */
-                                0x46, 0xa0, 0x09, 0xe0, 0x17, 0x40, 0x2b, 
0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };
+                                0x46, 0xa0, 0x08, 0x00, 0x17, 0x40, 0x2b, 
0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };

        check_imm_ass(tbf, false, GSM_L1_BURST_TYPE_ACCESS_1, res, sizeof(res), 
"ia_rest_egprs_uplink(SBA)");
 }
diff --git a/tests/types/TypesTest.ok b/tests/types/TypesTest.ok
index d4cc255..dd7dc4c 100644
--- a/tests/types/TypesTest.ok
+++ b/tests/types/TypesTest.ok
@@ -9,13 +9,13 @@
 [11] DL Immediate Assignment <ia_rest_downlink>:
        06 3f 30 0d 23 6d 7f 03 18 23 00 d0 00 00 00 08 17 47 08 0b 5b 2b 2b
 [11] UL Immediate Assignment <ia_rest_uplink(MBA)>:
-       06 3f 10 0d 23 6d 0d 03 18 23 00 c8 02 7b a0 2b 2b 2b 2b 2b 2b 2b 2b
+       06 3f 10 0d 23 6d 0d 03 18 23 00 c8 02 1b a0 2b 2b 2b 2b 2b 2b 2b 2b
 [11] UL Immediate Assignment <ia_rest_uplink(SBA)>:
        06 3f 10 0d 23 6d 0d 03 18 23 00 c5 d1 08 0b 5b 2b 2b 2b 2b 2b 2b 2b
 [11] UL Immediate Assignment <ia_rest_egprs_uplink(MBA)>:
        06 3f 10 0d 23 6d 7f 03 18 23 00 46 97 40 0b 58 2b 2b 2b 2b 2b 2b 2b
 [11] UL Immediate Assignment <ia_rest_egprs_uplink(SBA)>:
-       06 3f 10 0d 23 6d 7f 03 18 23 00 46 a0 09 e0 17 40 2b 2b 2b 2b 2b 2b
+       06 3f 10 0d 23 6d 7f 03 18 23 00 46 a0 08 00 17 40 2b 2b 2b 2b 2b 2b
 assignment reject: 06 3a 10 7f 06 36 14 7f 06 36 14 7f 06 36 14 7f 06 36 14 c0 
2b 2b
 assignment reject: 06 3a 10 70 06 36 14 70 06 36 14 70 06 36 14 70 06 36 14 0b 
2b 2b
 Testing LBS utility...

--
To view, visit https://gerrit.osmocom.org/13216
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I868062a81fffe6714a811c032215f25a79259905
Gerrit-Change-Number: 13216
Gerrit-PatchSet: 8
Gerrit-Owner: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
Gerrit-Reviewer: osmith <osm...@sysmocom.de>

Reply via email to