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

Issue 103750047: Added the concept of energy harvester to the existing ns-3 energy framework

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 6 months ago by cristiano.tapparello
Modified:
7 years, 2 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Added the concept of energy harvester to the existing ns-3 energy framework

Patch Set 1 #

Total comments: 39

Patch Set 2 : Implemented the reviewers comments #

Total comments: 18

Patch Set 3 : Implemented the reviewers comments to Patch Set 2 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1780 lines, -14 lines) Patch
A examples/energy/energy-model-with-harvesting-example.cc View 1 2 1 chunk +337 lines, -0 lines 0 comments Download
M examples/energy/wscript View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M src/energy/doc/energy.rst View 1 2 7 chunks +41 lines, -13 lines 0 comments Download
A src/energy/helper/basic-energy-harvester-helper.h View 1 2 1 chunk +53 lines, -0 lines 0 comments Download
A src/energy/helper/basic-energy-harvester-helper.cc View 1 2 1 chunk +59 lines, -0 lines 0 comments Download
A src/energy/helper/energy-harvester-container.h View 1 2 1 chunk +190 lines, -0 lines 0 comments Download
A src/energy/helper/energy-harvester-container.cc View 1 2 1 chunk +164 lines, -0 lines 0 comments Download
A src/energy/helper/energy-harvester-helper.h View 1 2 1 chunk +98 lines, -0 lines 0 comments Download
A src/energy/helper/energy-harvester-helper.cc View 1 2 1 chunk +79 lines, -0 lines 0 comments Download
A src/energy/model/basic-energy-harvester.h View 1 2 1 chunk +131 lines, -0 lines 0 comments Download
A src/energy/model/basic-energy-harvester.cc View 1 2 1 chunk +180 lines, -0 lines 0 comments Download
A src/energy/model/energy-harvester.h View 1 2 1 chunk +137 lines, -0 lines 0 comments Download
A src/energy/model/energy-harvester.cc View 1 2 1 chunk +105 lines, -0 lines 0 comments Download
M src/energy/model/energy-source.h View 1 5 chunks +29 lines, -1 line 0 comments Download
M src/energy/model/energy-source.cc View 1 4 chunks +30 lines, -0 lines 0 comments Download
A src/energy/test/basic-energy-harvester-test.cc View 1 1 chunk +134 lines, -0 lines 0 comments Download
M src/energy/wscript View 1 3 chunks +11 lines, -0 lines 0 comments Download

Messages

Total messages: 8
cristiano.tapparello
Dear all, I submitted this patch (issue 103750047) for the energy framework that includes the ...
7 years, 6 months ago (2014-05-30 23:50:28 UTC) #1
Stefano Avallone
Hello, I had a first look at this code review. Please find inline some comments. ...
7 years, 4 months ago (2014-07-21 15:57:48 UTC) #2
Stefano Avallone
https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy-harvester.cc File src/energy/model/basic-energy-harvester.cc (right): https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy-harvester.cc#newcode167 src/energy/model/basic-energy-harvester.cc:167: m_totalEnergyHarvestedJ += energyHarvested; On 2014/07/21 15:57:48, stavallo wrote: > ...
7 years, 4 months ago (2014-07-24 15:18:34 UTC) #3
Tom Henderson
Cristiano, this looks good in general. Aside from addressing the comments, I'd like to see ...
7 years, 4 months ago (2014-07-30 21:44:51 UTC) #4
cristiano.tapparello
Dear Stefano and Tom, thank you so much for all your valuable comments to the ...
7 years, 2 months ago (2014-09-04 00:36:31 UTC) #5
Stefano Avallone
Hi Cristiano, I had a look at your latest patch. Only a few comments: - ...
7 years, 2 months ago (2014-09-04 08:41:11 UTC) #6
Tom Henderson
On 2014/09/04 08:41:11, stavallo wrote: > Hi Cristiano, > > I had a look at ...
7 years, 2 months ago (2014-09-04 14:08:23 UTC) #7
cristiano.tapparello
7 years, 2 months ago (2014-09-04 17:17:06 UTC) #8
Dear Stefano and Tom,

thank you for returning the review so quickly!

I implemented your last comments and, hopefully, I fixed all the indentations. I
recently changed the IDE that I use and I forgot to update the coding style
configuration to the ns-3 standard... 

I will be happy to help with the integration of Stefano's wifi sleep patch. I
will start by looking at the code and then get back to you if I have any
comments.

Thank you both again!

Best,

Cristiano

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/basic-e...
File src/energy/helper/basic-energy-harvester-helper.cc (right):

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/basic-e...
src/energy/helper/basic-energy-harvester-helper.cc:29:
m_basicEnergyHarvester.SetTypeId ("ns3::BasicEnergyHarvester");
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/basic-e...
src/energy/helper/basic-energy-harvester-helper.cc:39:
m_basicEnergyHarvester.Set (name, v);
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/basic-e...
src/energy/helper/basic-energy-harvester-helper.cc:45: NS_ASSERT (source !=
NULL);
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/basic-e...
File src/energy/helper/basic-energy-harvester-helper.h (right):

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/basic-e...
src/energy/helper/basic-energy-harvester-helper.h:39: BasicEnergyHarvesterHelper
();
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/energy-...
File src/energy/helper/energy-harvester-helper.cc (right):

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/energy-...
src/energy/helper/energy-harvester-helper.cc:38: return Install
(EnergySourceContainer (source));
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/energy-...
File src/energy/helper/energy-harvester-helper.h (right):

https://codereview.appspot.com/103750047/diff/20001/src/energy/helper/energy-...
src/energy/helper/energy-harvester-helper.h:48: virtual ~EnergyHarvesterHelper
();
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/model/basic-en...
File src/energy/model/basic-energy-harvester.cc (right):

https://codereview.appspot.com/103750047/diff/20001/src/energy/model/basic-en...
src/energy/model/basic-energy-harvester.cc:85: return
m_harvestablePower->GetStream ();
Thanks! I was not completely sure about this... I changed it to 1.

On 2014/09/04 08:41:10, stavallo wrote:
> should return the number of assigned stream. In our case:
> 
> return 1;
> 
> because BasicEnergyHarvester only uses a single random variable.
> Tom, am I correct?

https://codereview.appspot.com/103750047/diff/20001/src/energy/model/basic-en...
File src/energy/model/basic-energy-harvester.h (right):

https://codereview.appspot.com/103750047/diff/20001/src/energy/model/basic-en...
src/energy/model/basic-energy-harvester.h:51: static TypeId GetTypeId (void);
On 2014/09/04 08:41:10, stavallo wrote:
> Please fix indentation

Done.

https://codereview.appspot.com/103750047/diff/20001/src/energy/model/basic-en...
src/energy/model/basic-energy-harvester.h:84: * \returns The number of the
stream set for the random variable.
Thanks for this, I changed it to the right statement.

On 2014/09/04 08:41:10, stavallo wrote:
> AssignStreams usually returns "the number of stream indices assigned by this
> model"
Sign in to reply to this message.

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