osmo-trx[master]: osmo-trx: Separate command line switch to enable EDGE filler.

[Tom Tsou]

Patch Set 5:

I would very much like to replace the command line and control socket 
interface. There are more than enough configuration options so that a config 
file makes sense - that is the direction that I prefer over command line 
arguments.

For the control socket interface, we need to consider backwards compatibility 
with OpenBTS.

-- 
To view, visit https://gerrit.osmocom.org/2143
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
Gerrit-PatchSet: 5
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Alexander Chemeris 
Gerrit-Reviewer: Alexander Chemeris 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: osmo-trx: Separate command line switch to enable EDGE filler.

[Alexander Chemeris]

Patch Set 2:

> Maybe we can replace both -r and -E with new --filler which will
 > figure out appropriate filler based on other options by checking if
 > edge is enabled or not?

That's exactly what this patch is moving away from. Right now -r option has 
different behavior based on whether -e is specified or not, which means that I 
can't generate random GMSK burst if I run in EDGE mode, which is very 
inconvenient.

What we can do is to make this a control interface command rather than a 
command line parameter. It'll make it harder to use, but more clean from the 
code perspective.

-- 
To view, visit https://gerrit.osmocom.org/2143
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Alexander Chemeris 
Gerrit-Reviewer: Alexander Chemeris 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: osmo-trx: Separate command line switch to enable EDGE filler.

[Max]

Patch Set 2:

Maybe we can replace both -r and -E with new --filler which will figure out 
appropriate filler based on other options by checking if edge is enabled or 
not? In general, there's GNU gengetopt which supports mutually exclusive groups 
of options but it's only used for OpenGGSN so far.

-- 
To view, visit https://gerrit.osmocom.org/2143
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Alexander Chemeris 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: No

osmo-trx[master]: osmo-trx: Separate command line switch to enable EDGE filler.

[Tom Tsou]

Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/2143/2/Transceiver52M/osmo-trx.cpp
File Transceiver52M/osmo-trx.cpp:

Line 419:   case 'E':
> So user can still specify ... -E 123 -r 321 ... without enabling EDGE and i
In general, I would very much like to stop adding more and more command line 
arguments. There are already other ambiguous cases similar to that mentioned by 
Max and most users never use them.

I'm open to ideas (e.g. config file, string-value arguments?).


-- 
To view, visit https://gerrit.osmocom.org/2143
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Alexander Chemeris 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: Yes

osmo-trx[master]: osmo-trx: Separate command line switch to enable EDGE filler.

[Max]

Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/#/c/2143/2/Transceiver52M/osmo-trx.cpp
File Transceiver52M/osmo-trx.cpp:

Line 419:   case 'E':
So user can still specify ... -E 123 -r 321 ... without enabling EDGE and it'll 
work (provided options are parsed from left to right) because "-r" will shadow 
"-E", but ... -r 123 -E 321 ... will fail.

I think it's better to make "-E" and "-r" mutually exclusive - unless there's 
use-case when we actually need both.


-- 
To view, visit https://gerrit.osmocom.org/2143
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
Gerrit-PatchSet: 2
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Alexander Chemeris 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max 
Gerrit-Reviewer: Tom Tsou 
Gerrit-HasComments: Yes

[PATCH] osmo-trx[master]: osmo-trx: Separate command line switch to enable EDGE filler.

[Alexander Chemeris]

Review at  https://gerrit.osmocom.org/2143

osmo-trx: Separate command line switch to enable EDGE filler.

Now -r comand line switch always enables GMSK filler even when EDGE mode is
enabled with -e switch. If you want to enable EDGE filler, use -E switch.

Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
---
M Transceiver52M/osmo-trx.cpp
1 file changed, 11 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/43/2143/1

diff --git a/Transceiver52M/osmo-trx.cpp b/Transceiver52M/osmo-trx.cpp
index 5e81586..dd80557 100644
--- a/Transceiver52M/osmo-trx.cpp
+++ b/Transceiver52M/osmo-trx.cpp
@@ -339,7 +339,8 @@
"  -cNumber of ARFCN channels (default=1)\n"
"  -fEnable C0 filler table\n"
"  -oSet baseband frequency offset (default=auto)\n"
-   "  -rRandom Normal Burst test mode with TSC\n"
+   "  -rRandom GMSK Normal Burst test mode with given TSC\n"
+   "  -ERandom 8-PSK Normal Burst test mode with given TSC\n"
"  -ARandom Access Burst test mode with delay\n"
"  -RRSSI to dBm offset in dB (default=0)\n"
"  -SSwap channels (UmTRX only)\n",
@@ -366,7 +367,7 @@
config->swap_channels = false;
config->edge = false;
 
-   while ((option = getopt(argc, argv, "ha:l:i:p:c:dmxgfo:s:b:r:A:R:Se")) 
!= -1) {
+   while ((option = getopt(argc, argv, 
"ha:l:i:p:c:dmxgfo:s:b:r:E:A:R:Se")) != -1) {
switch (option) {
case 'h':
print_help();
@@ -414,6 +415,10 @@
case 'r':
config->rtsc = atoi(optarg);
config->filler = Transceiver::FILLER_NORM_RAND;
+   break;
+   case 'E':
+   config->rtsc = atoi(optarg);
+   config->filler = Transceiver::FILLER_EDGE_RAND;
break;
case 'A':
config->rach_delay = atoi(optarg);
@@ -463,8 +468,10 @@
}
}
 
-   if (config->edge && (config->filler == Transceiver::FILLER_NORM_RAND))
-   config->filler = Transceiver::FILLER_EDGE_RAND;
+   if (!config->edge && (config->filler == Transceiver::FILLER_EDGE_RAND)) 
{
+   printf("Can't enable EDGE filler when EDGE mode is 
disabled\n\n");
+   goto bad_config;
+   }
 
if ((config->tx_sps != 1) && (config->tx_sps != 4) &&
(config->rx_sps != 1) && (config->rx_sps != 4)) {

-- 
To view, visit https://gerrit.osmocom.org/2143
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic8808bbe3f06740ef3fec1d1865ecb57fbcfabab
Gerrit-PatchSet: 1
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Owner: Alexander Chemeris