Bug 2007 - SetRxThresholdDb is marked deprecated but is used by the model
SetRxThresholdDb is marked deprecated but is used by the model
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: uan
pre-release
PC Linux
: P5 normal
Assigned To: Federico Guerra
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-14 12:40 UTC by Tom Henderson
Modified: 2016-10-07 02:07 UTC (History)
2 users (show)

See Also:


Attachments
proposed fix (1.29 KB, patch)
2016-07-10 10:55 UTC, Federico Guerra
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Henderson 2014-10-14 12:40:22 UTC
in uan-phy-dual.h, we have

  virtual void SetRxThresholdDb (double thresh) NS_DEPRECATED;

void
UanPhyDual::SetRxThresholdDb (double thresh)
{
  NS_LOG_WARN ("SetRxThresholdDb is deprecated and has no effect.  Look at PER Functor attribute");
  m_phy1->SetRxThresholdDb (thresh);
  m_phy2->SetRxThresholdDb (thresh);
}

However, this pure virtual function (declared in uan-phy.h) is used in UanPhyGen, which itself is used by UanPhyDual, so the statement that it has no effect seems to be incorrect (the variable m_rxThreshDb is used in UanPhyGen::StartRxPacket():

       double newsinr = CalculateSinrDb (pkt, Simulator::Now (), rxPowerDb, txMode, pdp);
        NS_LOG_DEBUG ("PHY " << m_mac->GetAddress () << ": Starting RX in IDLE mode.  SINR = " << newsinr);
        if (newsinr > m_rxThreshDb)
  ... 


So, either the model needs to be fixed or the comments that the method is deprecated need to be adjusted.
Comment 1 Tommaso Pecorella 2014-10-14 16:53:51 UTC
Due to the peculiar use, I'd un-deprecate the function.

I'm not aware about why it was deprecated in first place, but my best guess is that the getter and setters have been deprecated because all the underlying variables can be changed by resorting to standard attributes.
If this is the case, then we'd have to remove a LOT of functions, not just that one.
Comment 2 Tom Henderson 2014-10-14 17:20:13 UTC
(In reply to Tommaso Pecorella from comment #1)
> Due to the peculiar use, I'd un-deprecate the function.
> 
> I'm not aware about why it was deprecated in first place, but my best guess
> is that the getter and setters have been deprecated because all the
> underlying variables can be changed by resorting to standard attributes.
> If this is the case, then we'd have to remove a LOT of functions, not just
> that one.


I believe it is not so simple; the comments in the code suggest that the functionality has been replaced by the PER functor, but apparently not completely (as a result, I don't know whether it is a bug).
Comment 3 Tommaso Pecorella 2014-10-14 17:49:58 UTC
(In reply to Tom Henderson from comment #2)
> (In reply to Tommaso Pecorella from comment #1)
> > Due to the peculiar use, I'd un-deprecate the function.
> > 
> > I'm not aware about why it was deprecated in first place, but my best guess
> > is that the getter and setters have been deprecated because all the
> > underlying variables can be changed by resorting to standard attributes.
> > If this is the case, then we'd have to remove a LOT of functions, not just
> > that one.
> 
> 
> I believe it is not so simple; the comments in the code suggest that the
> functionality has been replaced by the PER functor, but apparently not
> completely (as a result, I don't know whether it is a bug).

They seems to be different things. One is the error rate calculator, the other is the Rx sensitivity value.
We had a long discussion with Sascha about this for 802.15.4, where the standard states that the sensitivity depends on the receiver's hardware (and must be measured). In other devices, you may assume that the sensitivity is a construction param.
Anyway, it's when the receiver will start thinking that the signal its receiving isn't thermal noise and should switch from idle to Rx.

Anyway, those are guesses, I haven't studied the module for more than 10 mins.
Comment 4 Federico Guerra 2016-05-11 05:33:10 UTC
Hi All,
I've checked the code and I agree with Tommaso.

those functions set and get the rx sinr threshold (aka sensitivity) in order to start the reception. the concept is correct and it is also used in other underwater models (i.e. NS-miracle underwater :) )

Once the rx is ended the PER object is using a the current packet SINR (m_minRxSinrDb) in order to calculate the PER.
the packet is dropped if the random value is <= of the computed PER.

long story short I think that they should not be deprecated
Comment 5 Federico Guerra 2016-07-10 10:55:37 UTC
Created attachment 2491 [details]
proposed fix
Comment 6 Federico Guerra 2016-10-04 09:29:26 UTC
Hi Tom,
has this fix included in ns3.26 release?
thanks
Federico
Comment 7 Tom Henderson 2016-10-05 01:00:30 UTC
(In reply to Federico Guerra from comment #6)
> Hi Tom,
> has this fix included in ns3.26 release?
> thanks
> Federico

No, sorry; I can push it shortly though.
Comment 8 Tom Henderson 2016-10-07 02:07:41 UTC
pushed in 12364:bcfe863658a7