Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(69)

Issue 1866042: LTE module - Giuseppe Piro

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by GiuseppePiro
Modified:
11 years, 1 month ago
Reviewers:
Nicola Baldo
CC:
ns-3-reviews_googlegroups.com, g.piro_poliba.it, Nicola Baldo, mmiozzo_cttc.es
Visibility:
Public.

Patch Set 1 #

Total comments: 54

Patch Set 2 : LTE module #

Total comments: 18
Unified diffs Side-by-side diffs Delta from patch set Stats (+14475 lines, -9 lines) Patch
A examples/lte/lte-amc.cc View 1 chunk +219 lines, -0 lines 0 comments Download
A examples/lte/lte-channel-model.cc View 1 chunk +189 lines, -0 lines 0 comments Download
A examples/lte/lte-device.cc View 1 1 chunk +163 lines, -0 lines 0 comments Download
A examples/lte/lte-phy-downlink.cc View 1 1 chunk +176 lines, -0 lines 0 comments Download
A examples/lte/lte-phy-uplink.cc View 1 1 chunk +172 lines, -0 lines 0 comments Download
A examples/lte/waf View 1 chunk +1 line, -0 lines 0 comments Download
A examples/lte/wscript View 1 1 chunk +23 lines, -0 lines 0 comments Download
M examples/spectrum/waf View 0 chunks +-1 lines, --1 lines 0 comments Download
A src/devices/lte/.lte-propagation-loss-model.cc.swp View Binary file 0 comments Download
A src/devices/lte/AUTHORS View 1 chunk +9 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v0_M10.h View 1 chunk +34 lines, -0 lines 0 comments Download
src/devices/lte/JakesTraces/multipath_v0_M12.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v0_M6.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v0_M8.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v120_M10.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v120_M12.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v120_M6.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v120_M8.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v30_M10.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v30_M12.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v30_M6.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v30_M8.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v3_M10.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v3_M12.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v3_M6.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/JakesTraces/multipath_v3_M8.h View 1 chunk +33 lines, -0 lines 0 comments Download
A src/devices/lte/amc-module.h View 1 1 chunk +91 lines, -0 lines 0 comments Download
A src/devices/lte/amc-module.cc View 1 1 chunk +198 lines, -0 lines 0 comments Download
A src/devices/lte/bearer.h View 1 chunk +223 lines, -0 lines 0 comments Download
A src/devices/lte/bearer.cc View 1 chunk +232 lines, -0 lines 0 comments Download
A src/devices/lte/bearer-manager.h View 1 chunk +110 lines, -0 lines 0 comments Download
A src/devices/lte/bearer-manager.cc View 1 chunk +103 lines, -0 lines 0 comments Download
A src/devices/lte/bearer-qos-parameters.h View 1 chunk +183 lines, -0 lines 0 comments Download
A src/devices/lte/bearer-qos-parameters.cc View 1 chunk +170 lines, -0 lines 0 comments Download
A src/devices/lte/channel-realization.h View 1 1 chunk +77 lines, -0 lines 0 comments Download
A src/devices/lte/channel-realization.cc View 1 chunk +122 lines, -0 lines 0 comments Download
A src/devices/lte/enb-lte-spectrum-phy.h View 1 1 chunk +56 lines, -0 lines 0 comments Download
A src/devices/lte/enb-lte-spectrum-phy.cc View 1 1 chunk +95 lines, -0 lines 0 comments Download
A src/devices/lte/enb-mac-entity.h View 1 1 chunk +82 lines, -0 lines 0 comments Download
A src/devices/lte/enb-mac-entity.cc View 1 1 chunk +144 lines, -0 lines 0 comments Download
A src/devices/lte/enb-net-device.h View 1 1 chunk +134 lines, -0 lines 0 comments Download
A src/devices/lte/enb-net-device.cc View 1 1 chunk +214 lines, -0 lines 2 comments Download
A src/devices/lte/enb-phy.h View 1 1 chunk +84 lines, -0 lines 0 comments Download
A src/devices/lte/enb-phy.cc View 1 1 chunk +208 lines, -0 lines 0 comments Download
A src/devices/lte/ideal-control-messages.h View 1 1 chunk +212 lines, -0 lines 0 comments Download
A src/devices/lte/ideal-control-messages.cc View 1 1 chunk +188 lines, -0 lines 0 comments Download
A src/devices/lte/ip-classifier.h View 1 1 chunk +63 lines, -0 lines 0 comments Download
A src/devices/lte/ip-classifier.cc View 1 1 chunk +83 lines, -0 lines 2 comments Download
A src/devices/lte/ip-classifier-record.h View 1 chunk +131 lines, -0 lines 0 comments Download
A src/devices/lte/ip-classifier-record.cc View 1 chunk +205 lines, -0 lines 0 comments Download
A src/devices/lte/loss-model.h View 1 chunk +94 lines, -0 lines 0 comments Download
A src/devices/lte/loss-model.cc View 1 chunk +100 lines, -0 lines 0 comments Download
A src/devices/lte/lte-device-test.cc View 1 1 chunk +200 lines, -0 lines 0 comments Download
A src/devices/lte/lte-mac-header.h View 1 chunk +55 lines, -0 lines 0 comments Download
A src/devices/lte/lte-mac-header.cc View 1 chunk +109 lines, -0 lines 0 comments Download
A src/devices/lte/lte-mac-queue.h View 1 chunk +144 lines, -0 lines 0 comments Download
A src/devices/lte/lte-mac-queue.cc View 1 chunk +247 lines, -0 lines 0 comments Download
A src/devices/lte/lte-net-device.h View 1 1 chunk +228 lines, -0 lines 0 comments Download
A src/devices/lte/lte-net-device.cc View 1 1 chunk +482 lines, -0 lines 0 comments Download
A src/devices/lte/lte-phy.h View 1 1 chunk +225 lines, -0 lines 0 comments Download
A src/devices/lte/lte-phy.cc View 1 1 chunk +264 lines, -0 lines 0 comments Download
A src/devices/lte/lte-phy-test.cc View 1 chunk +189 lines, -0 lines 0 comments Download
A src/devices/lte/lte-propagation-loss-model.h View 1 chunk +79 lines, -0 lines 0 comments Download
A src/devices/lte/lte-propagation-loss-model.cc View 1 1 chunk +157 lines, -0 lines 0 comments Download
A src/devices/lte/lte-propagation-loss-model-test.cc View 1 chunk +229 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-error-model.h View 1 chunk +74 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-error-model.cc View 1 chunk +72 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-model.h View 1 chunk +34 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-model.cc View 1 chunk +54 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-phy.h View 1 1 chunk +206 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-phy.cc View 1 1 chunk +500 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-value-helper.h View 1 1 chunk +59 lines, -0 lines 0 comments Download
A src/devices/lte/lte-spectrum-value-helper.cc View 1 chunk +87 lines, -0 lines 4 comments Download
A src/devices/lte/mac-entity.h View 1 chunk +69 lines, -0 lines 0 comments Download
A src/devices/lte/mac-entity.cc View 1 chunk +89 lines, -0 lines 0 comments Download
A src/devices/lte/multipath.h View 1 chunk +47 lines, -0 lines 0 comments Download
A src/devices/lte/multipath-loss-model.h View 1 1 chunk +81 lines, -0 lines 0 comments Download
A src/devices/lte/multipath-loss-model.cc View 1 1 chunk +314 lines, -0 lines 0 comments Download
A src/devices/lte/packet-scheduler.h View 1 1 chunk +76 lines, -0 lines 0 comments Download
A src/devices/lte/packet-scheduler.cc View 1 1 chunk +112 lines, -0 lines 0 comments Download
A src/devices/lte/path-loss-model.h View 1 chunk +69 lines, -0 lines 0 comments Download
A src/devices/lte/path-loss-model.cc View 1 1 chunk +86 lines, -0 lines 0 comments Download
A src/devices/lte/penetration-loss-model.h View 1 chunk +63 lines, -0 lines 0 comments Download
A src/devices/lte/penetration-loss-model.cc View 1 chunk +74 lines, -0 lines 0 comments Download
A src/devices/lte/qos-parameters.h View 1 1 chunk +164 lines, -0 lines 0 comments Download
A src/devices/lte/qos-parameters.cc View 1 chunk +156 lines, -0 lines 0 comments Download
A src/devices/lte/radio-bearer-instance.h View 1 chunk +226 lines, -0 lines 4 comments Download
A src/devices/lte/radio-bearer-instance.cc View 1 chunk +233 lines, -0 lines 0 comments Download
A src/devices/lte/rlc-entity.h View 1 chunk +78 lines, -0 lines 4 comments Download
A src/devices/lte/rlc-entity.cc View 1 chunk +70 lines, -0 lines 0 comments Download
A src/devices/lte/rrc-entity.h View 1 1 chunk +114 lines, -0 lines 0 comments Download
A src/devices/lte/rrc-entity.cc View 1 1 chunk +119 lines, -0 lines 2 comments Download
A src/devices/lte/shadowing-loss-model.h View 1 chunk +72 lines, -0 lines 0 comments Download
A src/devices/lte/shadowing-loss-model.cc View 1 chunk +89 lines, -0 lines 0 comments Download
A src/devices/lte/simple-packet-scheduler.h View 1 chunk +58 lines, -0 lines 0 comments Download
A src/devices/lte/simple-packet-scheduler.cc View 1 chunk +147 lines, -0 lines 0 comments Download
A src/devices/lte/ue-lte-spectrum-phy.h View 1 1 chunk +56 lines, -0 lines 0 comments Download
A src/devices/lte/ue-lte-spectrum-phy.cc View 1 1 chunk +139 lines, -0 lines 0 comments Download
A src/devices/lte/ue-mac-entity.h View 1 chunk +50 lines, -0 lines 0 comments Download
A src/devices/lte/ue-mac-entity.cc View 1 chunk +55 lines, -0 lines 0 comments Download
A src/devices/lte/ue-manager.h View 1 1 chunk +100 lines, -0 lines 0 comments Download
A src/devices/lte/ue-manager.cc View 1 1 chunk +174 lines, -0 lines 0 comments Download
A src/devices/lte/ue-net-device.h View 1 1 chunk +127 lines, -0 lines 0 comments Download
A src/devices/lte/ue-net-device.cc View 1 1 chunk +226 lines, -0 lines 0 comments Download
A src/devices/lte/ue-phy.h View 1 1 chunk +78 lines, -0 lines 0 comments Download
A src/devices/lte/ue-phy.cc View 1 1 chunk +242 lines, -0 lines 0 comments Download
A src/devices/lte/ue-record.h View 1 chunk +114 lines, -0 lines 0 comments Download
A src/devices/lte/ue-record.cc View 1 chunk +96 lines, -0 lines 0 comments Download
A src/devices/lte/wscript View 1 1 chunk +87 lines, -0 lines 0 comments Download
M src/devices/spectrum/multi-model-spectrum-channel.h View 1 chunk +2 lines, -0 lines 0 comments Download
M src/devices/spectrum/multi-model-spectrum-channel.cc View 1 chunk +7 lines, -0 lines 0 comments Download
M src/devices/spectrum/single-model-spectrum-channel.h View 2 chunks +1 line, -1 line 0 comments Download
M src/devices/spectrum/single-model-spectrum-channel.cc View 1 chunk +6 lines, -6 lines 0 comments Download
M src/devices/spectrum/spectrum-interference.h View 2 chunks +1 line, -1 line 0 comments Download
M src/devices/spectrum/spectrum-interference.cc View 1 chunk +8 lines, -0 lines 0 comments Download
A src/devices/wifi/spectrum-yans-wifi-phy.h View 1 chunk +166 lines, -0 lines 0 comments Download
A src/devices/wifi/spectrum-yans-wifi-phy.cc View 1 chunk +615 lines, -0 lines 0 comments Download
A src/devices/wifi/wifi-phy-tag.h View 1 chunk +60 lines, -0 lines 0 comments Download
A src/devices/wifi/wifi-phy-tag.cc View 1 chunk +100 lines, -0 lines 0 comments Download
A src/devices/wifi/wifi-spectrum-phy-interface.h View 1 chunk +52 lines, -0 lines 0 comments Download
A src/devices/wifi/wifi-spectrum-phy-interface.cc View 1 chunk +70 lines, -0 lines 0 comments Download
A src/helper/lte-helper.h View 1 chunk +132 lines, -0 lines 0 comments Download
A src/helper/lte-helper.cc View 1 1 chunk +323 lines, -0 lines 0 comments Download
M src/helper/wscript View 2 chunks +2 lines, -0 lines 0 comments Download
M src/node/spectrum-channel.h View 2 chunks +3 lines, -2 lines 0 comments Download
M src/wscript View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 5
Nicola Baldo
Hi Giuseppe, I went through most of the code that you uploaded for review. There ...
11 years, 2 months ago (2010-07-20 12:41:41 UTC) #1
GiuseppePiro
Dear Nicola, thank you for your review. Hope I have responded correctly to all your ...
11 years, 2 months ago (2010-07-22 16:17:11 UTC) #2
Nicola Baldo
Good work, most of the previous comments have been addressed. The most important issue which ...
11 years, 1 month ago (2010-08-04 12:40:38 UTC) #3
GiuseppePiro
Hi Nicola, thanks for your review. Hope that all problems have been fixed well! Regards ...
11 years, 1 month ago (2010-08-06 14:09:47 UTC) #4
g.piro_poliba.it
11 years, 1 month ago (2010-08-07 10:08:04 UTC) #5
The code has been moved to http://codereview.appspot.com/1869054/show

for final review.

Regards,

G.
-- 
Giuseppe Piro
Ph.D. Student
DEE - Politecnico di Bari
v. Orabona 4, 70125 - Bari, Italy
e-mail:    g.piro@poliba.it
Phone:   +39 0805963301
Web:       http://telematics.poliba.it/piro



Citando peppe.piro2@gmail.com:

> Hi Nicola, thanks for your review.
> Hope that all problems have been fixed well!
>
> Regards
>
> Giuseppe Piro
>
>
> http://codereview.appspot.com/1866042/diff/17001/18041
> File src/devices/lte/enb-net-device.cc (right):
>
> http://codereview.appspot.com/1866042/diff/17001/18041#newcode174
> src/devices/lte/enb-net-device.cc:174: return Enqueue (packet, bearer);
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> why don't you just do
>> bearer->Enqueue (packet)
>> and delete the method LteNetDevice::Enqueue (bearer, packet)?
>
> Done.
>
> http://codereview.appspot.com/1866042/diff/17001/18049
> File src/devices/lte/ip-classifier.cc (right):
>
> http://codereview.appspot.com/1866042/diff/17001/18049#newcode80
> src/devices/lte/ip-classifier.cc:80: return GetDevice ()->GetRrcEntity
> ()->GetDefaultBearer ();
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> I understand that implementing dedicated bearers is beyond scope of
> the GSoC,
>> and that for this reason it is good to stick with default bearers
> only. However,
>> I think it is important to be noted that there is one default bearer
> per UE,
>> i.e., the RRC at the eNB should not have a single default bearer, but
> rather a
>> separate default bearer for every connected UE.
>
>
> In order to implement one dedicated radio bearer for each UE it is
> necessary do complete the development of the ip-classifier, the
> ue-manager and so on. For this reason, this feature will be implemented
> after GSoC.
>
> http://codereview.appspot.com/1866042/diff/17001/18072
> File src/devices/lte/lte-spectrum-value-helper.cc (right):
>
> http://codereview.appspot.com/1866042/diff/17001/18072#newcode27
> src/devices/lte/lte-spectrum-value-helper.cc:27: Ptr<SpectrumModel>
> LteSpectrumModel;
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> There is still a single LteSpectrumModel used both for DL and UL. For
> FDD, there
>> should be two separates FddLteSpectrumModel, one for UL and one for
> DL. It is
>> only for TDD that the spectrum is the same for UL and DL.
>
> Done.
>
> http://codereview.appspot.com/1866042/diff/17001/18072#newcode42
> src/devices/lte/lte-spectrum-value-helper.cc:42: freqs.push_back
> (centralFrequencyOfPRB * 1e9);
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> You have exactly the same code in lte-spectrum-model.cc, please remove
> one of
>> these instances
>
> Done.
>
> http://codereview.appspot.com/1866042/diff/17001/18088
> File src/devices/lte/radio-bearer-instance.h (right):
>
> http://codereview.appspot.com/1866042/diff/17001/18088#newcode202
> src/devices/lte/radio-bearer-instance.h:202: Ptr<RlcEntity> GetRlcEntiry
> (void);
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> Entiry --> Entity
>
> Done.
>
> http://codereview.appspot.com/1866042/diff/17001/18088#newcode218
> src/devices/lte/radio-bearer-instance.h:218: Ptr<IpClassifierRecord>
> m_classifierRecord;
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> Why does the RlcEntity need to know the corresponding
> IpClassifierRecord? I
>> thought that the mapping between the two was to be managed by the
> IpClassifier
>> class only.
>
> Ok, it has been fixed.
>
> http://codereview.appspot.com/1866042/diff/17001/18090
> File src/devices/lte/rlc-entity.h (right):
>
> http://codereview.appspot.com/1866042/diff/17001/18090#newcode42
> src/devices/lte/rlc-entity.h:42: RLC_MODE_TM, RLC_MODEUM, RLC_MODE_AM
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> this should be done by class inheritance
>
> Done.
>
> http://codereview.appspot.com/1866042/diff/17001/18090#newcode60
> src/devices/lte/rlc-entity.h:60:
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> there should be methods realizing the interface between
> RadioBearerInstance as
>> well as with the Mac. I mean, now the PacketScheduler directly calls
> the
>> Dequeue() method of RadioBearerInstance, so in fact the RLC is
> useless.
>
>> You should modify the API to improve the interface between RadioBearer
> and RLC,
>> as well as between RLC and MAC. In particular:
>
>> 1)  RadioBearerInstance should forward the MSDUs to the associated
> RlcEntity,
>> using a dedicated method of RlcEntity
>
>> 2) the MAC should provide methods so that the RLC can forward the
> MPDUs to the
>> MAC. The API should provide the following functionality mentioned in
> TS 36.322:
>
>> 4.3.2 Services expected from lower layers
>> The following services are expected by RLC from lower layer (i.e.
> MAC):
>> -data transfer;
>> -notification of a transmission opportunity, together with the total
> size of the
>> RLC PDU(s) to be transmitted in the
>> transmission opportunity;
>
>> 3) Since for the GSoC only RLC/TM is expected, the actual
> implementation of
>> RlcEntity will be very simple.
>
>
> I improved the interaction among MAC, RLC and RRC
>
> http://codereview.appspot.com/1866042/diff/17001/18091
> File src/devices/lte/rrc-entity.cc (right):
>
> http://codereview.appspot.com/1866042/diff/17001/18091#newcode108
> src/devices/lte/rrc-entity.cc:108: return false;
> On 2010/08/04 12:40:38, Nicola Baldo wrote:
>> you mean return 0?
>
> Done.
>
> http://codereview.appspot.com/1866042/show



----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b