GSOC 2014 LTE FFR Final code review

GSOC 2014 LTE FFR Final code review

The review covers all the changes from the beginning of the project, so my patch contains:
-FfrAlgorithm abstract class
-Two SAPs FfrSchedulerSap and FfrRrcSap
-Updated all schedulers and eNB RRC with new SAPs
-Updated LteHelper
-Downlink Power Control (3 modes: Open Loop, Closed Loop Absolute, Closed Loop Accumulative)
-Updated RrcAsn1Header with PdschConfigDedicated structure
-Implementation of new CQI generation approach
-Code refactoring of LteSinrChunkProcessor
-Connected FR algorithms with PF, PSS, CQA, FD-TBFQ, TD-TBFQ Schedulers
-Fixed HARQ bug
-Uplink Power Control
-Frequency Reuse Algorithms: 
	-NoOp FR algorithm
	-Hard FR algorithm
	-Strict FR algorithm
	-Soft FR algorithm
	-Soft FFR algorithm
	-Enhanced FFR algorithm
	-Distributed FFR algorithm
-Tests:
	-lte-downlink-power-control to check correctness of Downlink Power Control
	-lte-frequency-reuse to check if scheduler (PF, PSS, CQA, FD-TBFQ, TD-TBFQ) is not using forbidden RBGs, 
         and if UE is served with proper power and RBGs in downlink according to FFR algorithm policy
	-lte-interference-fr checks interference level with Hard and Strict FR algorithms
	-lte-cqi-generation check new CQI generation approach
	-lte-uplink-power-control to check correctness of Uplink Power Control in 3 modes
-Design and test documentation 
-Automatic Frequency Reuse configuration in DL and UL (cell can be configured with only 1 attribute)
-Fixed problem with uplink scheduling with frequency reuse algorithm
-Examples (in scratch directory) show how FR algorithms can be configured:
	-lena-fr-simple
	-lena-hard-fr
	-lena-strict-fr
	-lena-soft-fr
	-lena-soft-ffr
	-lena-enhanced-ffr
	-lena-distributed-ffr
	-lena-upc (Uplink Power Control)

There are 5 test suites of interest, which can be run as follows:
     	./test.py -s lte-downlink-power-control
     	./test.py -s lte-frequency-reuse
     	./test.py -s lte-interference-fr
     	./test.py -s lte-cqi-generation
     	./test.py -s lte-uplink-power-control

More details can be found in the project Wiki page [1] and documentation page [2]. 
Source code is available for download in project Mercurial repository [3]. 

Please feel free to contact me if you have any comments or questions. 
Thanks in advance for reviewing.

[1] http://www.nsnam.org/wiki/GSOC2014LteFFR
[2] http://code.nsnam.org/pgawlowicz/ns-3-dev-lte-ffr/
[3] http://www.nsnam.org/~pgawlowicz/lte.html

[Tom Henderson, 2014-08-03 17:14:54]

he patch is very large so I don't have time to review all at the moment.  I
started with the .rst documentation, examples, and testing the builds.  I
haven't reviewed the Doxygen or the model code itself yet.  (In general, it is
more helpful to reviewers to split patch into smaller chunks).

Can you please also include a patch to RELEASE_NOTES and CHANGES.html (somewhat
condensed version of what you posted as the review issue summary to this
Rietveld issue)?  Please call out in particular any API changes or changed
behavior.

The configuration of all of these modes, and parameters within the modes, seems
to be largely performed by the attribute system, which is good, but can you
somehow summarize or cross reference (to Doxygen) in your documentation of the
various FFR modes, how users can configure the usage and operation of the
different modes?  For example, how the attributes are listed here:
http://www.nsnam.org/docs/release/3.20/models/html/energy.html#attributes

Can you provide an example that summarizes the different performance tradeoffs 

There are some valgrind errors that you should be able to reproduce by running:

./waf configure --enable-examples --enable-tests --disable-gtk
./waf build
./test.py -g

errors are in most everything having to do with LTE

https://codereview.appspot.com/120850043/diff/1/scratch/lena-fr-simple.cc
File scratch/lena-fr-simple.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-fr-simple.cc#new...
scratch/lena-fr-simple.cc:18: * Author: Manuel Requena <manuel.requena@cttc.es>
Are all author strings and copyrights correct?

Please move all new files out of scratch to an appropriate examples directory.

This file (and all examples) need description of what they are trying to show,
what output is produced, what are the key input arguments, etc.

https://codereview.appspot.com/120850043/diff/1/scratch/lena-soft-ffr.cc
File scratch/lena-soft-ffr.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-soft-ffr.cc#newc...
scratch/lena-soft-ffr.cc:98: if(0){
need to clean this up (if (0) ...)

https://codereview.appspot.com/120850043/diff/1/scratch/lena-soft-fr.cc
File scratch/lena-soft-fr.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-soft-fr.cc#newco...
scratch/lena-soft-fr.cc:98: if(0){
need to clean this up

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
File src/lte/doc/source/lte-design.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:509: Frequency Reuse algorithms are applied
only to PDSCH and they can recuce inter-cell interferences only during PDSCH
duration. Since in legacy approach CQI feedback is generated from control
channels, it does not allow to use higher MCS for data channels and to achieve
any gain in throughput. Generation CQI feedback only from data channels would be
the best option. Unfortunately is impossible solution, because PDSCH is be sent
only if there is data to be sent, and CQI needs to be generated every TTI. Some
mixed approach was implemented. CQI are generated from control channels as
signal and data channels (if received) as interference. It there is no
transmission in data channel, CQI is generated only from control channels, as in
legacy solution.
s/recuce/reduce

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3540: algorithms. All implemented algorithms
are described in [ASHamza2013]_.
perhaps a brief bullet list of the implemented algorithms would help here.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3542: Figures FR data architecture and FR
control architecture show how FR
the above figure hyperlinks are broken

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3570: Since some of Frequecny Reuse Algorithms
require Downlink Power Control,
s/Frequecny/Frequency

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3609: uses entire system bandwidth and
transmit wit uniform power over all RBGs. It
s/wit/with

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3689: with signal quality treshold (this
parameter can be easily tuned by attribute
s/treshold/threshold

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
File src/lte/doc/source/lte-testing.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1419: mesumenents, it immediately call
function to change P_A value for this UE and also
s/mesumenets/measurements

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1432: First type of test cases check if RBGs
are used correct according to FR algorithm
s/correct/correctly

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1444: Second eNB is responsible for
generation interferences in whole system bandwidth.
s/generation/generating

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1451: Test vector compirse a configuration
for Strict FR, Soft FR and Soft FFR algorithms
s/compirse/comprise

[Marco Miozzo, 2014-08-06 10:51:15]

Hi Piotr,

Very good work. 
I have only minor comments.
I would just add to include the matlab/octave simulation scripts you used for
the test vectors of the tests.

Best regards,
marco.

https://codereview.appspot.com/120850043/diff/1/scratch/lena-distributed-ffr.cc
File scratch/lena-distributed-ffr.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-distributed-ffr....
scratch/lena-distributed-ffr.cc:140: lteHelper->SetSchedulerAttribute
("HarqEnabled",  BooleanValue (false));
I would leave HARQ enabled since it seems to me that we fixed the problem and it
is better to have all the standard functionalities of LTE active.

https://codereview.appspot.com/120850043/diff/1/scratch/lena-no-ffr.cc
File scratch/lena-no-ffr.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-no-ffr.cc#newcod...
scratch/lena-no-ffr.cc:126: lteHelper->SetSchedulerAttribute ("HarqEnabled", 
BooleanValue (false));
I would enable the HARQ as commented in another script

https://codereview.appspot.com/120850043/diff/1/scratch/lena-upc.cc
File scratch/lena-upc.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-upc.cc#newcode62
scratch/lena-upc.cc:62: 
Please comment a bit also on the target of this simulation script (I got "upc"
but it might be a bit enigmatic.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
File src/lte/doc/source/lte-design.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3557: scheduling process with FR algorithm. In
the beginning of schedulling
s/schedulling/scheduling

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3558: process, scheduler ask FR entity for
avaiable RBGs. According to
s/ask/asks

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3560: on its policy. Then when trying to
assign some RBG to UE, scheduler ask FR
s/ask/asks

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3626: In ns-3, the Trivial FR algorithm always
allow scheduler to use full bandwidth
s/Trivial/NoOp

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3639: of sub-bands. This scheme allow to
significanlty reduce ICI at the cell edge,
s/allow/allows

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3690: mechanism). Threshold have influence on
interior to cell radius ratio.
s/have/has

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3814: automatically optimize cell-edge
sub-bands by focusing on user distribution (in particular,
s/optimize/optimizes

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
File src/lte/doc/source/lte-testing.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1419: mesumenents, it immediately call
function to change P_A value for this UE and also
s/call/calls

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1420: trigger callback connected with this
event. Then, test check if UE gets
s/trigger/triggers

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1421: RrcConnectionReconfiguration message
(it trigger callback). Finally, we check if eNB
s/we check/it checks

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.rst
File src/lte/doc/source/lte-user.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.r...
src/lte/doc/source/lte-user.rst:1549: The each selected FR algorithm provide
several configurable attributes. User do
s/User/Users

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.r...
src/lte/doc/source/lte-user.rst:1549: The each selected FR algorithm provide
several configurable attributes. User do
s/do/does

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.r...
src/lte/doc/source/lte-user.rst:1603: sub-bands, within UEs are served with
different power level. Example below configure
s/configure/configures

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
File src/lte/model/lte-ffr-distributed-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:296: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:302: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
File src/lte/model/lte-ffr-enhanced-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:608: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:705: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:705: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:711: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:757: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-fr-soft-alg...
File src/lte/model/lte-fr-soft-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-fr-soft-alg...
src/lte/model/lte-fr-soft-algorithm.cc:366: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty, same
for the followings.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-fr-strict-a...
File src/lte/model/lte-fr-strict-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-fr-strict-a...
src/lte/model/lte-fr-strict-algorithm.cc:388: NS_LOG_FUNCTION (this);
I would add a NS_WARNING in case this function is not intentionally empty, same
for the following ones.

[Nicola Baldo, 2014-08-06 15:30:10]

Hi Piotr, 

overall, very good work!

The most important general comment that I have is that there are several points
(especially indentation in .cc files) where you do not comply with the ns-3
coding style. The easiest way to fix it is to run check-style on your
modifications to fix them automatically.

For the rest, please see my detailed comments below.
Note that I looked at the code only, not at the documentation which we have
already been discussing previously (except for the latest addition such as UPC
which I still have to look at).

Kind regards,

Nicola

https://codereview.appspot.com/120850043/diff/1/src/lte/helper/lte-helper.cc
File src/lte/helper/lte-helper.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/helper/lte-helper.cc#...
src/lte/helper/lte-helper.cc:65: //to be removed
please remove

https://codereview.appspot.com/120850043/diff/1/src/lte/helper/lte-helper.cc#...
src/lte/helper/lte-helper.cc:618: if(m_usePdschForCqiGeneration){
coding style

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
File src/lte/model/lte-chunk-processor.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
src/lte/model/lte-chunk-processor.cc:76:
std::vector<LteChunkProcessorCallback>::iterator it;
coding style for indentation

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
File src/lte/model/lte-chunk-processor.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
src/lte/model/lte-chunk-processor.h:41: * This abstract class is used to process
the time-vs-frequency SINR chunk
this is not limited to SINR any more, could be also interf power etc.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-cphy-sap.h
File src/lte/model/lte-enb-cphy-sap.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-cphy-sa...
src/lte/model/lte-enb-cphy-sap.h:88: virtual void SetP_A (uint16_t rnti, double
p_a) = 0;
coding style: "Otherwise, the underscore character should not be used in a
variable name." http://www.nsnam.org/developers/contributing-code/coding-style/

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.cc
File src/lte/model/lte-enb-mac.cc (left):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.cc#...
src/lte/model/lte-enb-mac.cc:444: 
can you get rid of this whitespace change?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.h
File src/lte/model/lte-enb-mac.h (left):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.h#o...
src/lte/model/lte-enb-mac.h:123: 
can you get rid of this whitespace change?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-net-dev...
File src/lte/model/lte-enb-net-device.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-net-dev...
src/lte/model/lte-enb-net-device.cc:77: "The FFR algorithm associated to this
EnbNetDevice",
please check indentation (not sure if it is codereview that does not show it
properly)

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-phy.cc
File src/lte/model/lte-enb-phy.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-phy.cc#...
src/lte/model/lte-enb-phy.cc:405: std::map<int, double> powerMap)
powerMap is unused, can you please remove it? Is the rest of the method
consistent?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc
File src/lte/model/lte-enb-rrc.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc#...
src/lte/model/lte-enb-rrc.cc:851: // call a new CphySap Method to set the
per-user transmission power offset
no need to point out that it's a "new CphySap method"... this is going to become
part of the stable release :-)

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc#...
src/lte/model/lte-enb-rrc.cc:1058: m_needTransmissionModeConfiguration = true;
please rename this variable to m_needPhyMacConfiguration - the current name made
sense only as far as Tx mode configuration was the only thing being configured.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc#...
src/lte/model/lte-enb-rrc.cc:2016: //  NS_ASSERT ("Processing of LOAD
INFORMATION X2 message IS NOT IMPLEMENTED");
kill dead code

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-algorit...
File src/lte/model/lte-ffr-algorithm.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-algorit...
src/lte/model/lte-ffr-algorithm.h:48: * "provider" part of this interface, while
the eNodeB RRC instance takes the
you mean MAC scheduler here?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
File src/lte/model/lte-ffr-distributed-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:63: "Downlink Transmission
SubBandwidth Configuration in number of Resource Block Groups",
the description does not seem to match the name of the attribute

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:249: //this will be removed after
connecting Uplink Power Control here
UPC is implemented now, right?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:314: if (measResults.measId ==
m_rsrqMeasId) {
please fix the coding style throughout this and the following methods. Consider
running check-style.py

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
File src/lte/model/lte-ffr-enhanced-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:85: #define NUM_DOWNLINK_CONFS
(sizeof (g_ffrEnhancedDownlinkDefaultConfiguration) / sizeof
(FfrEnhancedDownlinkDefaultConfiguration))
ok it is a matter of preference, but I'd replace these #define with a const.
Note that there are many occurrences in your code (other files as well).

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-soft-al...
File src/lte/model/lte-ffr-soft-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-soft-al...
src/lte/model/lte-ffr-soft-algorithm.cc:149: "If the RSRQ of is worse than this
threshold, UE should be served in Edge sub-band",
please specify the units for this attribute (and similar ones)

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-header.cc
File src/lte/model/lte-rrc-header.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-header....
src/lte/model/lte-rrc-header.cc:2114: std::bitset<0> bitset0;
coding style: use spaces, not tabs

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-sap.h
File src/lte/model/lte-rrc-sap.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-sap.h#n...
src/lte/model/lte-rrc-sap.h:138: /*
coding style: indentation

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-phy.h
File src/lte/model/lte-spectrum-phy.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-ph...
src/lte/model/lte-spectrum-phy.h:337: * \param p the new LteChunkProcessor to be
added to the RS power
doxygen seems a copy/paste from AddRsPowerChunkProcessor, can you please make it
appropriate for this method?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-si...
File src/lte/model/lte-spectrum-signal-parameters.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-si...
src/lte/model/lte-spectrum-signal-parameters.cc:88:
Ptr<LteSpectrumSignalParametersDataFrame> lssp (new
LteSpectrumSignalParametersDataFrame (*this), false);
please avoid including in the codereview a file that contains just whitespace
changes.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-si...
File src/lte/model/lte-spectrum-signal-parameters.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-si...
src/lte/model/lte-spectrum-signal-parameters.h:71: 
please avoid including in your code review a file which is just full of
whitespace changes

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-va...
File src/lte/model/lte-spectrum-value-helper.cc (left):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-va...
src/lte/model/lte-spectrum-value-helper.cc:55: * awk '{if ((NR % 7) == 1)
printf("{"); printf ("%s",$0); if ((NR % 7) == 0) printf("},\n"); else printf(",
");}' | sed 's/ – /, /g'
why this change?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ue-phy.cc
File src/lte/model/lte-ue-phy.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ue-phy.cc#n...
src/lte/model/lte-ue-phy.cc:443: SpectrumValue mixedSinr =
m_rsReceivedPower/m_dataInterferencePower;
coding style: indentation (here and below)

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ue-power-co...
File src/lte/model/lte-ue-power-control.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ue-power-co...
src/lte/model/lte-ue-power-control.h:32: 
doxygen completely missing for this class

[gawlowicz.p, 2014-08-08 22:44:17]

https://codereview.appspot.com/120850043/diff/1/scratch/lena-distributed-ffr.cc
File scratch/lena-distributed-ffr.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-distributed-ffr....
scratch/lena-distributed-ffr.cc:140: lteHelper->SetSchedulerAttribute
("HarqEnabled",  BooleanValue (false));
On 2014/08/06 10:51:14, marco.miozzo wrote:
> I would leave HARQ enabled since it seems to me that we fixed the problem and
it
> is better to have all the standard functionalities of LTE active.

Done.

https://codereview.appspot.com/120850043/diff/1/scratch/lena-fr-simple.cc
File scratch/lena-fr-simple.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-fr-simple.cc#new...
scratch/lena-fr-simple.cc:18: * Author: Manuel Requena <manuel.requena@cttc.es>
I will provide two examples and descriptions for them. First example will be
simple to show basic FR functionality. Second one will be based on
lena-dual-stripe.

On 2014/08/03 17:14:54, Tom Henderson wrote:
> Are all author strings and copyrights correct?
> 
> Please move all new files out of scratch to an appropriate examples directory.
> 
> This file (and all examples) need description of what they are trying to show,
> what output is produced, what are the key input arguments, etc.

https://codereview.appspot.com/120850043/diff/1/scratch/lena-no-ffr.cc
File scratch/lena-no-ffr.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-no-ffr.cc#newcod...
scratch/lena-no-ffr.cc:126: lteHelper->SetSchedulerAttribute ("HarqEnabled", 
BooleanValue (false));
On 2014/08/06 10:51:14, marco.miozzo wrote:
> I would enable the HARQ as commented in another script

Done.

https://codereview.appspot.com/120850043/diff/1/scratch/lena-upc.cc
File scratch/lena-upc.cc (right):

https://codereview.appspot.com/120850043/diff/1/scratch/lena-upc.cc#newcode62
scratch/lena-upc.cc:62: 
On 2014/08/06 10:51:14, marco.miozzo wrote:
> Please comment a bit also on the target of this simulation script (I got "upc"
> but it might be a bit enigmatic. 

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
File src/lte/doc/source/lte-design.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:509: Frequency Reuse algorithms are applied
only to PDSCH and they can recuce inter-cell interferences only during PDSCH
duration. Since in legacy approach CQI feedback is generated from control
channels, it does not allow to use higher MCS for data channels and to achieve
any gain in throughput. Generation CQI feedback only from data channels would be
the best option. Unfortunately is impossible solution, because PDSCH is be sent
only if there is data to be sent, and CQI needs to be generated every TTI. Some
mixed approach was implemented. CQI are generated from control channels as
signal and data channels (if received) as interference. It there is no
transmission in data channel, CQI is generated only from control channels, as in
legacy solution.
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/recuce/reduce

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3540: algorithms. All implemented algorithms
are described in [ASHamza2013]_.
On 2014/08/03 17:14:54, Tom Henderson wrote:
> perhaps a brief bullet list of the implemented algorithms would help here.

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3542: Figures FR data architecture and FR
control architecture show how FR
I removed this line, because I think there is no need to contain to figures very
similar to 
"LTE radio protocol stack architecture for the eNB on the data plane" and 
"LTE radio protocol stack architecture for the eNB on the control plane",
maybe this figures should be updated?

On 2014/08/03 17:14:54, Tom Henderson wrote:
> the above figure hyperlinks are broken

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3557: scheduling process with FR algorithm. In
the beginning of schedulling
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/schedulling/scheduling

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3558: process, scheduler ask FR entity for
avaiable RBGs. According to
On 2014/08/06 10:51:15, marco.miozzo wrote:
> s/ask/asks

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3560: on its policy. Then when trying to
assign some RBG to UE, scheduler ask FR
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/ask/asks

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3570: Since some of Frequecny Reuse Algorithms
require Downlink Power Control,
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/Frequecny/Frequency

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3609: uses entire system bandwidth and
transmit wit uniform power over all RBGs. It
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/wit/with

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3626: In ns-3, the Trivial FR algorithm always
allow scheduler to use full bandwidth
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/Trivial/NoOp

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3639: of sub-bands. This scheme allow to
significanlty reduce ICI at the cell edge,
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/allow/allows

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3689: with signal quality treshold (this
parameter can be easily tuned by attribute
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/treshold/threshold

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3690: mechanism). Threshold have influence on
interior to cell radius ratio.
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/have/has

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3814: automatically optimize cell-edge
sub-bands by focusing on user distribution (in particular,
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/optimize/optimizes

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
File src/lte/doc/source/lte-testing.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1419: mesumenents, it immediately call
function to change P_A value for this UE and also
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/mesumenets/measurements

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1419: mesumenents, it immediately call
function to change P_A value for this UE and also
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/call/calls

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1420: trigger callback connected with this
event. Then, test check if UE gets
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/trigger/triggers

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1420: trigger callback connected with this
event. Then, test check if UE gets
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/trigger/triggers

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1421: RrcConnectionReconfiguration message
(it trigger callback). Finally, we check if eNB
On 2014/08/06 10:51:14, marco.miozzo wrote:
> s/we check/it checks

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1432: First type of test cases check if RBGs
are used correct according to FR algorithm
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/correct/correctly

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1444: Second eNB is responsible for
generation interferences in whole system bandwidth.
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/generation/generating

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-testin...
src/lte/doc/source/lte-testing.rst:1451: Test vector compirse a configuration
for Strict FR, Soft FR and Soft FFR algorithms
On 2014/08/03 17:14:54, Tom Henderson wrote:
> s/compirse/comprise

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.rst
File src/lte/doc/source/lte-user.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.r...
src/lte/doc/source/lte-user.rst:1549: The each selected FR algorithm provide
several configurable attributes. User do
On 2014/08/06 10:51:15, marco.miozzo wrote:
> s/User/Users

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-user.r...
src/lte/doc/source/lte-user.rst:1603: sub-bands, within UEs are served with
different power level. Example below configure
On 2014/08/06 10:51:15, marco.miozzo wrote:
> s/configure/configures

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/helper/lte-helper.cc
File src/lte/helper/lte-helper.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/helper/lte-helper.cc#...
src/lte/helper/lte-helper.cc:65: //to be removed
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> please remove

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/helper/lte-helper.cc#...
src/lte/helper/lte-helper.cc:618: if(m_usePdschForCqiGeneration){
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> coding style

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
File src/lte/model/lte-chunk-processor.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
src/lte/model/lte-chunk-processor.cc:76:
std::vector<LteChunkProcessorCallback>::iterator it;
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> coding style for indentation

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
File src/lte/model/lte-chunk-processor.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-chunk-proce...
src/lte/model/lte-chunk-processor.h:41: * This abstract class is used to process
the time-vs-frequency SINR chunk
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> this is not limited to SINR any more, could be also interf power etc.

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-cphy-sap.h
File src/lte/model/lte-enb-cphy-sap.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-cphy-sa...
src/lte/model/lte-enb-cphy-sap.h:88: virtual void SetP_A (uint16_t rnti, double
p_a) = 0;
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> coding style: "Otherwise, the underscore character should not be used in a
> variable name."
http://www.nsnam.org/developers/contributing-code/coding-style/

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.cc
File src/lte/model/lte-enb-mac.cc (left):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.cc#...
src/lte/model/lte-enb-mac.cc:444: 
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> can you get rid of this whitespace change?

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.h
File src/lte/model/lte-enb-mac.h (left):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-mac.h#o...
src/lte/model/lte-enb-mac.h:123: 
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> can you get rid of this whitespace change?

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-net-dev...
File src/lte/model/lte-enb-net-device.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-net-dev...
src/lte/model/lte-enb-net-device.cc:77: "The FFR algorithm associated to this
EnbNetDevice",
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> please check indentation (not sure if it is codereview that does not show it
> properly)

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-phy.cc
File src/lte/model/lte-enb-phy.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-phy.cc#...
src/lte/model/lte-enb-phy.cc:405: std::map<int, double> powerMap)
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> powerMap is unused, can you please remove it? Is the rest of the method
> consistent?

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc
File src/lte/model/lte-enb-rrc.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc#...
src/lte/model/lte-enb-rrc.cc:851: // call a new CphySap Method to set the
per-user transmission power offset
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> no need to point out that it's a "new CphySap method"... this is going to
become
> part of the stable release :-)

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc#...
src/lte/model/lte-enb-rrc.cc:1058: m_needTransmissionModeConfiguration = true;
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> please rename this variable to m_needPhyMacConfiguration - the current name
made
> sense only as far as Tx mode configuration was the only thing being
configured.

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-enb-rrc.cc#...
src/lte/model/lte-enb-rrc.cc:2016: //  NS_ASSERT ("Processing of LOAD
INFORMATION X2 message IS NOT IMPLEMENTED");
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> kill dead code

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-algorit...
File src/lte/model/lte-ffr-algorithm.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-algorit...
src/lte/model/lte-ffr-algorithm.h:48: * "provider" part of this interface, while
the eNodeB RRC instance takes the
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> you mean MAC scheduler here?

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
File src/lte/model/lte-ffr-distributed-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:63: "Downlink Transmission
SubBandwidth Configuration in number of Resource Block Groups",
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> the description does not seem to match the name of the attribute

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:249: //this will be removed after
connecting Uplink Power Control here
Yes, in newest version this code is removed

On 2014/08/06 15:30:09, Nicola Baldo wrote:
> UPC is implemented now, right?

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:296: NS_LOG_FUNCTION (this);
I am thinking of removing this function, since it is not used in any FR
algorithm. Should I?
On 2014/08/06 10:51:13, marco.miozzo wrote:
> I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:302: NS_LOG_FUNCTION (this);
I am thinking of removing this function, since it is not used in any FR
algorithm. Should I?
On 2014/08/06 10:51:13, marco.miozzo wrote:
> I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:314: if (measResults.measId ==
m_rsrqMeasId) {
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> please fix the coding style throughout this and the following methods.
Consider
> running check-style.py

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
File src/lte/model/lte-ffr-enhanced-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:85: #define NUM_DOWNLINK_CONFS
(sizeof (g_ffrEnhancedDownlinkDefaultConfiguration) / sizeof
(FfrEnhancedDownlinkDefaultConfiguration))
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> ok it is a matter of preference, but I'd replace these #define with a const.
> Note that there are many occurrences in your code (other files as well).

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:705: NS_LOG_FUNCTION (this);
method is not used, should I remove it?
On 2014/08/06 10:51:13, marco.miozzo wrote:
> I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:711: NS_LOG_FUNCTION (this);
method is not used, should I remove it?
On 2014/08/06 10:51:13, marco.miozzo wrote:
> I would add a NS_WARNING in case this function is not intentionally empty

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-enhance...
src/lte/model/lte-ffr-enhanced-algorithm.cc:757: NS_LOG_FUNCTION (this);
On 2014/08/06 10:51:13, marco.miozzo wrote:
> I would add a NS_WARNING in case this function is not intentionally empty

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-soft-al...
File src/lte/model/lte-ffr-soft-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-soft-al...
src/lte/model/lte-ffr-soft-algorithm.cc:149: "If the RSRQ of is worse than this
threshold, UE should be served in Edge sub-band",
but RSRQ does not have units, it is reported value, quantized from dBm value
On 2014/08/06 15:30:09, Nicola Baldo wrote:
> please specify the units for this attribute (and similar ones)

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-header.cc
File src/lte/model/lte-rrc-header.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-header....
src/lte/model/lte-rrc-header.cc:2114: std::bitset<0> bitset0;
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> coding style: use spaces, not tabs

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-sap.h
File src/lte/model/lte-rrc-sap.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-rrc-sap.h#n...
src/lte/model/lte-rrc-sap.h:138: /*
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> coding style: indentation

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-phy.h
File src/lte/model/lte-spectrum-phy.h (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-ph...
src/lte/model/lte-spectrum-phy.h:337: * \param p the new LteChunkProcessor to be
added to the RS power
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> doxygen seems a copy/paste from AddRsPowerChunkProcessor, can you please make
it
> appropriate for this method?

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-va...
File src/lte/model/lte-spectrum-value-helper.cc (left):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-spectrum-va...
src/lte/model/lte-spectrum-value-helper.cc:55: * awk '{if ((NR % 7) == 1)
printf("{"); printf ("%s",$0); if ((NR % 7) == 0) printf("},\n"); else printf(",
");}' | sed 's/ – /, /g'
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> why this change?

Done.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ue-phy.cc
File src/lte/model/lte-ue-phy.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ue-phy.cc#n...
src/lte/model/lte-ue-phy.cc:443: SpectrumValue mixedSinr =
m_rsReceivedPower/m_dataInterferencePower;
On 2014/08/06 15:30:10, Nicola Baldo wrote:
> coding style: indentation (here and below)

Done.

[Nicola Baldo, 2014-08-18 13:02:07]

Hi all, 

please find some further comments inline below.

Nicola

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
File src/lte/doc/source/lte-design.rst (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/doc/source/lte-design...
src/lte/doc/source/lte-design.rst:3542: Figures FR data architecture and FR
control architecture show how FR
On 2014/08/08 22:44:16, gawlowicz.p wrote:
> I removed this line, because I think there is no need to contain to figures
very
> similar to 
> "LTE radio protocol stack architecture for the eNB on the data plane" and 
> "LTE radio protocol stack architecture for the eNB on the control plane",
> maybe this figures should be updated?

Yes please update these figures to include the FFR class and the new SAPs.

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
File src/lte/model/lte-ffr-distributed-algorithm.cc (right):

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:296: NS_LOG_FUNCTION (this);
On 2014/08/08 22:44:14, gawlowicz.p wrote:
> I am thinking of removing this function, since it is not used in any FR
> algorithm. Should I?

yes please

https://codereview.appspot.com/120850043/diff/1/src/lte/model/lte-ffr-distrib...
src/lte/model/lte-ffr-distributed-algorithm.cc:302: NS_LOG_FUNCTION (this);
On 2014/08/08 22:44:14, gawlowicz.p wrote:
> I am thinking of removing this function, since it is not used in any FR
> algorithm. Should I?

yes please