Bug 2028

Summary:	remove unnecessary Time to double conversions in wifi models
Product:	ns-3	Reporter:	Tom Henderson <tomh>
Component:	wifi	Assignee:	sebastien.deronne
Status:	RESOLVED FIXED
Severity:	normal	CC:	ns-bugs, sebastien.deronne, tommaso.pecorella
Priority:	P5
Version:	ns-3-dev
Hardware:	All
OS:	All
Attachments:	patch to remove unnecessary Time to double conversions in wifi models

Description Tom Henderson 2014-12-12 15:06:31 UTC

This is a new bug for an issue discovered (and initially discussed) in the fix for bug 1969:  https://www.nsnam.org/bugzilla/﷒0﷓.

Some comments from Tommaso in bug 1969 follow:
---
Speaking of, it could be a wise idea to change ALL the functions to NnanoSeconds, i.e.:
- GetPlcpPreambleDurationMicroSeconds (payloadMode, preamble)
- GetPlcpHeaderDurationMicroSeconds (payloadMode, preamble)
- GetPlcpHtSigHeaderDurationMicroSeconds (payloadMode, preamble)
- GetPlcpHtTrainingSymbolDurationMicroSeconds (payloadMode, preamble,txvector)
- GetPayloadDurationMicroSeconds (size, txvector)
and maybe others.

Those should become:
- GetPlcpPreambleDurationNanoSeconds
etc.

If I'm right, the original code idea was to avoid altogether to use a double...

Speaking of... why GetPlcpPreambleDurationMicroSeconds ?

Shouldn't it be GetPlcpPreambleDuration and return a Time ?

WifiPhy::CalculateTxDuration is returning a Time. In that function a bunch of uint32_t are added, then a double (conversion to double), then a "Time" is created.

What is the performance hit to sum a bunch of Time variables instead ?

That would definitely make the code more foolproof and future-feature-proof.
----

so, the fix for this would be a patch that audits the code around this part of the codebase (WifiPhy) and removes unnecessary conversions to/from Time; may require some internal API changes...

Comment 1 sebastien.deronne 2014-12-13 04:13:56 UTC

Is someone already working on that or planned to work on that? 
If not, I may have a look at this issue.

Comment 2 Tommaso Pecorella 2014-12-13 11:40:38 UTC

I'd suggest to take a look at bug 1974 and see if the two can be harmonized.

Comment 3 sebastien.deronne 2014-12-23 17:00:24 UTC

Created attachment 1935 [details]
patch to remove unnecessary Time to double conversions in wifi models

I made a patch for this issue.

It turns the ...DurationMicroSeconds into ...Duration and now return a Time instead of a double. I updated Doxygen as well.

A good point is now that it removes the issues related to short guard intervals!

Note that I also added some additional unit tests in the devices-wifi-tx-duration test suite.

Comment 4 Tommaso Pecorella 2014-12-24 03:27:59 UTC

promoted to bug, since it can cause a real error,

+1 to push (need to rescan bindings).

Comment 5 Tom Henderson 2015-01-04 13:18:26 UTC

pushed in changeset 11119:067928a573cd