Change in osmo-pcu[master]: MCS: move Coding Scheme enum to C header

2019-02-26 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/13064 )

Change subject: MCS: move Coding Scheme enum to C header
..


Patch Set 1:

the code you're touching is undoubtedly C++ code.  I'm not sure if it's an 
improvement to turn [more] parts of it into C.  Also, I don't see from your 
changes or from the commitlog *why* this simplifies testing.  It's not clear 
that C++ can be tested less than C ?1?


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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I993b49d9a82b8c7ad677d52d11003794aeabe117
Gerrit-Change-Number: 13064
Gerrit-PatchSet: 1
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Harald Welte 
Gerrit-Comment-Date: Tue, 26 Feb 2019 22:41:43 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


Change in osmo-pcu[master]: MCS: move Coding Scheme enum to C header

2019-02-26 Thread Max
Max has uploaded this change for review. ( https://gerrit.osmocom.org/13064


Change subject: MCS: move Coding Scheme enum to C header
..

MCS: move Coding Scheme enum to C header

Move generic MCS enum to C header file to simplify further modifications
and testing.

Change-Id: I993b49d9a82b8c7ad677d52d11003794aeabe117
---
M src/Makefile.am
A src/coding_scheme.h
M src/gprs_coding_scheme.cpp
M src/gprs_coding_scheme.h
M src/gprs_ms.cpp
M src/gprs_ms.h
M src/rlc.cpp
M src/tbf_dl.cpp
M src/tbf_ul.cpp
M tests/edge/EdgeTest.cpp
M tests/tbf/TbfTest.cpp
11 files changed, 247 insertions(+), 230 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/64/13064/1

diff --git a/src/Makefile.am b/src/Makefile.am
index 50e0eda..eb4a2bb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -97,6 +97,7 @@
cxx_linuxlist.h \
gprs_codel.h \
gprs_coding_scheme.h \
+   coding_scheme.h \
egprs_rlc_compression.h
 
 osmo_pcu_SOURCES = pcu_main.cpp
diff --git a/src/coding_scheme.h b/src/coding_scheme.h
new file mode 100644
index 000..3705ea4
--- /dev/null
+++ b/src/coding_scheme.h
@@ -0,0 +1,29 @@
+/* coding_scheme.h
+ *
+ * Copyright (C) 2015-2019 by sysmocom s.f.m.c. GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#pragma once
+
+enum CodingScheme {
+   UNKNOWN,
+   /* GPRS Coding Schemes: */
+   CS1, CS2, CS3, CS4,
+   /* EDGE/EGPRS Modulation and Coding Schemes: */
+   MCS1, MCS2, MCS3, MCS4, MCS5, MCS6, MCS7, MCS8, MCS9,
+   NUM_SCHEMES
+};
diff --git a/src/gprs_coding_scheme.cpp b/src/gprs_coding_scheme.cpp
index 3094ae6..3769363 100644
--- a/src/gprs_coding_scheme.cpp
+++ b/src/gprs_coding_scheme.cpp
@@ -27,7 +27,7 @@
  * 1st level is Original MCS( index 0 corresponds to MCS1 and so on)
  * 2nd level is MS MCS (index 0 corresponds to MCS1 and so on)
  */
-enum GprsCodingScheme::Scheme GprsCodingScheme::egprs_mcs_retx_tbl[MAX_NUM_ARQ]
+enum CodingScheme GprsCodingScheme::egprs_mcs_retx_tbl[MAX_NUM_ARQ]
[MAX_NUM_MCS][MAX_NUM_MCS] = {
{
{MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1, MCS1},
@@ -64,7 +64,7 @@
const char *name;
GprsCodingScheme::HeaderType data_hdr;
GprsCodingScheme::Family family;
-} mcs_info[GprsCodingScheme::NUM_SCHEMES] = {
+} mcs_info[NUM_SCHEMES] = {
{{0, 0},   {0, 0},0,  0, "UNKNOWN",
GprsCodingScheme::HEADER_INVALID, 
GprsCodingScheme::FAMILY_INVALID},
{{23, 0},  {23, 0},  20,  0, "CS-1",
@@ -231,7 +231,7 @@
/* This should not happen. TODO: Use assert? */
return;

-   Scheme new_cs(Scheme(m_scheme + 1));
+   CodingScheme new_cs(CodingScheme(m_scheme + 1));
if (!GprsCodingScheme(new_cs).isCompatible(mode))
/* Clipping, do not change the value */
return;
@@ -245,7 +245,7 @@
/* This should not happen. TODO: Use assert? */
return;

-   Scheme new_cs(Scheme(m_scheme - 1));
+   CodingScheme new_cs(CodingScheme(m_scheme - 1));
if (!GprsCodingScheme(new_cs).isCompatible(mode))
/* Clipping, do not change the value */
return;
@@ -264,7 +264,7 @@
if (!isValid())
return;

-   m_scheme = Scheme(m_scheme + 1);
+   m_scheme = CodingScheme(m_scheme + 1);
 }

 void GprsCodingScheme::dec()
@@ -278,7 +278,7 @@
if (!isValid())
return;

-   m_scheme = Scheme(m_scheme - 1);
+   m_scheme = CodingScheme(m_scheme - 1);
 }

 const char *GprsCodingScheme::modeName(Mode mode)
diff --git a/src/gprs_coding_scheme.h b/src/gprs_coding_scheme.h
index ee30c8a..1121d13 100644
--- a/src/gprs_coding_scheme.h
+++ b/src/gprs_coding_scheme.h
@@ -25,6 +25,7 @@

 extern "C" {
#include 
+   #include "coding_scheme.h"
 }

 class GprsCodingScheme {
@@ -35,14 +36,6 @@
 #define EGPRS_ARQ10x0
 #define EGPRS_ARQ20x1

-   enum Scheme {
-   UNKNOWN,
-   CS1, CS2, CS3, CS4,
-   MCS1, MCS2, MCS3, MCS4,
-   MCS5, MCS6, MCS7, MCS8, MCS9,
-   NUM_SCHEMES
-   };
-
enum