<p>Max <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/13216">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Harald Welte: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Fix Channel Coding Command for MCS<br><br>Previously result of ".to_num() - 1" was used without any checks which<br>means that in case of to_num() returning zero we would effectively try<br>to encode (uint8_t)(-1).<br><br>Let's fix this by using proper mcs_chan_code() function which returns<br>Channel Coding Command for MCS without the need to further correct it<br>and adjust expected tests output accordingly.<br><br>Change-Id: I868062a81fffe6714a811c032215f25a79259905<br>---<br>M src/encoding.cpp<br>M tests/tbf/TbfTest.cpp<br>M tests/types/TypesTest.cpp<br>M tests/types/TypesTest.ok<br>4 files changed, 12 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/encoding.cpp b/src/encoding.cpp</span><br><span>index 9920b46..b0d0467 100644</span><br><span>--- a/src/encoding.cpp</span><br><span>+++ b/src/encoding.cpp</span><br><span>@@ -268,7 +268,7 @@</span><br><span> bitvec_write_field(dest, &wp, usf, 3); // USF</span><br><span> bitvec_write_field(dest, &wp, 0, 1); // USF_GRANULARITY</span><br><span> bitvec_write_field(dest, &wp, 0, 1); // "0" power control: Not Present</span><br><span style="color: hsl(0, 100%, 40%);">- bitvec_write_field(dest, &wp, tbf->current_cs().to_num() - 1, 2); // CHANNEL_CODING_COMMAND</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 2); // CHANNEL_CODING_COMMAND</span><br><span> bitvec_write_field(dest, &wp, 1, 1); // TLLI_BLOCK_CHANNEL_CODING</span><br><span> if (alpha) {</span><br><span> bitvec_write_field(dest, &wp, 0x1, 1); // ALPHA = present</span><br><span>@@ -315,7 +315,7 @@</span><br><span> CHECK(rc);</span><br><span> </span><br><span> /* 3GPP TS 44.060 §12.10d EGPRS Modulation and coding Scheme description: */</span><br><span style="color: hsl(0, 100%, 40%);">- rc = bitvec_set_u64(dest, tbf->current_cs().to_num() - 1, 4, false); /* EGPRS CHANNEL_CODING_COMMAND */</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = bitvec_set_u64(dest, mcs_chan_code(tbf->current_cs()), 4, false); /* EGPRS CHANNEL_CODING_COMMAND */</span><br><span> CHECK(rc);</span><br><span> </span><br><span> /* TLLI_BLOCK_CHANNEL_CODING */</span><br><span>@@ -571,7 +571,7 @@</span><br><span> </span><br><span> if (!use_egprs) {</span><br><span> bitvec_write_field(dest, &wp,0x0,1); // Message escape</span><br><span style="color: hsl(0, 100%, 40%);">- bitvec_write_field(dest, &wp,tbf->current_cs().to_num()-1, 2); // CHANNEL_CODING_COMMAND</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 2); // CHANNEL_CODING_COMMAND</span><br><span> bitvec_write_field(dest, &wp,0x1,1); // TLLI_BLOCK_CHANNEL_CODING</span><br><span> write_ta_ie(dest, wp,tbf->ta(), ta_idx, ta_ts);</span><br><span> } else { /* EPGRS */</span><br><span>@@ -579,7 +579,7 @@</span><br><span> bitvec_write_field(dest, &wp,0x0,2); // EGPRS message contents</span><br><span> bitvec_write_field(dest, &wp,0x0,1); // No CONTENTION_RESOLUTION_TLLI</span><br><span> bitvec_write_field(dest, &wp,0x0,1); // No COMPACT reduced MA</span><br><span style="color: hsl(0, 100%, 40%);">- bitvec_write_field(dest, &wp,tbf->current_cs().to_num()-1, 4); // EGPRS Modulation and Coding IE</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 4); // EGPRS Modulation and Coding IE</span><br><span> /* 0: no RESEGMENT, 1: Segmentation*/</span><br><span> bitvec_write_field(dest, &wp, 0x1, 1);</span><br><span> write_ws(dest, &wp, tbf->window_size()); // EGPRS Window Size</span><br><span>@@ -809,7 +809,7 @@</span><br><span> struct gprs_rlcmac_ul_tbf *tbf, bool is_final)</span><br><span> {</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- bitvec_write_field(dest, &wp, tbf->current_cs().to_num() - 1, 2); // CHANNEL_CODING_COMMAND</span><br><span style="color: hsl(120, 100%, 40%);">+ bitvec_write_field(dest, &wp, mcs_chan_code(tbf->current_cs()), 2); // CHANNEL_CODING_COMMAND</span><br><span> write_packet_ack_nack_desc_gprs(bts, dest, wp, tbf->window(), is_final);</span><br><span> </span><br><span> bitvec_write_field(dest, &wp, 1, 1); // 1: have CONTENTION_RESOLUTION_TLLI</span><br><span>@@ -991,7 +991,7 @@</span><br><span> bitvec_write_field(dest, &wp, 0, 2); // fixed 00</span><br><span> /* CHANNEL_CODING_COMMAND */</span><br><span> bitvec_write_field(dest, &wp,</span><br><span style="color: hsl(0, 100%, 40%);">- tbf->current_cs().to_num() - 1, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+ mcs_chan_code(tbf->current_cs()), 4);</span><br><span> /* 0: no RESEGMENT, 1: Segmentation*/</span><br><span> bitvec_write_field(dest, &wp, 1, 1);</span><br><span> bitvec_write_field(dest, &wp, 1, 1); // PRE_EMPTIVE_TRANSMISSION, TODO: This resembles GPRS, change it?</span><br><span>diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp</span><br><span>index 6dc802c..151e7fa 100644</span><br><span>--- a/tests/tbf/TbfTest.cpp</span><br><span>+++ b/tests/tbf/TbfTest.cpp</span><br><span>@@ -739,7 +739,7 @@</span><br><span> </span><br><span> struct msgb *msg1 = ul_tbf->create_ul_ack(*fn, ts_no);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- static uint8_t exp1[] = { 0x40, 0x24, 0x01, 0x3f, 0x3e, 0x24, 0x46, 0x68, 0x90, 0x87, 0xb0, 0x06,</span><br><span style="color: hsl(120, 100%, 40%);">+ static uint8_t exp1[] = { 0x40, 0x24, 0x01, 0x03, 0x3e, 0x24, 0x46, 0x68, 0x90, 0x87, 0xb0, 0x06,</span><br><span> 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b</span><br><span> };</span><br><span> </span><br><span>@@ -766,7 +766,7 @@</span><br><span> </span><br><span> msg1 = ul_tbf->create_ul_ack(*fn, ts_no);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- static uint8_t exp2[] = { 0x40, 0x24, 0x01, 0x3f, 0x3e, 0x24, 0x46, 0x68, 0x90, 0x88, 0xb0, 0x06, 0x8b,</span><br><span style="color: hsl(120, 100%, 40%);">+ static uint8_t exp2[] = { 0x40, 0x24, 0x01, 0x03, 0x3e, 0x24, 0x46, 0x68, 0x90, 0x88, 0xb0, 0x06, 0x8b,</span><br><span> 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b</span><br><span> };</span><br><span> </span><br><span>diff --git a/tests/types/TypesTest.cpp b/tests/types/TypesTest.cpp</span><br><span>index 9c24ffe..4879424 100644</span><br><span>--- a/tests/types/TypesTest.cpp</span><br><span>+++ b/tests/types/TypesTest.cpp</span><br><span>@@ -497,7 +497,7 @@</span><br><span> 0x23, /* TA */</span><br><span> 0x00, /* 0-length §10.5.2.21 Mobile Allocation */</span><br><span> /* ETSI TS 44.018 §10.5.2.16 IA Rest Octets */</span><br><span style="color: hsl(0, 100%, 40%);">- 0xc8, 0x02, 0x7b, 0xa0, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };</span><br><span style="color: hsl(120, 100%, 40%);">+ 0xc8, 0x02, 0x1b, 0xa0, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };</span><br><span> </span><br><span> check_imm_ass(tbf, false, GSM_L1_BURST_TYPE_ACCESS_0, res, sizeof(res), "ia_rest_uplink(MBA)");</span><br><span> }</span><br><span>@@ -537,7 +537,7 @@</span><br><span> 0x23, /* TA */</span><br><span> 0x00, /* 0-length §10.5.2.21 Mobile Allocation */</span><br><span> /* ETSI TS 44.018 §10.5.2.16 IA Rest Octets */</span><br><span style="color: hsl(0, 100%, 40%);">- 0x46, 0xa0, 0x09, 0xe0, 0x17, 0x40, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };</span><br><span style="color: hsl(120, 100%, 40%);">+ 0x46, 0xa0, 0x08, 0x00, 0x17, 0x40, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, 0x2b, };</span><br><span> </span><br><span> check_imm_ass(tbf, false, GSM_L1_BURST_TYPE_ACCESS_1, res, sizeof(res), "ia_rest_egprs_uplink(SBA)");</span><br><span> }</span><br><span>diff --git a/tests/types/TypesTest.ok b/tests/types/TypesTest.ok</span><br><span>index d4cc255..dd7dc4c 100644</span><br><span>--- a/tests/types/TypesTest.ok</span><br><span>+++ b/tests/types/TypesTest.ok</span><br><span>@@ -9,13 +9,13 @@</span><br><span> [11] DL Immediate Assignment <ia_rest_downlink>:</span><br><span> 06 3f 30 0d 23 6d 7f 03 18 23 00 d0 00 00 00 08 17 47 08 0b 5b 2b 2b </span><br><span> [11] UL Immediate Assignment <ia_rest_uplink(MBA)>:</span><br><span style="color: hsl(0, 100%, 40%);">- 06 3f 10 0d 23 6d 0d 03 18 23 00 c8 02 7b a0 2b 2b 2b 2b 2b 2b 2b 2b </span><br><span style="color: hsl(120, 100%, 40%);">+ 06 3f 10 0d 23 6d 0d 03 18 23 00 c8 02 1b a0 2b 2b 2b 2b 2b 2b 2b 2b </span><br><span> [11] UL Immediate Assignment <ia_rest_uplink(SBA)>:</span><br><span> 06 3f 10 0d 23 6d 0d 03 18 23 00 c5 d1 08 0b 5b 2b 2b 2b 2b 2b 2b 2b </span><br><span> [11] UL Immediate Assignment <ia_rest_egprs_uplink(MBA)>:</span><br><span> 06 3f 10 0d 23 6d 7f 03 18 23 00 46 97 40 0b 58 2b 2b 2b 2b 2b 2b 2b </span><br><span> [11] UL Immediate Assignment <ia_rest_egprs_uplink(SBA)>:</span><br><span style="color: hsl(0, 100%, 40%);">- 06 3f 10 0d 23 6d 7f 03 18 23 00 46 a0 09 e0 17 40 2b 2b 2b 2b 2b 2b </span><br><span style="color: hsl(120, 100%, 40%);">+ 06 3f 10 0d 23 6d 7f 03 18 23 00 46 a0 08 00 17 40 2b 2b 2b 2b 2b 2b </span><br><span> assignment reject: 06 3a 10 7f 06 36 14 7f 06 36 14 7f 06 36 14 7f 06 36 14 c0 2b 2b </span><br><span> assignment reject: 06 3a 10 70 06 36 14 70 06 36 14 70 06 36 14 70 06 36 14 0b 2b 2b </span><br><span> Testing LBS utility...</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13216">change 13216</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13216"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I868062a81fffe6714a811c032215f25a79259905 </div>
<div style="display:none"> Gerrit-Change-Number: 13216 </div>
<div style="display:none"> Gerrit-PatchSet: 8 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>