ns-3: Energy Model

Network Security Lab (NSL), University of Washington, Seattle has begun work on an generic energy model for ns3. The goal is to simulate an energy source and keep track of energy consumptions of various devices in the node. Current version focuses on radio energy consumption.

The energy model has 2 major components:
1. Energy Source: Energy source model (eg. battery) which provides energy to all devices.
2. Devices energy model: Energy consumption model for specific devices (eg. radio) on the node. Note that device energy model depends on an energy source.

This patch is based on the latest ns-3-dev tree. To use the energy model, apply the latest patch to ns-3-dev.

wiki link: http://www.nsnam.org/wiki/index.php/NS-3_energy_model

[Mathieu Lacage, 2010-04-30 06:52:51]

overall, it looks pretty good. My main concern is in the EnergyDeviceModel
helper class: it looks a bit suspicious. Detailed comments below.

http://codereview.appspot.com/1008043/diff/1/7
File src/devices/energy-model/device-energy-model.h (right):

http://codereview.appspot.com/1008043/diff/1/7#newcode62
src/devices/energy-model/device-energy-model.h:62: protected:
would be nice to control access to this data member through member methods

Based on how you use this field in the rest of the code, I would suggest the
following protected method:

protected:
  void DecreaseRemainingEnergy (...);

http://codereview.appspot.com/1008043/diff/1/9
File src/devices/energy-model/energy-source.h (right):

http://codereview.appspot.com/1008043/diff/1/9#newcode56
src/devices/energy-model/energy-source.h:56: TypeId tid;   // type ID of the
specific child device energy model
I fail to see why you need to keep track of the TypeId of each energy model. Why
can't you call ObjectBase::GetInstanceTypeId on your energy model ?

i.e. 
Ptr<DeviceEnergyModel> energyModel = ...;
TypeId tid = energyModel->GetInstanceTypeId ();

http://codereview.appspot.com/1008043/diff/1/9#newcode98
src/devices/energy-model/energy-source.h:98: * This function returns the
percentage of energy left in the energy source.
hrm. Does this return a number in the range [0,1] or in [0,100] ?

Depending on the answer, you might want to change the name of this method

http://codereview.appspot.com/1008043/diff/1/9#newcode131
src/devices/energy-model/energy-source.h:131: protected:
why is this field protected ? In general, we always avoid protected data
members. Sometimes, we use protected method members to access private data
members, but, even that is frowned upon.

From the rest of the code in this review, I would suggest the addition of the
following protected member method:
protected:
  // notify all registered energy models that all energy has been consumed
  void NotifyEnergyDrained (void);

http://codereview.appspot.com/1008043/diff/1/9#newcode134
src/devices/energy-model/energy-source.h:134: std::vector<DeviceEnergyModelInfo>
m_deviceEnergyModelList;
I see that EnergySource has a pointer to DeviceEnergyModel and DeviceEnergyModel
has a pointer back to EnergySource so, we have a reference cycle. You probably
will need to add a DoDispose implementation in both classes to clear these
pointers to make sure that the reference cycle gets broken at the end of the
simulation

http://codereview.appspot.com/1008043/diff/1/14
File src/helper/basic-energy-source-helper.h (right):

http://codereview.appspot.com/1008043/diff/1/14#newcode37
src/helper/basic-energy-source-helper.h:37: BasicEnergySourceHelper ();
I would argue for moving this implementation to EnergyModelHelper:

class EnergyModelHelper
{
public:
  void SetEnergySourceType (std::string type);
  void SetEnergySourceAttribute (std::string name, const AttributeValue &value);
private:
  Ptr<EnergySource> Create (void) const;
  ObjectFactory m_energySourceFactory;
};

http://codereview.appspot.com/1008043/diff/1/17
File src/helper/radio-energy-helper.cc (right):

http://codereview.appspot.com/1008043/diff/1/17#newcode68
src/helper/radio-energy-helper.cc:68: node->AggregateObject (model);
What is the reason for aggregating the energy model onto the node ? Do you need
to get it back later with GetObject ? It is especially troubling since there can
be more than one energy model of the same type in each node so, it will not work
in this case.

[(Tony) He Wu, 2010-04-30 23:45:45]

http://codereview.appspot.com/1008043/diff/1/7
File src/devices/energy-model/device-energy-model.h (right):

http://codereview.appspot.com/1008043/diff/1/7#newcode62
src/devices/energy-model/device-energy-model.h:62: protected:
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> would be nice to control access to this data member through member methods
> 
> Based on how you use this field in the rest of the code, I would suggest the
> following protected method:
> 
> protected:
>   void DecreaseRemainingEnergy (...);

Done.

http://codereview.appspot.com/1008043/diff/1/9
File src/devices/energy-model/energy-source.h (right):

http://codereview.appspot.com/1008043/diff/1/9#newcode56
src/devices/energy-model/energy-source.h:56: TypeId tid;   // type ID of the
specific child device energy model
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> I fail to see why you need to keep track of the TypeId of each energy model.
Why
> can't you call ObjectBase::GetInstanceTypeId on your energy model ?
> 
> i.e. 
> Ptr<DeviceEnergyModel> energyModel = ...;
> TypeId tid = energyModel->GetInstanceTypeId ();
> 

Done.
We were unaware of the GetInstanceTypeId () method.. This structure is removed
from the latest code.

http://codereview.appspot.com/1008043/diff/1/9#newcode98
src/devices/energy-model/energy-source.h:98: * This function returns the
percentage of energy left in the energy source.
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> hrm. Does this return a number in the range [0,1] or in [0,100] ?
> 
> Depending on the answer, you might want to change the name of this method

The range is [0,1]. We have added specification in the comment. Please suggest
for a better naming if the current name is misleading.

http://codereview.appspot.com/1008043/diff/1/9#newcode131
src/devices/energy-model/energy-source.h:131: protected:
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> why is this field protected ? In general, we always avoid protected data
> members. Sometimes, we use protected method members to access private data
> members, but, even that is frowned upon.
> 
> From the rest of the code in this review, I would suggest the addition of the
> following protected member method:
> protected:
>   // notify all registered energy models that all energy has been consumed
>   void NotifyEnergyDrained (void);

Done.
This list is protected because we want to enable user written child classes (of
EnergySource) to freely access in case they need to access the list of
DeviceEnergyModel objects. One example of such case will be logging of total
energy consumption of certain (but not all) DeviceEnergyModel objects.
We decide to move this list as private and provide public methods to search the
list for desired DeviceEnergyModel objects. This change is included in the
latest update.

http://codereview.appspot.com/1008043/diff/1/9#newcode134
src/devices/energy-model/energy-source.h:134: std::vector<DeviceEnergyModelInfo>
m_deviceEnergyModelList;
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> I see that EnergySource has a pointer to DeviceEnergyModel and
DeviceEnergyModel
> has a pointer back to EnergySource so, we have a reference cycle. You probably
> will need to add a DoDispose implementation in both classes to clear these
> pointers to make sure that the reference cycle gets broken at the end of the
> simulation

Done.

http://codereview.appspot.com/1008043/diff/1/14
File src/helper/basic-energy-source-helper.h (right):

http://codereview.appspot.com/1008043/diff/1/14#newcode37
src/helper/basic-energy-source-helper.h:37: BasicEnergySourceHelper ();
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> I would argue for moving this implementation to EnergyModelHelper:
> 
> class EnergyModelHelper
> {
> public:
>   void SetEnergySourceType (std::string type);
>   void SetEnergySourceAttribute (std::string name, const AttributeValue
&value);
> private:
>   Ptr<EnergySource> Create (void) const;
>   ObjectFactory m_energySourceFactory;
> };

We intentionally separated the EnergySourceHelper from EnergyModelHelper so that
when implementing a new EnergySource child (and its helper) without changing the
EnergyModelHelper class (the main install method for energy model).
When implementing a more complicated EnergySource, eg. AdvEnergySource. One may
wish to perform setups specific to AdvEnergySource, hence requiring its own
helper class.
We think we will prefer to keep the helper class structure as is for now. Please
let us know your comments.

http://codereview.appspot.com/1008043/diff/1/17
File src/helper/radio-energy-helper.cc (right):

http://codereview.appspot.com/1008043/diff/1/17#newcode68
src/helper/radio-energy-helper.cc:68: node->AggregateObject (model);
On 2010/04/30 06:52:51, Mathieu Lacage wrote:
> What is the reason for aggregating the energy model onto the node ? Do you
need
> to get it back later with GetObject ? It is especially troubling since there
can
> be more than one energy model of the same type in each node so, it will not
work
> in this case.

Done.
Originally we were assuming no identical child DeviceEnergyModel objects can
co-exist on a single node (i.e. we cannot install 2 BasicRadioEnergyModel onto
the same node). And some classes defined in our wireless interference (jamming)
modules uses the GetObject method to obtain a pointer to RadioEnergyModel.
Since a list of all DeviceEnergyModel objects installed on node is kept in the
EnergySource object, it is no longer necessary to aggregate DeviceEnergyModel to
the node now. The latest code includes this change.

[Mathieu Lacage, 2010-05-04 09:19:59]

nothing to add to the minor comments below.

http://codereview.appspot.com/1008043/diff/8001/9008
File src/devices/energy-model/energy-source.h (right):

http://codereview.appspot.com/1008043/diff/8001/9008#newcode87
src/devices/energy-model/energy-source.h:87: * \return Energy fraction =
remaining energy / initial energy (0 - 1)
I don't know what american math conventions are but the above indicates range
0,1 with 0 and 1 excluded. I would have expected the "[0,1]" notation instead to
indicate that the bounds 0 and 1 are included.

http://codereview.appspot.com/1008043/diff/8001/9008#newcode91
src/devices/energy-model/energy-source.h:91: double GetEnergyFraction (void)
const;
I guess I would rename Fraction to Ratio which feels more descriptive to me but
if you disagree, feel free to ignore.

http://codereview.appspot.com/1008043/diff/8001/9011
File src/devices/wimax/simple-ofdm-wimax-phy.cc (right):

http://codereview.appspot.com/1008043/diff/8001/9011#newcode336
src/devices/wimax/simple-ofdm-wimax-phy.cc:336: double Nwb = -114 +
m_noiseFigure + 10 * log (GetBandwidth () / 1000000000.0) / 2.303;
I suspect that you screwed up some kind of merge because this change was not
generated by you.

http://codereview.appspot.com/1008043/diff/8001/9014
File src/helper/radio-energy-helper.cc (right):

http://codereview.appspot.com/1008043/diff/8001/9014#newcode83
src/helper/radio-energy-helper.cc:83: * TODO Change this to fatal error. Because
by definition (or the
can you call NS_FATAL_ERROR then ?

[Tom Henderson, 2010-05-04 20:35:36]

I just had minor comments; looks good.

http://codereview.appspot.com/1008043/diff/1/18
File src/helper/radio-energy-helper.h (right):

http://codereview.appspot.com/1008043/diff/1/18#newcode40
src/helper/radio-energy-helper.h:40: * Construct a Mobility Helper which is used
to add an energy model to a node
s/Mobility/RadioEnergy

http://codereview.appspot.com/1008043/diff/8001/9008
File src/devices/energy-model/energy-source.h (right):

http://codereview.appspot.com/1008043/diff/8001/9008#newcode69
src/devices/energy-model/energy-source.h:69: * converter function should
suffice.
Maybe this blanket statement about the assumed units of this object (Joules) and
how to convert to other units could be instead stated once in the class doxygen
above.

http://codereview.appspot.com/1008043/diff/8001/9008#newcode87
src/devices/energy-model/energy-source.h:87: * \return Energy fraction =
remaining energy / initial energy (0 - 1)
On 2010/05/04 09:19:59, Mathieu Lacage wrote:
> I don't know what american math conventions are but the above indicates range
> 0,1 with 0 and 1 excluded. I would have expected the "[0,1]" notation instead
to
> indicate that the bounds 0 and 1 are included.

agree with Mathieu

http://codereview.appspot.com/1008043/diff/8001/9008#newcode91
src/devices/energy-model/energy-source.h:91: double GetEnergyFraction (void)
const;
On 2010/05/04 09:19:59, Mathieu Lacage wrote:
> I guess I would rename Fraction to Ratio which feels more descriptive to me
but
> if you disagree, feel free to ignore.

I think fraction is more specific.

http://codereview.appspot.com/1008043/diff/8001/9008#newcode112
src/devices/energy-model/energy-source.h:112: 
It doesn't seem that there is a method to get all energy models (regardless of
typeid).

[(Tony) He Wu, 2010-05-05 17:21:56]

http://codereview.appspot.com/1008043/diff/8001/9008
File src/devices/energy-model/energy-source.h (right):

http://codereview.appspot.com/1008043/diff/8001/9008#newcode87
src/devices/energy-model/energy-source.h:87: * \return Energy fraction =
remaining energy / initial energy (0 - 1)
On 2010/05/04 20:35:36, Tom Henderson wrote:
> On 2010/05/04 09:19:59, Mathieu Lacage wrote:
> > I don't know what american math conventions are but the above indicates
range
> > 0,1 with 0 and 1 excluded. I would have expected the "[0,1]" notation
instead
> to
> > indicate that the bounds 0 and 1 are included.
> 
> agree with Mathieu
> 

Done.

I have fixed the comment. It should be [0,1], which will include 0 and 1.

http://codereview.appspot.com/1008043/diff/8001/9008#newcode91
src/devices/energy-model/energy-source.h:91: double GetEnergyFraction (void)
const;
On 2010/05/04 20:35:36, Tom Henderson wrote:
> On 2010/05/04 09:19:59, Mathieu Lacage wrote:
> > I guess I would rename Fraction to Ratio which feels more descriptive to me
> but
> > if you disagree, feel free to ignore.
> 
> I think fraction is more specific.

I think we'll keep it as fraction.

http://codereview.appspot.com/1008043/diff/8001/9008#newcode112
src/devices/energy-model/energy-source.h:112: 
On 2010/05/04 20:35:36, Tom Henderson wrote:
> It doesn't seem that there is a method to get all energy models (regardless of
> typeid).

All energy models are inhered from DeviceEnergyModel. We can call
FindDeviceEnergyModels ("ns3::DeviceEnergyModel"); to get all energy models.

http://codereview.appspot.com/1008043/diff/8001/9011
File src/devices/wimax/simple-ofdm-wimax-phy.cc (right):

http://codereview.appspot.com/1008043/diff/8001/9011#newcode336
src/devices/wimax/simple-ofdm-wimax-phy.cc:336: double Nwb = -114 +
m_noiseFigure + 10 * log (GetBandwidth () / 1000000000.0) / 2.303;
On 2010/05/04 09:19:59, Mathieu Lacage wrote:
> I suspect that you screwed up some kind of merge because this change was not
> generated by you.

This was grabbed from the main dev tree. I shall remove it from the patch set.
Sorry for the confusion.

http://codereview.appspot.com/1008043/diff/8001/9014
File src/helper/radio-energy-helper.cc (right):

http://codereview.appspot.com/1008043/diff/8001/9014#newcode83
src/helper/radio-energy-helper.cc:83: * TODO Change this to fatal error. Because
by definition (or the
On 2010/05/04 09:19:59, Mathieu Lacage wrote:
> can you call NS_FATAL_ERROR then ?

Done.

[(Tony) He Wu, 2010-05-05 23:31:17]

Patch set 3 is based on comments from all previous reviews.

http://codereview.appspot.com/1008043/diff/8001/9008
File src/devices/energy-model/energy-source.h (right):

http://codereview.appspot.com/1008043/diff/8001/9008#newcode69
src/devices/energy-model/energy-source.h:69: * converter function should
suffice.
On 2010/05/04 20:35:36, Tom Henderson wrote:
> Maybe this blanket statement about the assumed units of this object (Joules)
and
> how to convert to other units could be instead stated once in the class
doxygen
> above.

Done.

The comments on energy units are moved to the class doxygen.

[(Tony) He Wu, 2010-05-07 22:06:39]

Patch set 4 highlights:

1. Add unit test suite (devices-basic-energy-model) for BasicEnergySource and
BasicRadioEnergyModel.
2. Introduce energy depletion callbacks in BasicRadioEnergyModel. For flexible
energy depletion behavior modeling.
3. Bug fixes from previous patch set.

Comments are appreciated.

Thanks,
Tony

[Tom Henderson, 2010-06-14 04:35:17]

On 2010/05/07 22:06:39, (Tony) He Wu wrote:
> Patch set 4 highlights:
> 
> 1. Add unit test suite (devices-basic-energy-model) for BasicEnergySource and
> BasicRadioEnergyModel.
> 2. Introduce energy depletion callbacks in BasicRadioEnergyModel. For flexible
> energy depletion behavior modeling.
> 3. Bug fixes from previous patch set.
> 
> Comments are appreciated.
> 

The main comment I have after reviewing tonight is whether all of this should
end up in src/devices, or whether it should perhaps go into a new directory
src/energy, or src/node, or src/node/energy.  It seems to be more general than
src/devices (and not a device per-se).

- Tom

[(Tony) He Wu, 2010-06-14 22:59:51]

On 2010/06/14 04:35:17, Tom Henderson wrote:
> The main comment I have after reviewing tonight is whether all of this should
> end up in src/devices, or whether it should perhaps go into a new directory
> src/energy, or src/node, or src/node/energy.  It seems to be more general than
> src/devices (and not a device per-se).
> 
> - Tom

Hello,

Yes, we are open to where the energy model would be placed, based on the
judgement of ns-3 maintainers. "src/energy" seems to be a good option.

In order to help the decision, here is what we envision for the energy module:

1. EnergySource class represents sources such as battery/solar panel, etc. that
supply energy to the node.
2. Subclasses of DeviceEnergyModel represent the energy consumption of various
devices (peripherals) of the node. eg. Wifi radio, WiMAX radio .

Thanks,
Tony

[Tom Henderson, 2010-07-02 23:20:23]

On 2010/06/14 22:59:51, (Tony) He Wu wrote:
> On 2010/06/14 04:35:17, Tom Henderson wrote:
> > The main comment I have after reviewing tonight is whether all of this
should
> > end up in src/devices, or whether it should perhaps go into a new directory
> > src/energy, or src/node, or src/node/energy.  

After some discussion with you today, the suggestion I have is to create the
following structure for your code for this release:

src/contrib/energy/model
src/contrib/energy/helper
src/contrib/energy/test

The rationale for contrib/ for ns-3.9 is that presently we cannot use it without
modifying the existing wireless Phy until some integration issues are worked
out.  I wasn't aware that we need additional callbacks in WifiPhy to make use of
this (needs resolved in ns-3.10 timeframe).

[(Tony) He Wu, 2010-07-03 02:35:31]

On 2010/07/02 23:20:23, Tom Henderson wrote:
> 
> After some discussion with you today, the suggestion I have is to create the
> following structure for your code for this release:
> 
> src/contrib/energy/model
> src/contrib/energy/helper
> src/contrib/energy/test
> 
> The rationale for contrib/ for ns-3.9 is that presently we cannot use it
without
> modifying the existing wireless Phy until some integration issues are worked
> out.  I wasn't aware that we need additional callbacks in WifiPhy to make use
of
> this (needs resolved in ns-3.10 timeframe).

The energy model has been moved (in patch set 8) to src/contrib/energy. It is
ready for release.

Thanks,
Tony

[(Tony) He Wu, 2010-07-13 02:27:09]

Hi,

Highlight for patch set 9:
1. Remove RadioEnergyModel. A parent class for all radio devices energy models
is not necessary.
2. Rename BasicRadioEnergyModel to WifiRadioEnergyModel.
3. Change how EnergySource and DeviceEnergyModel objects are installed on node.
EnergySource objects are installed separately using node pointer/container/name.
DeviceEnergyModel objects are installed after installing EnergySource using
NetDevice pointer/container. EnergyModelHelper is removed.
4. EnergySourceContainer. Enable easy access to EnergySource objects.
5. DeviceEnergyModelContainer. Enable easy access to DeviceEnergyModel objects.
6. Modify YansWifiPhy to support energy model.
7. Example using modified YansWifiPhy.

Thanks,
Tony

[Tom Henderson, 2010-07-13 04:14:46]

On 2010/07/13 02:27:09, (Tony) He Wu wrote:
> Hi,
> 
> Highlight for patch set 9:
> 1. Remove RadioEnergyModel. A parent class for all radio devices energy models
> is not necessary.
> 2. Rename BasicRadioEnergyModel to WifiRadioEnergyModel.
> 3. Change how EnergySource and DeviceEnergyModel objects are installed on
node.
> EnergySource objects are installed separately using node
pointer/container/name.
> DeviceEnergyModel objects are installed after installing EnergySource using
> NetDevice pointer/container. EnergyModelHelper is removed.
> 4. EnergySourceContainer. Enable easy access to EnergySource objects.
> 5. DeviceEnergyModelContainer. Enable easy access to DeviceEnergyModel
objects.
> 6. Modify YansWifiPhy to support energy model.
> 7. Example using modified YansWifiPhy.
> 

Tony, it looks like the above patchset does two things:
1) reorganization of the already merged code
2) patches to wifi to enable its use

Can you please factor out 1) above and prepare a patch for ns-3-dev to apply now
so that all that is remaining is the wifi-specific patch?  This will allow other
users who may start to use it now that it is in ns-3-dev (for instance, Andrea
Sacco's UAN project) to not have to deal with a future change.

[(Tony) He Wu, 2010-07-13 05:45:13]

On 2010/07/13 04:14:46, Tom Henderson wrote:
> Tony, it looks like the above patchset does two things:
> 1) reorganization of the already merged code
> 2) patches to wifi to enable its use
> 
> Can you please factor out 1) above and prepare a patch for ns-3-dev to apply
now
> so that all that is remaining is the wifi-specific patch?  This will allow
other
> users who may start to use it now that it is in ns-3-dev (for instance, Andrea
> Sacco's UAN project) to not have to deal with a future change.

Hi Tom,

I will upload a new patch set tomorrow. It will contain only patches to wifi.

Thanks,
Tony

[(Tony) He Wu, 2010-07-14 00:32:00]

Hi All,

Highlights for patch set 10:
1. Modify YansWifiPhy to support energy model.
2. Example using modified YansWifiPhy.

Note that patch set 10 is based on the latest ns-3-dev (changset: 6444, patch
set 8). Patch set 10 does not include changes to energy model and energy model
helpers introduced in patch set 9.

Thanks,
Tony

[(Tony) He Wu, 2010-08-07 03:03:19]

Hi All,

Highlights of patch set 12:

EnergySource:
1. Add new interfaces (using current, or Ampere) that allows the EnergySource to
use total current from all devices to update remaining energy.
2. Add "supply voltage" parameter.
3. Add EnergySourceContainer class.
4. Installs on the node separately from DeviceEnergyModel obejcts.
5. BasicEnergySource now performs periodic update of remaining energy (was done
by BasicRadioEnergyModel or DeviceEnergyModels).

DeviceEnergyModel:
1. Add "current" parameter. Each state of the device will have a different
current draw.
2. RadioEnergyModel class has been removed. It is difficult and unnecessary to
have a base class for all radio devices.
3. BasicRadioEnerygModel is renamed to WifiRadioEnergyModel.
4. Add DeviceEnergyModelContainer class.
5. DeviceEnergyModels are now installed using NetDevices or NetDeviceContainers.

Note:
1. Python bindings are re-generated and included in this patch set.
2. Patch to WifiPhy & YansWifiPhy are not included in this patch set. Sections
in WifiRadioEnergyModelHelper that uses the modified WifiPhy & YansWifiPhy is
commented out. In order to use WifiRadioEnergyModel, modification to WifiPhy and
YansWifiPhy is required.
3. Interfaces for updating energy in terms of Joules are preserved. Developers
of new DeviceEnergyModel or EnergySource can chooses to use either interfaces.
4. It is not recommended (but not prevented by the model) to mix models that use
Energy (in joules) and current (in Ampere) in the same simulation.

Thanks,
Tony

[(Tony) He Wu, 2010-10-07 00:39:39]

Hi,

Highlights of patch set 14.
1. Add callback to WifiPhy to support energy model.
2. Modified YansWifiPhy to make use of energy model callbacks added in WifiPhy.
3. Energy model test case.
4. Energy model example.

Thanks,
Tony

[(Tony) He Wu, 2010-10-19 01:08:17]

Hi All,

Highlight of patch set 15:

Add Rakhmatov-Vrudhula non-linear battery model. The model is capable of
capturing load capacity and recovery effects of batteries. Batteries are
characterized by 2 parameters: alpha and beta. Which can both be obtained from
the discharge curve of the batteries.

The model is developed by Daler Rakhmatov & Sarma Vrudhula in: "Battery Lifetime
Prediction for Energy-Aware Computing" and "An Analytical High-Level Battery
Model for Use in Energy Management of Portable Electronic Systems".

The real-time algorithm is developed by Matthias Handy & Dirk Timmermann in:
"Simulation of Mobile Wireless Networks with Accurate Modeling of non-linear
battery effects". The real-time algorithm is modified by us of this code for
improved accuracy and reduced computation (sampling) overhead.

Thanks,
Tony

[Mathieu Lacage, 2010-10-19 11:56:07]

these are really minor comments. 

I looked again at the energy model code in src/contrib/energy. A couple of
questions:

1) What is the purpose of DeviceEnergyModel::Set/GetNode ? i.e., what do you
need them for ? Could we not kill them ?
2) Could we not kill EnergySource::DecreaseRemainingEnergy and
EnergySource::IncreaseRemainingEnergy ?

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
File src/contrib/energy/model/rv-battery-model.cc (right):

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
src/contrib/energy/model/rv-battery-model.cc:102: NS_LOG_FUNCTION (this);
in general, adding logging code to Get methods merely increases the amount of
log output without helping debugging. Simple Set methods likewise (but some of
them might be useful for critical variables)

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
src/contrib/energy/model/rv-battery-model.cc:390: double firstDelta =
(t.GetSeconds () - sk.GetSeconds ()) / 60;
how about adding a Time::GetMinutes method instead ?

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
File src/contrib/energy/model/rv-battery-model.h (right):

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
src/contrib/energy/model/rv-battery-model.h:191: virtual void DoStart (void);
add comment:
// defined in ns3::Object

[(Tony) He Wu, 2010-10-19 18:44:16]

On 2010/10/19 11:56:07, Mathieu Lacage wrote:
> 1) What is the purpose of DeviceEnergyModel::Set/GetNode ? i.e., what do you
> need them for ? Could we not kill them ?

Right now DeviceEnergyModel::Set/GetNode are used for printing the node ID in
debug statements. We can remove them from our code. However, the UAN energy
model code makes use of this method for printing node IDs. Changes on our side
will break their code.

> 2) Could we not kill EnergySource::DecreaseRemainingEnergy and
> EnergySource::IncreaseRemainingEnergy ?

These interfaces are intended for device models that consumes energy (Joules)
directly from energy sources (they do not have a current value for each state).
We will remove these interfaces in the next patch.

Thanks,
Tony

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
File src/contrib/energy/model/rv-battery-model.cc (right):

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
src/contrib/energy/model/rv-battery-model.cc:102: NS_LOG_FUNCTION (this);
On 2010/10/19 11:56:08, Mathieu Lacage wrote:
> in general, adding logging code to Get methods merely increases the amount of
> log output without helping debugging. Simple Set methods likewise (but some of
> them might be useful for critical variables)

Will remove them in next patch.

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
src/contrib/energy/model/rv-battery-model.cc:390: double firstDelta =
(t.GetSeconds () - sk.GetSeconds ()) / 60;
On 2010/10/19 11:56:08, Mathieu Lacage wrote:
> how about adding a Time::GetMinutes method instead ?

Adding a GetMinutes method will be better.

To add the GetMinutes method, it seems necessary to add a new time unit
(minutes) to Time::unit and corresponding functions. If adding the GetMinutes
method involves several changes to the Time class. I think it is better to make
this change as a separate patch rather then combining it with the energy model
patch.

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
File src/contrib/energy/model/rv-battery-model.h (right):

http://codereview.appspot.com/1008043/diff/217007/src/contrib/energy/model/rv...
src/contrib/energy/model/rv-battery-model.h:191: virtual void DoStart (void);
On 2010/10/19 11:56:08, Mathieu Lacage wrote:
> add comment:
> // defined in ns3::Object

Done.

[Mathieu Lacage, 2010-10-20 08:21:21]

On Tue, Oct 19, 2010 at 20:44,  <mdzzzz@gmail.com> wrote:

> Right now DeviceEnergyModel::Set/GetNode are used for printing the node
> ID in debug statements. We can remove them from our code. However, the
> UAN energy model code makes use of this method for printing node IDs.
> Changes on our side will break their code.

You already have the node id in NS_LOG statements so, you don't need
to print it yourself.

>> how about adding a Time::GetMinutes method instead ?
>
> Adding a GetMinutes method will be better.
>
> To add the GetMinutes method, it seems necessary to add a new time unit
> (minutes) to Time::unit and corresponding functions. If adding the
> GetMinutes method involves several changes to the Time class. I think it
> is better to make this change as a separate patch rather then combining
> it with the energy model patch.

yes.

-- 
Mathieu Lacage <mathieu.lacage@gmail.com>

[Nicola Baldo, 2010-10-21 13:51:47]

Hi Tony,

I reviewed your code as far as the wifi modifications are concerned. I think it
is not necessary (and hence not advisable) to modify the wifi code for your
purpose. What you should really do is to make your energy model implement the
WifiPhyListener interface, and then register it by calling
WifiPhy::RegisterListener (). 

If you follow this approach, you won't need to touch any wifi code, hence you
won't require my approval to have your patch accepted in ns-3-dev.

Regards,

Nicola

[(Tony) He Wu, 2010-10-22 06:44:53]

Hi All,

Highlights of patch set 16:

1. Remove all modification to Wifi classes (WifiPhy and YansWifiPhy). Introduced
WifiRadioEnergyModelPhyListener to bridge between WifiPhy and
WifiRadioEnergyModel. Thanks to Nicola's comment.
2. Various changes according to Mathieu's comment.

Thanks,
Tony