[Ns-developers] spectrum

[Nicola Baldo]

Hi all,

recently I have been doing further development on the spectrum 
framework. I think the status of the code is now decent enough, so I am 
asking for the code to be reviewed for inclusion in ns-3.9.

I've uploaded the code for review here:
http://codereview.appspot.com/230045

if you prefer, you can also have a look at this mercurial repository:
http://code.nsnam.org/nbaldo/ns-3-dev-spectrum/

though you have to be aware that the latter includes some code which is 
still work in progress (e.g., the wifi spectrum phy) and which I am not 
proposing for merging into ns-3-dev at this moment.

Finally, below some answers to some comments by Mathieu on my previous 
release of the code (~1 year ago!) to clarify a few issues.

Feedback welcome!

Regards,

Nicola



Mathieu Lacage wrote:
> I looked carefully at your code but, what is really missing is a picture
> of how this would be used in real devices so, I can't really comment on
> the overall direction.

Now there is a NetDevice implementation using the spectrum framework in 
the intended way. Have a look at the following files:

src/devices/spectrum/half-duplex-ideal-ofdm-phy.h
src/devices/spectrum/aloha-noack-net-device.h
src/helper/adhoc-aloha-noack-ideal-ofdm-helper.h



> On Fri, 2009-04-17 at 11:41 +0200, Nicola Baldo wrote:
> 
>> 1) a class (ValuesVsFreq) providing the representation of 
>> frequency-dependent things (such as spectral power density, propagation 
>> loss, tx/rx mask...);
> 
> a) If you intend to use code in src/devices/spectrum as shared, I think
> you can move it to src/common instead. 

I've moved generic code to src/common, and some other code (e.g., the 
definition of the channel/phy interface) to src/node


> The initial idea beyond
> src/devices was that each subdirectory would contain a single NetDevice
> class. Maybe someone has a better idea.

My code actually does not fit very strictly in this definition, since in 
   src/devices/spectrum there are a PHY and a MAC implementation, and 
both could be used within other devices as well (e.g.: the PHY might be 
used with wifi, and the MAC might be used with a non-spectrum PHY that I 
am planning to write).

However I don't think we can find a generic and clean solution to this 
problem easily...


> 
> b) Multiple inheritance !! What happened to you to become evil
> overnight ? Instead, please, make std::vector<double> a member of
> SetOfFreqs. Make it a public member if you must (or use a struct).
> 

the root of this evil disappeared from the new version of the code ;-)


> c) It's really a bad idea to make arithmetic operators member methods
> (they should be friend functions) because that makes them asymetric (A+B
> becomes different from B+A): you should consider reading "more effective
> c++", items 6, 7, 21, 22. Also:
> http://www.parashift.com/c++-faq-lite/operator-overloading.html#faq-13.9
> 

arithmetic operators are now friend functions.


> Now, given the complexity of getting this kind of C++ operator
> overloading magic right, I would like to suggest that you don't do it at
> all. Yes, I know that we do this in src/simulator/nstime.h but I think I
> can comment about this code to say that doing it once taught me to not
> do it ever again. I don't claim I never make any mistake but I can claim
> that I _try_ to not do the same mistake more than once (ok, maybe,
> sometimes, hubris makes me do the same mistake twice, or more).
> 
> Ok, what could you do to avoid this ? I have to confess that I don't
> know: I am still missing the big picture of your code.

The initial idea was to make it possible to write operations like this:

SpectrumValue signal, noise, interference, sinr;
sinr = signal / (noise + interference);

rather than something like this:

sinr = signal.Divide (noise.Add (interference));	


unfortunately, in the end you are almost always manipulating 
SpectrumValue by means of a Ptr<SpectrumValue>.
This makes the syntax more awkward. See this example:

class SpectrumInterference
{
...
  Ptr<const SpectrumValue> m_rxSignal;
  Ptr<SpectrumValue> m_allSignals;
  Ptr<const SpectrumValue> m_noise;
...
}

SpectrumInterference::ConditionallyEvaluateChunk ()
{
...
  SpectrumValue sinr = (*m_rxSignal) / ((*m_allSignals) - (*m_rxSignal) 
+ (*m_noise));
...
}


I know that this approach is not very appealing, but I don't know any 
better solution either...


> 
>> 2) interfaces definitions for the spectrum-aware channel, phy and 
>> propagation loss model (delay model is the same as wifi);
> 
> d) How do you envision that SpectrumPhy::StartRx would be used ?

have a look at HalfDuplexIdealOfdmPhy::StartRx in this file:
src/devices/spectrum/half-duplex-ideal-ofdm-phy.cc