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

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

[cristiano.tapparello, 2014-05-30 23:50:28]

Dear all, 

I submitted this patch (issue 103750047) for the energy framework that includes
the introduction of the concept of energy harvester and a simple example
implementation.

This is the first part of the work that I presented at the poster session of the
WNS3 2014.

Thanks,

Cristiano Tapparello

[Stefano Avallone, 2014-07-21 15:57:48]

Hello,

I had a first look at this code review. Please find inline some comments.

Furthermore, I find it rather limiting the assumption that m_harvestablePower is
a uniform random variable. With little effort, it might be made a generic
RandomVariableStream, so that it can be set to whatever random variable through
the attribute system. What do you think? Do you want me to implement such a
change?

Bests,
Stefano

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

https://codereview.appspot.com/103750047/diff/1/src/energy/helper/basic-energ...
src/energy/helper/basic-energy-harvester-helper.cc:48: // check if energy
harvester already exists
Maybe I am missing something but... an EnergySource has a vector of
EnergyHarvester pointers, so why do you check if an energy harvester already
exists?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:167: m_totalEnergyHarvestedJ +=
energyHarvested;
m_totalEnergyHarvestedJ might be used uninitialized here. Shouldn't it be
initialized to zero in DoInitialize?

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
File src/energy/model/energy-source.cc (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
src/energy/model/energy-source.cc:175: NS_LOG_DEBUG ("EnergySource("<< GetNode
()->GetId () << "): Total harvested power = " << totalHarvestedPower);
totalHarvestedPower is always zero here. Shouldn't this macro moved after the
following for loop?

[Stefano Avallone, 2014-07-24 15:18:34]

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

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:167: m_totalEnergyHarvestedJ +=
energyHarvested;
On 2014/07/21 15:57:48, stavallo wrote:
> m_totalEnergyHarvestedJ might be used uninitialized here. Shouldn't it be
> initialized to zero in DoInitialize?

To reply to myself: m_totalEnergyHarvestedJ is a traced value, hence it is
initialized to zero by the TracedValue constructor. Sorry for the noise.

[Tom Henderson, 2014-07-30 21:44:51]

Cristiano, this looks good in general.  Aside from addressing the comments, I'd
like to see a patch to the doc/energy.rst file, and at least one regression test
that uses these classes.

Regarding Stefano's comment to make the Basic class not depend on Uniform random
variable, I'll leave that up to you (whether you want the model to support
non-uniform RVs-- whether that makes any sense to model that).

You'll need to hook the random variable to an AssignStreams() method in any
case.  I can help with random variable stream changes if needed.

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
File examples/energy/energy-model-with-harvesting-example.cc (right):

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
examples/energy/energy-model-with-harvesting-example.cc:25: * harvester to the
nodes.
Can you say more about the (default) behavior here?  The wifi radio starts with
an initial energy of X and drains at a rate of Y/s (what aspects of the
simulation affect this drain rate).  Then, the harvester replenishes the energy
source at a rate of Z/sec....

And say a little more about how the scenario is configured (e.g. hard stop at
time 10 seconds, etc.).

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
examples/energy/energy-model-with-harvesting-example.cc:108: NS_LOG_UNCOND
(Simulator::Now ().GetSeconds ()
avoid using NS_LOG_UNCOND(); these disappear in optimized builds.  Use std::cout
for these kind of example trace sinks.

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
examples/energy/energy-model-with-harvesting-example.cc:162: * configuration.
can you say more why this is magic; what happens if offset is higher or lower
than 81?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/helper/basic-energ...
src/energy/helper/basic-energy-harvester-helper.h:48: ObjectFactory
m_basicEnergyHarvester;
If you have an object factory and configure this helper based on TypeId(), then
why do you need both an abstract base class and derived class for this helper? 
Can you collapse these and provide just a single helper class?  If so, I think
it would be preferable and more in line with the rest of ns-3's helpers (which
in general are not subclassed).

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

https://codereview.appspot.com/103750047/diff/1/src/energy/helper/energy-harv...
src/energy/helper/energy-harvester-container.h:42: * \see NetDeviceContainer
why "see NetDeviceContainer"-- what does NetDevice have to do with it?

Is there any example code that makes use of this class?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:83: NS_LOG_FUNCTION (this <<
maxPower);
provide single NS_LOG_FUNCTION (this << updateInterval << minPower << maxPower);

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:161: return; // stop periodic update
is this the right behavior (to just log a debug message and return).  Or should
the program assert or abort in this case?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:40: * Energy Source
please add more details to the class Doxygen

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:50: BasicEnergyHarvester (double
minPower, double maxPower, Time updateInterval);
doxygen needed

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:100: double GetMaxPower (void) const;
Consider instead to just make (min, max) part of the random variable
configuration.  Also, Stefano suggested making this random variable generic and
not tied to uniform.

See how arp-l3-protocol.cc attribute "RequestJitter" is configured.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:109: virtual void UpdateHarvestedPower
(void);
why is this public?

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
File src/energy/model/energy-harvester.cc (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
src/energy/model/energy-harvester.cc:29: NS_OBJECT_ENSURE_REGISTERED
(EnergyHarvester);
see other comment about indentation (in the header file).

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
File src/energy/model/energy-harvester.h (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
src/energy/model/energy-harvester.h:36: class EnergySource;
do not indent under 'namespace ns3 {'; 
left-align lines 35-125

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
src/energy/model/energy-harvester.h:95: virtual void UpdateHarvestedPower (void)
= 0;
above two statements are missing doxygen

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-source.h
File src/energy/model/energy-source.h (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
src/energy/model/energy-source.h:176: * EnergyHarvester pointer.
what does it mean to the user that there is an underlying vector implementation
and that this appends?  If you append A then B, what is the difference between
that and appending B first then A?  Please clarify the doxygen.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
src/energy/model/energy-source.h:203: * List of device energy models installed
on the same node.
This doxygen is copy/pasted from above.

[cristiano.tapparello, 2014-09-04 00:36:31]

Dear Stefano and Tom,

thank you so much for all your valuable comments to the energy harvesting code!!


I implemented all your comment and suggestions. I answered to all the comments
in line so that you can follow the work that I did for the second patch. 

In addition, the basic energy harvester now uses a generic random variable, I
included a test file for checking that the execution is correct and modified the
energy.srt to include the description of the new code.

Thanks again!

Best,

Cristiano

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
File examples/energy/energy-model-with-harvesting-example.cc (right):

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
examples/energy/energy-model-with-harvesting-example.cc:25: * harvester to the
nodes.
I added a description of the considered scenario with all the parameters of the
different components. Should I also write the value of the residual, total
consumed and harvested energy values at the end of the simulation?

On 2014/07/30 21:44:50, Tom Henderson wrote:
> Can you say more about the (default) behavior here?  The wifi radio starts
with
> an initial energy of X and drains at a rate of Y/s (what aspects of the
> simulation affect this drain rate).  Then, the harvester replenishes the
energy
> source at a rate of Z/sec....
> 
> And say a little more about how the scenario is configured (e.g. hard stop at
> time 10 seconds, etc.).

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
examples/energy/energy-model-with-harvesting-example.cc:108: NS_LOG_UNCOND
(Simulator::Now ().GetSeconds ()
On 2014/07/30 21:44:50, Tom Henderson wrote:
> avoid using NS_LOG_UNCOND(); these disappear in optimized builds.  Use
std::cout
> for these kind of example trace sinks.

Done.

https://codereview.appspot.com/103750047/diff/1/examples/energy/energy-model-...
examples/energy/energy-model-with-harvesting-example.cc:162: * configuration.
I inherit this from energy-model-example.cc example that I am extending by
adding the energy harvester to the nodes.
I personally don't know from where this come from and what is its impact, should
I investigate it and add an explanation?

On 2014/07/30 21:44:50, Tom Henderson wrote:
> can you say more why this is magic; what happens if offset is higher or lower
> than 81?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/helper/basic-energ...
src/energy/helper/basic-energy-harvester-helper.cc:48: // check if energy
harvester already exists
When I first wrote the code I only allowed for a single harvester to be
installed on a node. I then extended it to multiple harvesters and forgot to
remove this check.
Good catch, thanks!

On 2014/07/21 15:57:48, stavallo wrote:
> Maybe I am missing something but... an EnergySource has a vector of
> EnergyHarvester pointers, so why do you check if an energy harvester already
> exists?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/helper/basic-energ...
src/energy/helper/basic-energy-harvester-helper.h:48: ObjectFactory
m_basicEnergyHarvester;
Yes, you are right. Here I followed the implementations of the other helpers of
the framework. I believe that they decided to define an abstract class and then
derive it for the actual implementation to reduce the amount of code to write
when creating a new class (in this case, a new harvester).

In particular, when creating multiple harvesters, the code inside
energy-harvester-helper.cc need to be replicated in all the relative helpers.
The only harvester implementation-specific code is line 47 of
energy-harvester-helper.cc.

Anyway, if collapsing these in a single helper is in line with the rest of
ns-3's helpers, I will update them. If I do this, it is probably best if I also
modify the energy-source-helper and energy-model-helper so that they all follow
the same principle?

 

On 2014/07/30 21:44:50, Tom Henderson wrote:
> If you have an object factory and configure this helper based on TypeId(),
then
> why do you need both an abstract base class and derived class for this helper?

> Can you collapse these and provide just a single helper class?  If so, I think
> it would be preferable and more in line with the rest of ns-3's helpers (which
> in general are not subclassed).

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

https://codereview.appspot.com/103750047/diff/1/src/energy/helper/energy-harv...
src/energy/helper/energy-harvester-container.h:42: * \see NetDeviceContainer
I inherited this from the other containers of the framework. I believe that the
reason is that this class follows exactly the principle behind the
NetDeviceContainer class. Should I remove this reference (and, consequently,
also from the other containers of the framework)? 

On 2014/07/30 21:44:50, Tom Henderson wrote:
> why "see NetDeviceContainer"-- what does NetDevice have to do with it?
> 
> Is there any example code that makes use of this class?

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

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:83: NS_LOG_FUNCTION (this <<
maxPower);
I changed the random variable to be generic, thus this constructor now only
requires the updateInterval parameter.

On 2014/07/30 21:44:50, Tom Henderson wrote:
> provide single NS_LOG_FUNCTION (this << updateInterval << minPower <<
maxPower);

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:161: return; // stop periodic update
I realized that this check is unnecessary since it is actually possible that the
user wants to include in the simulation the internal power consumption of the
harvester. I removed it from the new version.

On 2014/07/30 21:44:50, Tom Henderson wrote:
> is this the right behavior (to just log a debug message and return).  Or
should
> the program assert or abort in this case?

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.cc:167: m_totalEnergyHarvestedJ +=
energyHarvested;
Thanks for both the comments!

On 2014/07/24 15:18:33, stavallo wrote:
> On 2014/07/21 15:57:48, stavallo wrote:
> > m_totalEnergyHarvestedJ might be used uninitialized here. Shouldn't it be
> > initialized to zero in DoInitialize?
> 
> To reply to myself: m_totalEnergyHarvestedJ is a traced value, hence it is
> initialized to zero by the TracedValue constructor. Sorry for the noise.

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

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:40: * Energy Source
On 2014/07/30 21:44:50, Tom Henderson wrote:
> please add more details to the class Doxygen

Done.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:50: BasicEnergyHarvester (double
minPower, double maxPower, Time updateInterval);
On 2014/07/30 21:44:50, Tom Henderson wrote:
> doxygen needed

Done.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:100: double GetMaxPower (void) const;
I think that, as Stefano suggested, allowing to the user to set the particular
random variable to use is actually a good things.
I followed the implementation of the RequestJitter random variable and changed
the code to reflect this.

On 2014/07/30 21:44:50, Tom Henderson wrote:
> Consider instead to just make (min, max) part of the random variable
> configuration.  Also, Stefano suggested making this random variable generic
and
> not tied to uniform.
> 
> See how arp-l3-protocol.cc attribute "RequestJitter" is configured.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/basic-energy...
src/energy/model/basic-energy-harvester.h:109: virtual void UpdateHarvestedPower
(void);
Good observation! There is no need for the method to be public so in the new
version it is private.

On 2014/07/30 21:44:50, Tom Henderson wrote:
> why is this public?

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
File src/energy/model/energy-harvester.cc (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
src/energy/model/energy-harvester.cc:29: NS_OBJECT_ENSURE_REGISTERED
(EnergyHarvester);
On 2014/07/30 21:44:50, Tom Henderson wrote:
> see other comment about indentation (in the header file).

Done.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
File src/energy/model/energy-harvester.h (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
src/energy/model/energy-harvester.h:36: class EnergySource;
On 2014/07/30 21:44:51, Tom Henderson wrote:
> do not indent under 'namespace ns3 {'; 
> left-align lines 35-125

Done.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-harve...
src/energy/model/energy-harvester.h:95: virtual void UpdateHarvestedPower (void)
= 0;
On 2014/07/30 21:44:50, Tom Henderson wrote:
> above two statements are missing doxygen

Done.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
File src/energy/model/energy-source.cc (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
src/energy/model/energy-source.cc:175: NS_LOG_DEBUG ("EnergySource("<< GetNode
()->GetId () << "): Total harvested power = " << totalHarvestedPower);
Definitely, thanks for catching this!

On 2014/07/21 15:57:48, stavallo wrote:
> totalHarvestedPower is always zero here. Shouldn't this macro moved after the
> following for loop?

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-source.h
File src/energy/model/energy-source.h (right):

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
src/energy/model/energy-source.h:176: * EnergyHarvester pointer.
Thanks for this. 

The order does not have any impact on the results of the simulation. However,
since the word "Append" can somehow suggests something different, I renamed the
method "ConnectEnergyHarvester" and clarified this on the doxygen.

On 2014/07/30 21:44:51, Tom Henderson wrote:
> what does it mean to the user that there is an underlying vector
implementation
> and that this appends?  If you append A then B, what is the difference between
> that and appending B first then A?  Please clarify the doxygen.

https://codereview.appspot.com/103750047/diff/1/src/energy/model/energy-sourc...
src/energy/model/energy-source.h:203: * List of device energy models installed
on the same node.
Oops, sorry about this. Fixed in the new version.

On 2014/07/30 21:44:51, Tom Henderson wrote:
> This doxygen is copy/pasted from above.

[Stefano Avallone, 2014-09-04 08:41:11]

Hi Cristiano,

I had a look at your latest patch. Only a few comments:

- indentation is broken in several places, there is a tab instead of two blank
spaces (I started leaving comments then at some point I gave up :-) )

- I proposed to change the return value of AssignStreams. However, please wait
for Tom's approval

- please check the energy.rst diff, as the side-by-side diff returns "bad
content"

Thanks,
Stefano

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");
Please fix indentation

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);
Please fix indentation

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);
Please fix indentation

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
();
Please fix indentation

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));
Please fix indentation

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
();
Please fix indentation

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 ();
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);
Please fix indentation

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.
AssignStreams usually returns "the number of stream indices assigned by this
model"

[Tom Henderson, 2014-09-04 14:08:23]

On 2014/09/04 08:41:11, stavallo wrote:
> Hi Cristiano,
> 
> I had a look at your latest patch. Only a few comments:
> 
> - indentation is broken in several places, there is a tab instead of two blank
> spaces (I started leaving comments then at some point I gave up :-) )

I will run the new files through "check-style.py" before committing. 
check-style.py can be found in utils directory and requires the uncrustify tool
installed on the system.

For new files, it is a good idea to run 'check-style.py -i -f <filename> -l 3'
on each file when ready to commit.

For patched files, it is often not a good idea if the style checker causes a lot
of additional whitespace changes unrelated to the patch.  For these, I usually
just visually inspect.

> 
> - I proposed to change the return value of AssignStreams. However, please wait
> for Tom's approval

You are correct; it should return the value 1.

> 
> - please check the energy.rst diff, as the side-by-side diff returns "bad
> content"

I didn't encounter this problem with my browser.


Thanks for this review.  I intend to apply this patch in the next day or two,
followed by Stefano's wifi sleep patch (which he said will coexist with this
patch).

[cristiano.tapparello, 2014-09-04 17:17:06]

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"