Bug 2609

Summary: code review: LTE Carrier Aggregation
Product: ns-3 Reporter: Tom Henderson <tomh>
Component: lteAssignee: Biljana Bojović <bbojovic>
Status: RESOLVED FIXED    
Severity: enhancement CC: ns-bugs
Priority: P3    
Version: unspecified   
Hardware: All   
OS: All   

Description Tom Henderson 2017-01-03 23:36:41 UTC
https://codereview.appspot.com/316980043
Comment 1 Biljana Bojović 2017-01-10 12:21:59 UTC
Should I upload here the patch for carrier aggregation feature?
Comment 2 Tom Henderson 2017-01-10 15:06:50 UTC
(In reply to Biljana Bojović from comment #1)
> Should I upload here the patch for carrier aggregation feature?

Patch set is too large for Rietveld download; if there is a way for reviewers to download it who might want to try it out, can you either provide a pointer to a public repo or post a (compressed) patch in the tracker or somewhere else?

Can you summarize, are there any concerns that ns-3 maintainers should focus on, or is this uncontroversial extension and ready to commit?  I see some template expansion of the event scheduling, but otherwise, it seems to be self-contained.
Comment 3 Biljana Bojović 2017-01-11 11:44:03 UTC
(In reply to Tom Henderson from comment #2)

Is it good enough for this purpose the LENA public repository http://lena.cttc.es/hg/lena/? It is merged 3 weeks ago with ns-3-dev. If not, I will can post a compressed patch.

> Can you summarize, are there any concerns that ns-3 maintainers should focus
> on, or is this uncontroversial extension and ready to commit?  I see some
> template expansion of the event scheduling, but otherwise, it seems to be
> self-contained.

Yes, it is mostly self-contained, except for additional functions that are added to the core module. My main concern is whether ns-3 maintainer of the core module agrees with these changes. If not, I would need to do it in another way. These extensions in core module were written by Danilo Abrignani as part of original CA implementation and CA code still depends on it. The code could be re-factored to not depend on this extension in ns-3 core, and to eliminate it, but I thought that this extension may be useful for other developers so I left it.
Comment 4 Biljana Bojović 2017-02-02 10:26:34 UTC
I committed all the code to ns-3-dev. 

There were several changesets, starting from changeset 12534:fea54522b923 until changeset 12608: 894a205ead11, but the one that is changing the architecture of LTE module and the way the LTE module works is changeset 12602, 2b0e3e9eb4e4:http://code.nsnam.org/ns-3-dev/rev/2b0e3e9eb4e4.Other changesets were extensions of LTE module for carrier aggregation.

I addressed the comments from reviewers: https://codereview.appspot.com/316980043