Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1107)

Issue 329500043: Wi-Fi adjacent channel modeling through transmit spectrum mask (OFDM) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 1 month ago by Rediet
Modified:
9 months, 1 week ago
Reviewers:
S. Deronne, michael.rademacher, Tom Henderson, Nicola Baldo, skalpaiit, micha.rademacher
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

This patch models the transmit spectrum mask defined for OFDM-based Wi-Fi standards (within the WifiSpectrumValueHelper) so as to be able to model adjacent channel interference. It takes ns-3-dev revision 13114:ba42ffbfbe1e (Oct 12 2017) as reference. Any suggestions / reviews would be much appreciated.

Patch Set 1 : Diff with ns-3-dev rev:13114 #

Total comments: 13

Patch Set 2 : Handle review comments and add unit tests #

Total comments: 58

Patch Set 3 : Patch Set 3: Handle Sebastien's comments on 2nd patch #

Total comments: 2

Patch Set 4 : Handle Sebastien's and S.Kalpalatha's comments concerning previous patchset #

Patch Set 5 : Correct channel width and frequency for non-HT OFDM PSD #

Patch Set 6 : Adapting channel width following Sebastien's recommendations in Bug 2483 #

Total comments: 35

Patch Set 7 : Diff with ns-3-dev rev:13336 (Feb 25th) #

Total comments: 16

Patch Set 8 : Diff with ns-3-dev rev:13352 (March 1st) -> First step (with missing changes) [obsolete] #

Patch Set 9 : Diff with ns-3-dev rev:13352 (March 1st) -> First step: guard band not limited to 20 MHz #

Patch Set 10 : Diff with ns-3-dev rev:13352 (March 1st) -> Second step: guard band limited to 20 MHz #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+1582 lines, -260 lines) Patch
M src/spectrum/model/wifi-spectrum-value-helper.h View 1 2 3 4 5 6 7 8 9 10 chunks +79 lines, -9 lines 0 comments Download
M src/spectrum/model/wifi-spectrum-value-helper.cc View 1 2 3 4 5 6 7 8 9 14 chunks +302 lines, -221 lines 0 comments Download
A src/wifi/examples/wifi-trans-example.cc View 1 1 chunk +311 lines, -0 lines 0 comments Download
A src/wifi/examples/wifi-trans-example.sh View 1 2 3 4 5 6 7 8 9 1 chunk +145 lines, -0 lines 0 comments Download
M src/wifi/examples/wscript View 1 chunk +4 lines, -0 lines 0 comments Download
M src/wifi/model/spectrum-wifi-phy.h View 1 2 3 4 5 6 7 8 9 2 chunks +12 lines, -2 lines 0 comments Download
M src/wifi/model/spectrum-wifi-phy.cc View 1 2 3 4 5 6 7 8 9 6 chunks +36 lines, -25 lines 0 comments Download
M src/wifi/model/wifi-remote-station-manager.cc View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 1 comment Download
M src/wifi/test/spectrum-wifi-phy-test.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/test/wifi-test.cc View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -2 lines 0 comments Download
A src/wifi/test/wifi-transmit-mask-test.cc View 1 2 3 4 5 6 7 8 9 1 chunk +685 lines, -0 lines 0 comments Download
M src/wifi/wscript View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 36
Rediet
Hello Sebastien, Michael, Skalpa, everyone, This patch models the transmit spectrum mask defined for OFDM-based ...
1 year, 1 month ago (2017-10-18 08:35:32 UTC) #1
S. Deronne
Rediet, thanks for your nice job. I am wondering: could you add unit tests? This ...
1 year, 1 month ago (2017-10-24 19:26:05 UTC) #2
S. Deronne
https://codereview.appspot.com/329500043/diff/1/src/wifi/model/spectrum-wifi-phy.cc File src/wifi/model/spectrum-wifi-phy.cc (right): https://codereview.appspot.com/329500043/diff/1/src/wifi/model/spectrum-wifi-phy.cc#newcode381 src/wifi/model/spectrum-wifi-phy.cc:381: guardBandwidth = uint32_t (GetChannelWidth ()); why this? In case ...
1 year, 1 month ago (2017-10-24 19:26:16 UTC) #3
S. Deronne
https://codereview.appspot.com/329500043/diff/1/src/wifi/examples/wifi-trans-gnuplot.plg File src/wifi/examples/wifi-trans-gnuplot.plg (right): https://codereview.appspot.com/329500043/diff/1/src/wifi/examples/wifi-trans-gnuplot.plg#newcode42 src/wifi/examples/wifi-trans-gnuplot.plg:42: #pause -1 Why do you need a separate file? ...
1 year, 1 month ago (2017-10-24 19:27:25 UTC) #4
micha.rademacher
Dear Rediet, Thanks a lot for the effort. Sadly, I am unable to get any ...
1 year, 1 month ago (2017-10-25 13:16:19 UTC) #5
micha.rademacher
Some c++ compile erros. https://codereview.appspot.com/329500043/diff/1/src/spectrum/model/wifi-spectrum-value-helper.cc File src/spectrum/model/wifi-spectrum-value-helper.cc (right): https://codereview.appspot.com/329500043/diff/1/src/spectrum/model/wifi-spectrum-value-helper.cc#newcode249 src/spectrum/model/wifi-spectrum-value-helper.cc:249: Values::iterator vit = c->ValuesBegin (); ...
1 year, 1 month ago (2017-10-25 13:16:47 UTC) #6
Rediet
On 2017/10/24 19:26:05, S. Deronne wrote: > Rediet, thanks for your nice job. > I ...
1 year, 1 month ago (2017-10-30 14:54:32 UTC) #7
Rediet
Thanks Sébastien, thanks Michael. https://codereview.appspot.com/329500043/diff/1/src/spectrum/model/wifi-spectrum-value-helper.cc File src/spectrum/model/wifi-spectrum-value-helper.cc (right): https://codereview.appspot.com/329500043/diff/1/src/spectrum/model/wifi-spectrum-value-helper.cc#newcode249 src/spectrum/model/wifi-spectrum-value-helper.cc:249: Values::iterator vit = c->ValuesBegin (); ...
1 year, 1 month ago (2017-10-30 15:07:57 UTC) #8
Rediet
On 2017/10/25 13:16:19, micha.rademacher wrote: > Dear Rediet, > > Thanks a lot for the ...
1 year, 1 month ago (2017-10-30 15:13:34 UTC) #9
Rediet
https://codereview.appspot.com/329500043/diff/1/src/wifi/examples/wifi-trans-gnuplot.plg File src/wifi/examples/wifi-trans-gnuplot.plg (right): https://codereview.appspot.com/329500043/diff/1/src/wifi/examples/wifi-trans-gnuplot.plg#newcode42 src/wifi/examples/wifi-trans-gnuplot.plg:42: #pause -1 On 2017/10/30 15:07:57, Rediet wrote: > On ...
1 year, 1 month ago (2017-10-30 16:21:42 UTC) #10
Rediet
On 2017/10/30 15:13:34, Rediet wrote: > On 2017/10/25 13:16:19, micha.rademacher wrote: > > Dear Rediet, ...
1 year, 1 month ago (2017-10-31 12:52:09 UTC) #11
Rediet
Dear Sébastien, Michael, Here is a second patch taking into account your comments/corrections, namely: - ...
1 year, 1 month ago (2017-11-07 13:05:52 UTC) #12
S. Deronne
Thanks, I still need to find some time to review the helper class itself. Can ...
1 year, 1 month ago (2017-11-07 19:08:22 UTC) #13
Rediet
Thanks Sébastien. My replies below. https://codereview.appspot.com/329500043/diff/20001/src/wifi/test/wifi-test-spectrum-value-h.cc File src/wifi/test/wifi-test-spectrum-value-h.cc (right): https://codereview.appspot.com/329500043/diff/20001/src/wifi/test/wifi-test-spectrum-value-h.cc#newcode30 src/wifi/test/wifi-test-spectrum-value-h.cc:30: NS_LOG_COMPONENT_DEFINE ("WifiTestSpectrumValueHelper"); On 2017/11/07 ...
1 year, 1 month ago (2017-11-08 10:51:41 UTC) #14
S. Deronne
https://codereview.appspot.com/329500043/diff/20001/src/spectrum/model/wifi-spectrum-value-helper.cc File src/spectrum/model/wifi-spectrum-value-helper.cc (right): https://codereview.appspot.com/329500043/diff/20001/src/spectrum/model/wifi-spectrum-value-helper.cc#newcode175 src/spectrum/model/wifi-spectrum-value-helper.cc:175: double lowestPointDbr = -40.0; this is not needed in ...
1 year, 1 month ago (2017-11-09 07:15:17 UTC) #15
Rediet
Dear Sébastien, Thanks for your comments (and sorry for the delay, just managed to find ...
1 year ago (2017-11-22 16:22:39 UTC) #16
S. Deronne
https://codereview.appspot.com/329500043/diff/20001/src/spectrum/model/wifi-spectrum-value-helper.cc File src/spectrum/model/wifi-spectrum-value-helper.cc (right): https://codereview.appspot.com/329500043/diff/20001/src/spectrum/model/wifi-spectrum-value-helper.cc#newcode175 src/spectrum/model/wifi-spectrum-value-helper.cc:175: double lowestPointDbr = -40.0; I agree with those values, ...
1 year ago (2017-11-22 16:53:20 UTC) #17
Rediet
Dear Sébastien, Thank you for your review of the second patch and sorry for the ...
1 year ago (2017-12-07 15:07:47 UTC) #18
S. Deronne
Rediet, thanks for your new patchset, I have some small remarks, but I did not ...
1 year ago (2017-12-09 16:24:11 UTC) #19
skalpaiit
Dear Rediet Thanks for patch and all the recent updates and modifications. Here are some ...
12 months ago (2017-12-15 08:36:11 UTC) #20
Rediet
Dear Sébastien, S.Kalpalatha, Nicola, Thanks for your comments Sébastien and S.Kalpalatha. Nicola could you please ...
12 months ago (2017-12-15 15:05:44 UTC) #21
Rediet
Dear Sébastien, I've updated the patch with bug's 2843's corrections. I think I've covered all ...
11 months, 1 week ago (2018-01-05 11:10:06 UTC) #22
Rediet
Updated version following Sebastien's recommendation in bug 2483.
11 months ago (2018-01-12 14:31:35 UTC) #23
S. Deronne
Rediet, could you please merge with ns-3-dev? I see already a lot of wifi changes ...
9 months, 2 weeks ago (2018-02-24 09:57:29 UTC) #24
S. Deronne
https://codereview.appspot.com/329500043/diff/100001/examples/wireless/wifi-spectrum-saturation-example.cc File examples/wireless/wifi-spectrum-saturation-example.cc (right): https://codereview.appspot.com/329500043/diff/100001/examples/wireless/wifi-spectrum-saturation-example.cc#newcode202 examples/wireless/wifi-spectrum-saturation-example.cc:202: = CreateObject<MultiModelSpectrumChannel> (); On 2018/02/24 09:57:28, S. Deronne wrote: ...
9 months, 2 weeks ago (2018-02-24 09:59:39 UTC) #25
Rediet
Hi Sébastien, This new patchset takes the latest changeset (13336 of Feb 25th) as reference ...
9 months, 2 weeks ago (2018-02-26 16:22:35 UTC) #26
S. Deronne
https://codereview.appspot.com/329500043/diff/120001/src/wifi/model/spectrum-wifi-phy.cc File src/wifi/model/spectrum-wifi-phy.cc (right): https://codereview.appspot.com/329500043/diff/120001/src/wifi/model/spectrum-wifi-phy.cc#newcode395 src/wifi/model/spectrum-wifi-phy.cc:395: case WIFI_PHY_STANDARD_80211b: I do not think spectrum does model ...
9 months, 2 weeks ago (2018-02-26 22:37:58 UTC) #27
Rediet
https://codereview.appspot.com/329500043/diff/120001/src/wifi/model/spectrum-wifi-phy.cc File src/wifi/model/spectrum-wifi-phy.cc (right): https://codereview.appspot.com/329500043/diff/120001/src/wifi/model/spectrum-wifi-phy.cc#newcode395 src/wifi/model/spectrum-wifi-phy.cc:395: case WIFI_PHY_STANDARD_80211b: On 2018/02/26 22:37:57, S. Deronne wrote: > ...
9 months, 2 weeks ago (2018-02-27 06:56:55 UTC) #28
Tom Henderson
Thank you for this thorough patch; my only main comment/concern is setting guard BW to ...
9 months, 1 week ago (2018-03-01 21:43:06 UTC) #29
Rediet
Hi Tom, Thanks for your review. Taking into account the neighboring bands (even when >20 ...
9 months, 1 week ago (2018-03-02 10:25:42 UTC) #30
Rediet
Here's the first version (taking into account all comments except limitation of guard band) wrt ...
9 months, 1 week ago (2018-03-02 11:09:00 UTC) #31
Tom Henderson
On 2018/03/02 10:25:42, Rediet wrote: > Hi Tom, > > Thanks for your review. > ...
9 months, 1 week ago (2018-03-02 14:30:24 UTC) #32
Rediet
On 2018/03/02 14:30:24, Tom Henderson wrote: > Rediet, my comment was based on the assumption ...
9 months, 1 week ago (2018-03-02 15:45:58 UTC) #33
S. Deronne
What is the current status of the latest patchset? We should decide to push this ...
9 months, 1 week ago (2018-03-03 20:12:26 UTC) #34
Tom Henderson
On 2018/03/03 20:12:26, S. Deronne wrote: > What is the current status of the latest ...
9 months, 1 week ago (2018-03-04 01:16:20 UTC) #35
S. Deronne
9 months, 1 week ago (2018-03-04 09:07:24 UTC) #36
On 2018/03/04 01:16:20, Tom Henderson wrote:
> On 2018/03/03 20:12:26, S. Deronne wrote:
> > What is the current status of the latest patchset? We should decide to push
> this
> > on Sunday so that Tom can prepare a RC on Monday.
> 
> I'm fine with patchset 9; we can consider the optimization of patchset 10 if
> patchset 9 proves to be a performance problem at some point, I suppose.

OK, I will push changeset 9 to ns-3-dev
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b