Bug 2368 - code review: wifi frame capture model
code review: wifi frame capture model
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: wifi
pre-release
All All
: P5 enhancement
Assigned To: sebastien.deronne
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-04-07 13:46 EDT by Tom Henderson
Modified: 2017-05-06 12:03 EDT (History)
4 users (show)

See Also:


Attachments
documentation (pages 7-15) (771.39 KB, application/pdf)
2016-04-07 13:49 EDT, Tom Henderson
Details
Version 3.25 compatible code (8.28 KB, patch)
2016-06-28 13:26 EDT, Ali Rostami
Details | Diff
Modified Interference Helper Example for Testing Purpose (14.08 KB, text/plain)
2016-06-28 13:28 EDT, Ali Rostami
Details
Frame Capture Model - ns-3.25 compatible code (bzip) (8.40 KB, patch)
2016-07-02 14:39 EDT, Ali Rostami
Details | Diff
PER vs. Distance bins for the simulation with/without frame capture model (17.82 KB, image/esp)
2016-10-24 17:15 EDT, Ali Rostami
Details
Frame capture implementation based ns-3-dev: implements the frame capture effect using the default ns-3 model and the proposed frame capture model (35.38 KB, patch)
2017-01-02 20:04 EST, Bin Cheng
Details | Diff
Updated frame capture patch (30.00 KB, patch)
2017-01-05 00:26 EST, Bin Cheng
Details | Diff
frame-capture-patch v3 (41.51 KB, patch)
2017-01-24 19:46 EST, Bin Cheng
Details | Diff
Updated patch to fix the regression test failure (49.06 KB, patch)
2017-01-27 19:06 EST, Bin Cheng
Details | Diff
Patch remove per check when determining whether the reception can be continue (68.84 KB, patch)
2017-02-18 17:07 EST, Bin Cheng
Details | Diff
New proposal based on rx power comparison (28.50 KB, patch)
2017-03-06 16:49 EST, sebastien.deronne
Details | Diff
new patch addressing Tom's comments (37.50 KB, patch)
2017-03-29 17:18 EDT, sebastien.deronne
Details | Diff
new patch that includes a test in the regression (45.35 KB, patch)
2017-04-09 15:19 EDT, sebastien.deronne
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Henderson 2016-04-07 13:46:53 EDT
Posted on behalf of Ali Rostami.

http://codereview.appspot.com/292300043

This code is from a much older version of ns-3 and I did my best to copy/paste it to the right places in ns-3-dev, but it does not compile and would need to be ported to ns-3-dev since there have been many changes to wifi module since then.

Please read pages 7-15 of the attached PDF for documentation.
Comment 1 Tom Henderson 2016-04-07 13:49:26 EDT
Created attachment 2375 [details]
documentation (pages 7-15)
Comment 2 sebastien.deronne 2016-04-16 04:02:23 EDT
I did not went through all the document yet, but I had read so far that frame capture mode could only occur during the preamble phase and not during the payload part.

Do you have any reference that show that payload capture is implemented in the field?
Comment 3 Ali Rostami 2016-04-19 17:53:59 EDT
(In reply to sebastien.deronne from comment #2)
> I did not went through all the document yet, but I had read so far that
> frame capture mode could only occur during the preamble phase and not during
> the payload part.
> 
> Do you have any reference that show that payload capture is implemented in
> the field?

Hi Sebastien,

To implement this part, CAMP used some help from a hardware company in contract with them. That company could have implemented this successfully, but unfortunately, I can't provide any reference for that task. But, I can point out to one of the paper that experimentally evaluate the frame capture mode.

Lee, Jeongkeun, et al. "An experimental study on the capture effect in 802.11 a networks." Proceedings of the second ACM international workshop on Wireless network testbeds, experimental evaluation and characterization. ACM, 2007.
Comment 4 Ali Rostami 2016-04-19 18:00:08 EDT
Between, thanks Tom for posting these patches.
I will take a look to port the current code to the latest version of the ns3 soon. I'll post here whenever I got a compilable code.
Comment 5 Ali Rostami 2016-06-10 13:23:01 EDT
I am almost done with porting the code to the newest version. I'm waiting for my teammate's approval before releasing the code. I'm expecting to post the final version sometime during next week. Thanks.
Comment 6 Ali Rostami 2016-06-28 13:25:39 EDT
Tom, Sebastien,

We finally ported the code for ns-3.25. My colleague, Bin Cheng, also modified the example script "test-interference-helper.cc" a bit for testing purpose. We tested the code and it seems to be working fine.

Please find the attached tarball for the code. I also tried to modify the codereview files but I didn't have the write permission, so, you may want to replace those files as well.

Please let me know if you have any further questions. Bin also sees the comments/questions and may be able to answer questions, if any.

Best,
-Ali
Comment 7 Ali Rostami 2016-06-28 13:26:46 EDT
Created attachment 2487 [details]
Version 3.25 compatible code
Comment 8 Ali Rostami 2016-06-28 13:28:38 EDT
Created attachment 2488 [details]
Modified Interference Helper Example for Testing Purpose
Comment 9 sebastien.deronne 2016-06-29 14:36:44 EDT
Thanks, I won't have time to have a look before the upcoming release, but I'll do my best to review it for ns-3.27
Comment 10 sebastien.deronne 2016-07-02 06:12:51 EDT
There seems to be a problem with attachment, could you please upload it again?
Comment 11 Ali Rostami 2016-07-02 14:39:42 EDT
Created attachment 2490 [details]
Frame Capture Model - ns-3.25 compatible code (bzip)

Done
Comment 12 sebastien.deronne 2016-10-24 12:52:37 EDT
Ali, I have some more questions before going through the review...

What are the differences between figures 2 and 3? 
Is figure 2 used for the first arriving signal and 3 for the second arriving signal?
                  
Are those curves obtained for 802.11a? b? g? n? ac?

You mention a shifted version of the ns-3 probability model, which one exactly?
And why for 3Mbit/s case?

Which conditions did you consider to obtain your model?
This looks quite empirical, is your implemented model generic enough to be adapted to any other conditions?

Could you explain what are the differences between figure 5 and 6? 
What do they refer to exactly?
Comment 13 Ali Rostami 2016-10-24 14:01:24 EDT
(In reply to sebastien.deronne from comment #12)
> Ali, I have some more questions before going through the review...
> 
> What are the differences between figures 2 and 3? 
> Is figure 2 used for the first arriving signal and 3 for the second arriving
> signal?

Figure 2 is used for a situation illustrated in Figure 1, which means the curves from Fig 2 and 3 are being used if the arrival of the second frame is withing the duration of the first frame preamble receiving. Now, when such a situation happens, the device first tries to recover the rest of the frame A with the probability model obtained from Figure 2 (which we call it DECODING in the document). If wasn't successful in terms of the additional interference because of frame B, the device drops frame A and it tries to capture frame B using a probability model obtained from Figure 3 (which we call it CAPTURE here).

Same thing happens when frame B arrives withing the duration which the device is receiving the PAYLOAD part of the frame A (Figure 4). In that case, Figures 5 and 6 are used, instead of Figures 2 and 3.

>                   
> Are those curves obtained for 802.11a? b? g? n? ac?

These curves are obtained from lab tests of actual hardware for 802.11p by their vendors.

> 
> You mention a shifted version of the ns-3 probability model, which one
> exactly?
> And why for 3Mbit/s case?

The reason we mentioned a shifted version is because ns3 already uses probability model in comparison to ns2. The main contribution here is not introducing the probability based receive model, but introducing the frame capture model + calibrating the curves for 802.11p channels using lab tests.

As you know, 802.11p uses (mostly) 10 MHz channels, which makes the preamble datarate 3Mbps, still using 6Mbps for the payload. This is why we shifted these two curves obtained from ns3.

> 
> Which conditions did you consider to obtain your model?
> This looks quite empirical, is your implemented model generic enough to be
> adapted to any other conditions?

I am not sure if I understand what you mean by "condition" here. However, I think we can consider these results robust enough since these results are done by two hardware vendors. Furthermore, as far as I know, these curves are containing  hardware-specific behavior and shouldn't be changed under different conditions.

> 
> Could you explain what are the differences between figure 5 and 6? 
> What do they refer to exactly?

I answered this in your first question.


Please don't hesitate if you had further questions, and I'll try to get back to you asap.
Comment 14 sebastien.deronne 2016-10-24 14:11:04 EDT
(In reply to Ali Rostami from comment #13)
> (In reply to sebastien.deronne from comment #12)
> > Ali, I have some more questions before going through the review...
> > 
> > What are the differences between figures 2 and 3? 
> > Is figure 2 used for the first arriving signal and 3 for the second arriving
> > signal?
> 
> Figure 2 is used for a situation illustrated in Figure 1, which means the
> curves from Fig 2 and 3 are being used if the arrival of the second frame is
> withing the duration of the first frame preamble receiving. Now, when such a
> situation happens, the device first tries to recover the rest of the frame A
> with the probability model obtained from Figure 2 (which we call it DECODING
> in the document). If wasn't successful in terms of the additional
> interference because of frame B, the device drops frame A and it tries to
> capture frame B using a probability model obtained from Figure 3 (which we
> call it CAPTURE here).
> 
> Same thing happens when frame B arrives withing the duration which the
> device is receiving the PAYLOAD part of the frame A (Figure 4). In that
> case, Figures 5 and 6 are used, instead of Figures 2 and 3.
> 

Why do we need different curves? 
It is in both cases to decode the preamble part, or do I miss something?

> >                   
> > Are those curves obtained for 802.11a? b? g? n? ac?
> 
> These curves are obtained from lab tests of actual hardware for 802.11p by
> their vendors.

How can those curves be adapted to fit other 802.11 standards??

> 
> > 
> > You mention a shifted version of the ns-3 probability model, which one
> > exactly?
> > And why for 3Mbit/s case?
> 
> The reason we mentioned a shifted version is because ns3 already uses
> probability model in comparison to ns2. The main contribution here is not
> introducing the probability based receive model, but introducing the frame
> capture model + calibrating the curves for 802.11p channels using lab tests.
> 
> As you know, 802.11p uses (mostly) 10 MHz channels, which makes the preamble
> datarate 3Mbps, still using 6Mbps for the payload. This is why we shifted
> these two curves obtained from ns3.

Ok.

> 
> > 
> > Which conditions did you consider to obtain your model?
> > This looks quite empirical, is your implemented model generic enough to be
> > adapted to any other conditions?
> 
> I am not sure if I understand what you mean by "condition" here. However, I
> think we can consider these results robust enough since these results are
> done by two hardware vendors. Furthermore, as far as I know, these curves
> are containing  hardware-specific behavior and shouldn't be changed under
> different conditions.

I am a bit confused: are those results from lab trials performed with several hardwares or are those results directly provided by different vendors?
 
> 
> > 
> > Could you explain what are the differences between figure 5 and 6? 
> > What do they refer to exactly?
> 
> I answered this in your first question.
> 
> 
> Please don't hesitate if you had further questions, and I'll try to get back
> to you asap.

Thanks for replying so quickly, I want to have it clear in my mind before getting through the review of the code itself.
Comment 15 Ali Rostami 2016-10-24 17:12:46 EDT
(In reply to sebastien.deronne from comment #14)
> (In reply to Ali Rostami from comment #13)
> > (In reply to sebastien.deronne from comment #12)
> > > Ali, I have some more questions before going through the review...
> > > 
> > > What are the differences between figures 2 and 3? 
> > > Is figure 2 used for the first arriving signal and 3 for the second arriving
> > > signal?
> > 
> > Figure 2 is used for a situation illustrated in Figure 1, which means the
> > curves from Fig 2 and 3 are being used if the arrival of the second frame is
> > withing the duration of the first frame preamble receiving. Now, when such a
> > situation happens, the device first tries to recover the rest of the frame A
> > with the probability model obtained from Figure 2 (which we call it DECODING
> > in the document). If wasn't successful in terms of the additional
> > interference because of frame B, the device drops frame A and it tries to
> > capture frame B using a probability model obtained from Figure 3 (which we
> > call it CAPTURE here).
> > 
> > Same thing happens when frame B arrives withing the duration which the
> > device is receiving the PAYLOAD part of the frame A (Figure 4). In that
> > case, Figures 5 and 6 are used, instead of Figures 2 and 3.
> > 
> 
> Why do we need different curves? 
> It is in both cases to decode the preamble part, or do I miss something?

The different curves in Figures 2 and 3 are for different frames. Figure 2 implies that lower SINR is enough to decode (resume) the rest of the first frame (frame A) when frame B arrives. If the model fails to resume receiving of A, then it switches to the newly arrived frame (frame B), but it needs more SINR to start synching with B's preamble (the curve in Figure 3).

> 
> > >                   
> > > Are those curves obtained for 802.11a? b? g? n? ac?
> > 
> > These curves are obtained from lab tests of actual hardware for 802.11p by
> > their vendors.
> 
> How can those curves be adapted to fit other 802.11 standards??

I would say that we will need to conduct more tests with the rest of the 802.11 variations.
It might help to mention that the focus of this patch contribution was not calibrating the probability model for different 802.11 standards. In these patches, we have focused on the logic of the frame capture model. As a result of our work, we happened to obtain pretty accurate curves from the lab test during our project and we provided it with the patches.

Btw, We recently have published a paper which within we briefly explain the effect of the frame capture model on the accuracy of the simulation results for shorter links http://winlab.rutgers.edu/~rostami/files/pdf/rostami2016performance.pdf
I also attached the graph for Packet Error Ratio Vs. Distance bins for the exact same simulation configurations except that one of them are using the default ns3 receive model, and the other used the frame capture model.
 

> 
> > 
> > > 
> > > You mention a shifted version of the ns-3 probability model, which one
> > > exactly?
> > > And why for 3Mbit/s case?
> > 
> > The reason we mentioned a shifted version is because ns3 already uses
> > probability model in comparison to ns2. The main contribution here is not
> > introducing the probability based receive model, but introducing the frame
> > capture model + calibrating the curves for 802.11p channels using lab tests.
> > 
> > As you know, 802.11p uses (mostly) 10 MHz channels, which makes the preamble
> > datarate 3Mbps, still using 6Mbps for the payload. This is why we shifted
> > these two curves obtained from ns3.
> 
> Ok.
> 
> > 
> > > 
> > > Which conditions did you consider to obtain your model?
> > > This looks quite empirical, is your implemented model generic enough to be
> > > adapted to any other conditions?
> > 
> > I am not sure if I understand what you mean by "condition" here. However, I
> > think we can consider these results robust enough since these results are
> > done by two hardware vendors. Furthermore, as far as I know, these curves
> > are containing  hardware-specific behavior and shouldn't be changed under
> > different conditions.
> 
> I am a bit confused: are those results from lab trials performed with
> several hardwares or are those results directly provided by different
> vendors?

These results are coming from one of the vendors which is one of the contractors for the tests conducted under USDoT and CAMP supervision.

>  
> > 
> > > 
> > > Could you explain what are the differences between figure 5 and 6? 
> > > What do they refer to exactly?
> > 
> > I answered this in your first question.
> > 
> > 
> > Please don't hesitate if you had further questions, and I'll try to get back
> > to you asap.
> 
> Thanks for replying so quickly, I want to have it clear in my mind before
> getting through the review of the code itself.

No worries. Let me know if more questions came up.
Comment 16 Ali Rostami 2016-10-24 17:15:31 EDT
Created attachment 2636 [details]
PER vs. Distance bins for the simulation with/without frame capture model
Comment 17 Bin Cheng 2016-10-24 19:12:57 EDT
(In reply to sebastien.deronne from comment #14)
> (In reply to Ali Rostami from comment #13)
> > (In reply to sebastien.deronne from comment #12)
> > > Ali, I have some more questions before going through the review...
> > > 
> > > What are the differences between figures 2 and 3? 
> > > Is figure 2 used for the first arriving signal and 3 for the second arriving
> > > signal?
> > 
> > Figure 2 is used for a situation illustrated in Figure 1, which means the
> > curves from Fig 2 and 3 are being used if the arrival of the second frame is
> > withing the duration of the first frame preamble receiving. Now, when such a
> > situation happens, the device first tries to recover the rest of the frame A
> > with the probability model obtained from Figure 2 (which we call it DECODING
> > in the document). If wasn't successful in terms of the additional
> > interference because of frame B, the device drops frame A and it tries to
> > capture frame B using a probability model obtained from Figure 3 (which we
> > call it CAPTURE here).
> > 
> > Same thing happens when frame B arrives withing the duration which the
> > device is receiving the PAYLOAD part of the frame A (Figure 4). In that
> > case, Figures 5 and 6 are used, instead of Figures 2 and 3.
> > 
> 
> Why do we need different curves? 
> It is in both cases to decode the preamble part, or do I miss something?
> 
> > >                   
> > > Are those curves obtained for 802.11a? b? g? n? ac?
> > 
> > These curves are obtained from lab tests of actual hardware for 802.11p by
> > their vendors.
> 
> How can those curves be adapted to fit other 802.11 standards??
> 
> > 
> > > 
> > > You mention a shifted version of the ns-3 probability model, which one
> > > exactly?
> > > And why for 3Mbit/s case?
> > 
> > The reason we mentioned a shifted version is because ns3 already uses
> > probability model in comparison to ns2. The main contribution here is not
> > introducing the probability based receive model, but introducing the frame
> > capture model + calibrating the curves for 802.11p channels using lab tests.
> > 
> > As you know, 802.11p uses (mostly) 10 MHz channels, which makes the preamble
> > datarate 3Mbps, still using 6Mbps for the payload. This is why we shifted
> > these two curves obtained from ns3.
> 
> Ok.
> 
> > 
> > > 
> > > Which conditions did you consider to obtain your model?
> > > This looks quite empirical, is your implemented model generic enough to be
> > > adapted to any other conditions?
> > 
> > I am not sure if I understand what you mean by "condition" here. However, I
> > think we can consider these results robust enough since these results are
> > done by two hardware vendors. Furthermore, as far as I know, these curves
> > are containing  hardware-specific behavior and shouldn't be changed under
> > different conditions.
> 
> I am a bit confused: are those results from lab trials performed with
> several hardwares or are those results directly provided by different
> vendors?
>  
> > 
> > > 
> > > Could you explain what are the differences between figure 5 and 6? 
> > > What do they refer to exactly?
> > 
> > I answered this in your first question.
> > 
> > 
> > Please don't hesitate if you had further questions, and I'll try to get back
> > to you asap.
> 
> Thanks for replying so quickly, I want to have it clear in my mind before
> getting through the review of the code itself.

Hi Sebastien,

I am Bin Cheng. I am also a contributor of this patch. Thank you for starting reviewing our patch and asking these questions.

Please let me provide more background information which may help you to better understand our patch and its focus. This patch is an outcome of a multiple-year project which focuses on accurately simulating vehicle-vehicle communication networks, specifically DSRC (which uses 802.11p as MAC and PHY layer protocol) networks. Therefore, all the proposed models in this patch are primarily calibrated for the IEEE 802.11p standard, i.e., for the OFDM signals with 10MHz bandwidth, 6Mbps user data rate for the packet payload (the modulation mode is QPSK) and 3Mbps data rate for the packet preamble (the modulation is BPSK).  To be honest, we are not sure how accuracy the simulation results will be when using the proposed models for other IEEE 802.11 standards with a different data rate or a different bandwidth. However, we would like to say that the framework we are proposing for handling the frame capture may be very useful. This framework was largely inspired by the frame capture implementation in ns-2 [1] and also Bingmann's implementation in ns-3 [2]. Different from the threshold-based models in these implementations, we used a probability-based model which can capture these transition zones from unable-capture to successful-capture, as depicted in figure 2-3 and figure 5-6. We believe these transition zones are the important nature when modeling the device's behaviors. 

Regarding the modeling details, these models were essentially fitted and calibrated based on the data from a set of hardware lab tests. These tests were conducted by very experienced engineers from a major chip company in U.S. and the focuses of these tests were to clarify some of the radio characteristics of the DSRC transceivers and to help to determine the correct values for these parameters. However, as the frame capture, the hardware tests were only able to provide data for the payload capture (or data capture) not for the preamble capture. This is the major reason why we used a shifted ns-3 NIST error model for the preamble capture. We then tried different levels of shifts on the model and select the one which produces the best match to the field test results. 

Regarding why we always use two models, one for so-called decoding and one for so-called capture, I think Ali has explained very well. Basically, the decoding models are trying to model that given the current SINR level, what the probability that the currently received signal can still be successfully recovered. The capture models are trying to model that given the current SINR level, what the probability that the currently received signal is dropped and the receiver switches to the newly arrived signal. However, for the preamble part, it may be tricky. It is because that during the preamble period, the receiver has not locked to any signal yet (different receivers may have different behaviors, some receivers only need a few short training symbol to lock the signal but some receivers may need long training symbols to do so), so it may not be very accurate to say this is "decoding" or "capture". But we simplify this process and say that from the very beginning of the packet, the receiver is locked to that packet. 

One more thing to note is that our models are primarily based on relationships between SINR and decoding/capture probability (let us call this SINR vs PDR). It is very different from NIST model which is BER based. May I ask what your plan is to enable frame capture effect in the future release, do you want to keep using NIST model or do you want to simplify it to use models with SINR vs PDR? I tried another implementation of the frame capture which keeps using the current BER-based structure. We can have more discussion on this in the future.

Thanks again for your help, and please let us know if you have more questions.

Best, 
Bin


[1] Q. Chen, F. Schmidt-Eisenlohr, D. Jiang,M. Torrent-Moreno, L. Delgrossi, and H. Hartenstein.Overhaul of ieee 802.11 modeling and simulation in ns-2. In Proceedings of the 10th ACM Symposium on Modeling, Analysis, and Simulation of Wireless and Mobile Systems, MSWiM ’07, pages 159{168. ACM, 2007 
[2]  T. Bingmann. Accuracy enhancements of the 802.11 model and edca qos extensions in ns-3. Master’s thesis, University of Karlsruhe, 2009.
Comment 18 sebastien.deronne 2016-10-30 04:28:56 EDT
(In reply to Bin Cheng from comment #17)
> (In reply to sebastien.deronne from comment #14)
> > (In reply to Ali Rostami from comment #13)
> > > (In reply to sebastien.deronne from comment #12)
> > > > Ali, I have some more questions before going through the review...
> > > > 
> > > > What are the differences between figures 2 and 3? 
> > > > Is figure 2 used for the first arriving signal and 3 for the second arriving
> > > > signal?
> > > 
> > > Figure 2 is used for a situation illustrated in Figure 1, which means the
> > > curves from Fig 2 and 3 are being used if the arrival of the second frame is
> > > withing the duration of the first frame preamble receiving. Now, when such a
> > > situation happens, the device first tries to recover the rest of the frame A
> > > with the probability model obtained from Figure 2 (which we call it DECODING
> > > in the document). If wasn't successful in terms of the additional
> > > interference because of frame B, the device drops frame A and it tries to
> > > capture frame B using a probability model obtained from Figure 3 (which we
> > > call it CAPTURE here).
> > > 
> > > Same thing happens when frame B arrives withing the duration which the
> > > device is receiving the PAYLOAD part of the frame A (Figure 4). In that
> > > case, Figures 5 and 6 are used, instead of Figures 2 and 3.
> > > 
> > 
> > Why do we need different curves? 
> > It is in both cases to decode the preamble part, or do I miss something?
> > 
> > > >                   
> > > > Are those curves obtained for 802.11a? b? g? n? ac?
> > > 
> > > These curves are obtained from lab tests of actual hardware for 802.11p by
> > > their vendors.
> > 
> > How can those curves be adapted to fit other 802.11 standards??
> > 
> > > 
> > > > 
> > > > You mention a shifted version of the ns-3 probability model, which one
> > > > exactly?
> > > > And why for 3Mbit/s case?
> > > 
> > > The reason we mentioned a shifted version is because ns3 already uses
> > > probability model in comparison to ns2. The main contribution here is not
> > > introducing the probability based receive model, but introducing the frame
> > > capture model + calibrating the curves for 802.11p channels using lab tests.
> > > 
> > > As you know, 802.11p uses (mostly) 10 MHz channels, which makes the preamble
> > > datarate 3Mbps, still using 6Mbps for the payload. This is why we shifted
> > > these two curves obtained from ns3.
> > 
> > Ok.
> > 
> > > 
> > > > 
> > > > Which conditions did you consider to obtain your model?
> > > > This looks quite empirical, is your implemented model generic enough to be
> > > > adapted to any other conditions?
> > > 
> > > I am not sure if I understand what you mean by "condition" here. However, I
> > > think we can consider these results robust enough since these results are
> > > done by two hardware vendors. Furthermore, as far as I know, these curves
> > > are containing  hardware-specific behavior and shouldn't be changed under
> > > different conditions.
> > 
> > I am a bit confused: are those results from lab trials performed with
> > several hardwares or are those results directly provided by different
> > vendors?
> >  
> > > 
> > > > 
> > > > Could you explain what are the differences between figure 5 and 6? 
> > > > What do they refer to exactly?
> > > 
> > > I answered this in your first question.
> > > 
> > > 
> > > Please don't hesitate if you had further questions, and I'll try to get back
> > > to you asap.
> > 
> > Thanks for replying so quickly, I want to have it clear in my mind before
> > getting through the review of the code itself.
> 
> Hi Sebastien,
> 
> I am Bin Cheng. I am also a contributor of this patch. Thank you for
> starting reviewing our patch and asking these questions.
> 
> Please let me provide more background information which may help you to
> better understand our patch and its focus. This patch is an outcome of a
> multiple-year project which focuses on accurately simulating vehicle-vehicle
> communication networks, specifically DSRC (which uses 802.11p as MAC and PHY
> layer protocol) networks. Therefore, all the proposed models in this patch
> are primarily calibrated for the IEEE 802.11p standard, i.e., for the OFDM
> signals with 10MHz bandwidth, 6Mbps user data rate for the packet payload
> (the modulation mode is QPSK) and 3Mbps data rate for the packet preamble
> (the modulation is BPSK).  To be honest, we are not sure how accuracy the
> simulation results will be when using the proposed models for other IEEE
> 802.11 standards with a different data rate or a different bandwidth.
> However, we would like to say that the framework we are proposing for
> handling the frame capture may be very useful. This framework was largely
> inspired by the frame capture implementation in ns-2 [1] and also Bingmann's
> implementation in ns-3 [2]. Different from the threshold-based models in
> these implementations, we used a probability-based model which can capture
> these transition zones from unable-capture to successful-capture, as
> depicted in figure 2-3 and figure 5-6. We believe these transition zones are
> the important nature when modeling the device's behaviors. 
> 
> Regarding the modeling details, these models were essentially fitted and
> calibrated based on the data from a set of hardware lab tests. These tests
> were conducted by very experienced engineers from a major chip company in
> U.S. and the focuses of these tests were to clarify some of the radio
> characteristics of the DSRC transceivers and to help to determine the
> correct values for these parameters. However, as the frame capture, the
> hardware tests were only able to provide data for the payload capture (or
> data capture) not for the preamble capture. This is the major reason why we
> used a shifted ns-3 NIST error model for the preamble capture. We then tried
> different levels of shifts on the model and select the one which produces
> the best match to the field test results. 
> 
> Regarding why we always use two models, one for so-called decoding and one
> for so-called capture, I think Ali has explained very well. Basically, the
> decoding models are trying to model that given the current SINR level, what
> the probability that the currently received signal can still be successfully
> recovered. The capture models are trying to model that given the current
> SINR level, what the probability that the currently received signal is
> dropped and the receiver switches to the newly arrived signal. However, for
> the preamble part, it may be tricky. It is because that during the preamble
> period, the receiver has not locked to any signal yet (different receivers
> may have different behaviors, some receivers only need a few short training
> symbol to lock the signal but some receivers may need long training symbols
> to do so), so it may not be very accurate to say this is "decoding" or
> "capture". But we simplify this process and say that from the very beginning
> of the packet, the receiver is locked to that packet. 
> 
> One more thing to note is that our models are primarily based on
> relationships between SINR and decoding/capture probability (let us call
> this SINR vs PDR). It is very different from NIST model which is BER based.
> May I ask what your plan is to enable frame capture effect in the future
> release, do you want to keep using NIST model or do you want to simplify it
> to use models with SINR vs PDR? I tried another implementation of the frame
> capture which keeps using the current BER-based structure. We can have more
> discussion on this in the future.

Maybe this would be good to this in two phases.
Could you then also attach that implementation?
I'll have a look to that one first then.

> 
> Thanks again for your help, and please let us know if you have more
> questions.
> 
> Best, 
> Bin
> 
> 
> [1] Q. Chen, F. Schmidt-Eisenlohr, D. Jiang,M. Torrent-Moreno, L. Delgrossi,
> and H. Hartenstein.Overhaul of ieee 802.11 modeling and simulation in ns-2.
> In Proceedings of the 10th ACM Symposium on Modeling, Analysis, and
> Simulation of Wireless and Mobile Systems, MSWiM ’07, pages 159{168. ACM,
> 2007 
> [2]  T. Bingmann. Accuracy enhancements of the 802.11 model and edca qos
> extensions in ns-3. Master’s thesis, University of Karlsruhe, 2009.

Thanks a lot for all those clarifications.
Comment 19 Ali Rostami 2016-11-27 17:18:36 EST
Hi, Sebastien. Could you give us an update on the code review progress? Please feel free to reach out in case you need more information about any part of the code.
Comment 20 sebastien.deronne 2016-11-29 02:32:33 EST
(In reply to Ali Rostami from comment #19)
> Hi, Sebastien. Could you give us an update on the code review progress?
> Please feel free to reach out in case you need more information about any
> part of the code.

Ali, could you please answer the query above? I would prefer to integrate your model in two steps: first step based on the existing error models, and a second phase that includes your proposed error model.
Comment 21 Bin Cheng 2016-11-29 09:47:05 EST
(In reply to sebastien.deronne from comment #20)
> (In reply to Ali Rostami from comment #19)
> > Hi, Sebastien. Could you give us an update on the code review progress?
> > Please feel free to reach out in case you need more information about any
> > part of the code.
> 
> Ali, could you please answer the query above? I would prefer to integrate
> your model in two steps: first step based on the existing error models, and
> a second phase that includes your proposed error model.

Hi Sebastien, 

Thanks a lot for the update. May I ask whether you want to include the frame capture functionality in your phase 1 test, i.e., are you going to test the existing error in a scenario where the receiver can switch to another packet reception while currently in the RX state. Do you mind to share us what the idea in your  mind to do so? I hope we can also help you on that. 

The main reason I ask this is because the existing error model in the ns-3 is BER based and the reception process is what we call "look back", i.e., at the end of the packet reception, every SINR change during the time period of the packet reception will be check. If we let the receiver switch between two receptions, that means before reaching the end of the reception of one packet, the receiver has to decide if the reception can be continued based on the output of the error model. I am not sure how this process can fit into the current reception logic. If you already have some idea about this, could you please let us know? 

One main idea is that instead of "look back", we can use a method called "look forward". Let us consider the following case: the receiver is receiving packet A, while the packet B arrives. At this time point, the receiver can check all the SINR changes that happen between the current time point and the end time of the packet A, and then use the existing error model to calculate the probability of successfully decoding the packet A in the future. In this way, the SINR changes caused by  these events partially overlapped with the packet A are also considered. I have investigated this method a little bit. If you are interested, we can discuss this more. 

Thanks, and please let us know your thoughts. 
Bin
Comment 22 sebastien.deronne 2016-12-09 14:27:43 EST
(In reply to Bin Cheng from comment #21)
> (In reply to sebastien.deronne from comment #20)
> > (In reply to Ali Rostami from comment #19)
> > > Hi, Sebastien. Could you give us an update on the code review progress?
> > > Please feel free to reach out in case you need more information about any
> > > part of the code.
> > 
> > Ali, could you please answer the query above? I would prefer to integrate
> > your model in two steps: first step based on the existing error models, and
> > a second phase that includes your proposed error model.
> 
> Hi Sebastien, 
> 
> Thanks a lot for the update. May I ask whether you want to include the frame
> capture functionality in your phase 1 test, i.e., are you going to test the
> existing error in a scenario where the receiver can switch to another packet
> reception while currently in the RX state. Do you mind to share us what the
> idea in your  mind to do so? I hope we can also help you on that. 

Yes my idea is to include the frame capture implementation itself.

> 
> The main reason I ask this is because the existing error model in the ns-3
> is BER based and the reception process is what we call "look back", i.e., at
> the end of the packet reception, every SINR change during the time period of
> the packet reception will be check. If we let the receiver switch between
> two receptions, that means before reaching the end of the reception of one
> packet, the receiver has to decide if the reception can be continued based
> on the output of the error model. I am not sure how this process can fit
> into the current reception logic. If you already have some idea about this,
> could you please let us know? 

I do not see what is exactly blocking, could you detail a bit more?

> 
> One main idea is that instead of "look back", we can use a method called
> "look forward". Let us consider the following case: the receiver is
> receiving packet A, while the packet B arrives. At this time point, the
> receiver can check all the SINR changes that happen between the current time
> point and the end time of the packet A, and then use the existing error
> model to calculate the probability of successfully decoding the packet A in
> the future. In this way, the SINR changes caused by  these events partially
> overlapped with the packet A are also considered. I have investigated this
> method a little bit. If you are interested, we can discuss this more. 

I suggest to start with a common method that is used in the field.

> 
> Thanks, and please let us know your thoughts. 
> Bin

I think we urgently need to have a working draft if we still want this in the next release. Maybe we could have some discussions offline.
Comment 23 Ali Rostami 2016-12-09 15:16:29 EST
(In reply to sebastien.deronne from comment #20)
> (In reply to Ali Rostami from comment #19)
> > Hi, Sebastien. Could you give us an update on the code review progress?
> > Please feel free to reach out in case you need more information about any
> > part of the code.
> 
> Ali, could you please answer the query above? I would prefer to integrate
> your model in two steps: first step based on the existing error models, and
> a second phase that includes your proposed error model.

Thanks, Sebastien.

I think your proposal is more structured and helps to integrate the frame capture model more smoothly. The problem, however, is that we are not aware of any error model to be used when the second frame arrives. Neither to check if the hardware can continue receiving the first frame nor to check if the second frame's signal is strong enough to start receiving it. On the other hand, we only have these error models for 802.11p. Can you please share your thoughts on this issue? I will try to dedicate more time to this task, so we can make it ready for next release.
Comment 24 Bin Cheng 2016-12-10 08:10:16 EST
(In reply to sebastien.deronne from comment #22)
> (In reply to Bin Cheng from comment #21)
> > (In reply to sebastien.deronne from comment #20)
> > > (In reply to Ali Rostami from comment #19)
> > > > Hi, Sebastien. Could you give us an update on the code review progress?
> > > > Please feel free to reach out in case you need more information about any
> > > > part of the code.
> > > 
> > > Ali, could you please answer the query above? I would prefer to integrate
> > > your model in two steps: first step based on the existing error models, and
> > > a second phase that includes your proposed error model.
> > 
> > Hi Sebastien, 
> > 
> > Thanks a lot for the update. May I ask whether you want to include the frame
> > capture functionality in your phase 1 test, i.e., are you going to test the
> > existing error in a scenario where the receiver can switch to another packet
> > reception while currently in the RX state. Do you mind to share us what the
> > idea in your  mind to do so? I hope we can also help you on that. 
> 
> Yes my idea is to include the frame capture implementation itself.
> 
> > 
> > The main reason I ask this is because the existing error model in the ns-3
> > is BER based and the reception process is what we call "look back", i.e., at
> > the end of the packet reception, every SINR change during the time period of
> > the packet reception will be check. If we let the receiver switch between
> > two receptions, that means before reaching the end of the reception of one
> > packet, the receiver has to decide if the reception can be continued based
> > on the output of the error model. I am not sure how this process can fit
> > into the current reception logic. If you already have some idea about this,
> > could you please let us know? 
> 
> I do not see what is exactly blocking, could you detail a bit more?
> 
> > 
> > One main idea is that instead of "look back", we can use a method called
> > "look forward". Let us consider the following case: the receiver is
> > receiving packet A, while the packet B arrives. At this time point, the
> > receiver can check all the SINR changes that happen between the current time
> > point and the end time of the packet A, and then use the existing error
> > model to calculate the probability of successfully decoding the packet A in
> > the future. In this way, the SINR changes caused by  these events partially
> > overlapped with the packet A are also considered. I have investigated this
> > method a little bit. If you are interested, we can discuss this more. 
> 
> I suggest to start with a common method that is used in the field.
> 
> > 
> > Thanks, and please let us know your thoughts. 
> > Bin
> 
> I think we urgently need to have a working draft if we still want this in
> the next release. Maybe we could have some discussions offline.

Hi Sebastien, 

Thank you again for your email. Regarding the question I asked about integrating the BER based error model with the frame capture process, I was just not sure about how the reception switching between two packets will be handled. Let us say, a receiver is receiving packet A while packet B arrives. According to the frame capture logic, whether the reception of packet A can be continued should be assessed and the error model is applied here. Assuming the BER based model is used, I am not sure how the decision about the reception of packet A can be made, i.e., should the decision is made based on the probability of successfully decoding the past bits before this time or should be based on the probability of successfully decoding the future bits after now with respect to the changed SINR? If the decoding probability of the past bits is considered, there are also two cases in my mind: 1) there was no probability check for packet A before, which means before packet B, no other packets arrive at the receiver while the receiver is receiving packet A. In this case, I think the probability assessment of successfully decoding each past bit should start from the beginning of the reception of packet A; 2) however, another case is that before arrival of packet B, the probability assessment of successfully  decoding for packet A has been performed, due to the arrival of other packets while receiving packet A. Then in case, the probability assessment should start from the time of the last check or start from the beginning of the reception of packet A? 

Please let me know your thoughts on this and also please let us know if you have more questions.  We can, for sure, have more discussions about this offline. 

Thanks again for the help and enjoy the weekend, 

Best, 
Bin
Comment 25 sebastien.deronne 2016-12-11 10:09:40 EST
I think you should take into account the current SINR, and not look at the past SINR to make your decision.

I just want to include your implementation progressively and not all your changes at once, here I mainly want to see the switch to another packet reception for this first phase.
Comment 26 Bin Cheng 2016-12-11 12:06:57 EST
(In reply to sebastien.deronne from comment #25)
> I think you should take into account the current SINR, and not look at the
> past SINR to make your decision.
> 
> I just want to include your implementation progressively and not all your
> changes at once, here I mainly want to see the switch to another packet
> reception for this first phase.

In our current implementation, whether the current reception can be continued is based on the current SINR, but the error model is different. 

Thank you very much for letting us know your plan. Do you want us to provide you a new code update combining our frame capture model (the model decides when to switch to the new packet ) and the default ns-3 error model (the model used to decide whether the current reception can be continued), or you want to include the frame capture model into the current release yourself? 

Thanks.
Comment 27 sebastien.deronne 2016-12-12 14:51:44 EST
(In reply to Bin Cheng from comment #26)
> (In reply to sebastien.deronne from comment #25)
> > I think you should take into account the current SINR, and not look at the
> > past SINR to make your decision.
> > 
> > I just want to include your implementation progressively and not all your
> > changes at once, here I mainly want to see the switch to another packet
> > reception for this first phase.
> 
> In our current implementation, whether the current reception can be
> continued is based on the current SINR, but the error model is different. 
> 
> Thank you very much for letting us know your plan. Do you want us to provide
> you a new code update combining our frame capture model (the model decides
> when to switch to the new packet ) and the default ns-3 error model (the
> model used to decide whether the current reception can be continued), or you
> want to include the frame capture model into the current release yourself? 
> 
> Thanks.

I am waiting for your patch, or eventually several patches with the different increments even if we do not deliver all at once in the next release.
Comment 28 Ali Rostami 2016-12-12 15:08:15 EST
(In reply to sebastien.deronne from comment #27)
> (In reply to Bin Cheng from comment #26)
> > (In reply to sebastien.deronne from comment #25)
> > > I think you should take into account the current SINR, and not look at the
> > > past SINR to make your decision.
> > > 
> > > I just want to include your implementation progressively and not all your
> > > changes at once, here I mainly want to see the switch to another packet
> > > reception for this first phase.
> > 
> > In our current implementation, whether the current reception can be
> > continued is based on the current SINR, but the error model is different. 
> > 
> > Thank you very much for letting us know your plan. Do you want us to provide
> > you a new code update combining our frame capture model (the model decides
> > when to switch to the new packet ) and the default ns-3 error model (the
> > model used to decide whether the current reception can be continued), or you
> > want to include the frame capture model into the current release yourself? 
> > 
> > Thanks.
> 
> I am waiting for your patch, or eventually several patches with the
> different increments even if we do not deliver all at once in the next
> release.

Sounds good to me. We can have a second look at them before publishing, if you think it's necessary. Thanks.
Comment 29 Bin Cheng 2016-12-12 16:13:37 EST
Thanks, Sebastien. I will send you a new patch implementing packet reception switching soon.
Comment 30 sebastien.deronne 2017-01-02 02:53:00 EST
(In reply to Bin Cheng from comment #29)
> Thanks, Sebastien. I will send you a new patch implementing packet reception
> switching soon.

Thanks. Any update on this? I think we have now 2 weeks left before the release.
Comment 31 Bin Cheng 2017-01-02 20:01:58 EST
(In reply to sebastien.deronne from comment #30)
> (In reply to Bin Cheng from comment #29)
> > Thanks, Sebastien. I will send you a new patch implementing packet reception
> > switching soon.
> 
> Thanks. Any update on this? I think we have now 2 weeks left before the
> release.

Hi Sebastien, 

Happy new year!

I have prepared a new implementation of the frame capture. This implementation was based on the current ns-3-dev. In this new implementation, whether the reception of the old packet can be continued is evaluated based on the current ns-3 error model. While whether the receiver can switch to the new packet is decided based on our frame capture model. The frame capture effect can be turned off when configuring the simulation. 

I also created an example script based on "test-interference-helper.cc". In this file, by changing the sending time of the two packets, the collision time can be modified, i.e., by changing sending interval of the two senders, we can let the two packets collide at the preamble portion or the payload portion. Also by changing the tx power of the two senders, we can test the reception situation with different SINR. I am generally using this script to test the implementation. 

I have attached the patch of this implementation and we can use this as our working version. I am still working on other tests and also polishing the code. Please let me know if you have any question and comment. 

Thank you for helping us on this. 

Best,
Bin
Comment 32 Bin Cheng 2017-01-02 20:04:16 EST
Created attachment 2728 [details]
Frame capture implementation based ns-3-dev: implements the frame capture effect using the default ns-3 model and the proposed frame capture model
Comment 33 sebastien.deronne 2017-01-04 03:58:54 EST
Happy new year too, thanks.
Maybe it is best you create a review for it so that we can directly comment in the code.
Also, a lot of regression tests/examples are broken with this patch. I always prefer to have them fixed before reviewing code, unless it is quite obvious (like rescanning pcap, ...).
I agree it is important to have an example, thanks for this. To my knowledge, there is already a test for interference helper, can't this one be adapted and added to regression if not yet part of it?

FYI: regression tests results:

List of FAILed tests:
    devices-mesh-dot11s-regression
    devices-mesh-flame-regression
    routing-aodv-regression
    spectrum-wifi-phy
List of CRASHed tests:
    examples/wireless/ht-wifi-network --simulationTime=0.1
    examples/wireless/simple-ht-hidden-stations --simulationTime=1
    examples/wireless/vht-wifi-network --simulationTime=0.1
    examples/wireless/wifi-aggregation --simulationTime=1
    examples/wireless/wifi-multi-tos --simulationTime=1 --nWifi=16 --useRts=1 --useShortGuardInterval=1
    examples/wireless/wifi-spectrum-per-example --distance=52 --index=3 --wifiType=ns3::SpectrumWifiPhy --simulationTime=1
    examples/wireless/wifi-spectrum-saturation-example --simulationTime=1 --index=63
    examples/wireless/wifi-tcp
    src/mesh/examples/mesh
Comment 34 Tom Henderson 2017-01-04 13:15:39 EST
(In reply to sebastien.deronne from comment #33)
> Happy new year too, thanks.
> Maybe it is best you create a review for it so that we can directly comment
> in the code.

New review for this:

https://codereview.appspot.com/319050043/

will close the previous one.
Comment 35 Bin Cheng 2017-01-05 00:26:10 EST
Created attachment 2732 [details]
Updated frame capture patch
Comment 36 Bin Cheng 2017-01-05 00:30:30 EST
(In reply to Tom Henderson from comment #34)
> (In reply to sebastien.deronne from comment #33)
> > Happy new year too, thanks.
> > Maybe it is best you create a review for it so that we can directly comment
> > in the code.
> 
> New review for this:
> 
> https://codereview.appspot.com/319050043/
> 
> will close the previous one.

Thanks a lot, Tom, for creating this code review. 

I have just uploaded an updated patch which fixed all the crashed examples on my machine and passed most tests except three regression ones due to pcap rescanning. 

I am wondering how I can use this code for code review. Do I have to recreate a new code review?

Thank you and I am sorry for the troubles.
Comment 37 Bin Cheng 2017-01-05 00:31:53 EST
(In reply to Tom Henderson from comment #34)
> (In reply to sebastien.deronne from comment #33)
> > Happy new year too, thanks.
> > Maybe it is best you create a review for it so that we can directly comment
> > in the code.
> 
> New review for this:
> 
> https://codereview.appspot.com/319050043/
> 
> will close the previous one.

Thanks a lot, Tom, for creating this code review. 

I have just uploaded an updated patch which fixed all the crashed examples on my machine and passed most tests except three regression ones due to pcap rescanning. 

I am wondering how I can use this code for code review. Do I have to recreate a new code review?

Thank you and I am sorry for the troubles.
Comment 38 Tom Henderson 2017-01-05 01:00:06 EST
(In reply to Bin Cheng from comment #37)
> (In reply to Tom Henderson from comment #34)
> > (In reply to sebastien.deronne from comment #33)
> > > Happy new year too, thanks.
> > > Maybe it is best you create a review for it so that we can directly comment
> > > in the code.
> > 
> > New review for this:
> > 
> > https://codereview.appspot.com/319050043/
> > 
> > will close the previous one.
> 
> Thanks a lot, Tom, for creating this code review. 
> 
> I have just uploaded an updated patch which fixed all the crashed examples
> on my machine and passed most tests except three regression ones due to pcap
> rescanning. 
> 
> I am wondering how I can use this code for code review. Do I have to
> recreate a new code review?

I will get the review updated tomorrow.

> 
> Thank you and I am sorry for the troubles.

No troubles; thank you for the update.
Comment 39 sebastien.deronne 2017-01-05 04:07:04 EST
Thanks, I'll have a look this weekend.
Any idea why simple-ht-hidden-stations does fail?
Comment 40 sebastien.deronne 2017-01-05 04:13:09 EST
(In reply to sebastien.deronne from comment #39)
> Thanks, I'll have a look this weekend.
> Any idea why simple-ht-hidden-stations does fail?

It is also not clear why pcap needs to be rescanned. If you have frame capture mode disabled, it should still behave the same right?
Comment 41 Bin Cheng 2017-01-05 08:54:52 EST
(In reply to sebastien.deronne from comment #40)
> (In reply to sebastien.deronne from comment #39)
> > Thanks, I'll have a look this weekend.
> > Any idea why simple-ht-hidden-stations does fail?
> 
> It is also not clear why pcap needs to be rescanned. If you have frame
> capture mode disabled, it should still behave the same right?

Thanks, Sebastien. Somehow, I did not have this test failed on my machine. I will check it later.
Comment 42 Tom Henderson 2017-01-05 09:53:14 EST
(In reply to Bin Cheng from comment #35)
> Created attachment 2732 [details]
> Updated frame capture patch


This is the corresponding Rietveld issue update:
https://codereview.appspot.com/319050043/#ps20001
Comment 43 sebastien.deronne 2017-01-05 16:20:16 EST
(In reply to Bin Cheng from comment #41)
> (In reply to sebastien.deronne from comment #40)
> > (In reply to sebastien.deronne from comment #39)
> > > Thanks, I'll have a look this weekend.
> > > Any idea why simple-ht-hidden-stations does fail?
> > 
> > It is also not clear why pcap needs to be rescanned. If you have frame
> > capture mode disabled, it should still behave the same right?
> 
> Thanks, Sebastien. Somehow, I did not have this test failed on my machine. I
> will check it later.

I guess you are not running on the latest ns-3-dev, this example has been added to regression recently.
Comment 44 sebastien.deronne 2017-01-24 14:46:59 EST
Do you have an updated patch?

My main concerns are:
- no real example to show frame capture effects;
- interference test could be extended to cover frame capture scenarios;
- regression tests are still failing;
- changes done in InterferenceHelper are not fully clear.

I'd like we finish this up by the end of the week :-)
Comment 45 Bin Cheng 2017-01-24 19:45:33 EST
(In reply to sebastien.deronne from comment #44)
> Do you have an updated patch?
> 
> My main concerns are:
> - no real example to show frame capture effects;
> - interference test could be extended to cover frame capture scenarios;
> - regression tests are still failing;
> - changes done in InterferenceHelper are not fully clear.
> 
> I'd like we finish this up by the end of the week :-)

I am sorry that I am quite busy recently and I could not response early. I have made a new patch which incorporates almost all the comments on the previous version. However, I am still struggling with the failed regression test. With this new patch, I still have one test failed, devices-mesh-dot11s-regression. I checked the generated pcap files and found that it seems the packet generation sequence has been changed, i.e., the packets are generated in a different order from the reference pcap files. I am still not sure which part of my changes triggered this. If you have any suggestion, please let me know. The following describes the changes from the last version.  

In this new patch, I created a new file in src/wifi/examples/frame-capture-model-validation.cc. This file can generate plots which show the curves for the capture model we used. These plots are comparable to the curves depicted in fig. 6(b) and 6(d) of this paper (http://www.winlab.rutgers.edu/~cb3974/Bin%20Cheng's%20Homepage_files/publication/Experience_Simulation_MobiCom_2016.pdf)

I also modified file test-interference-helper.cc to support an option of testing frame capture capability. By changing the packet sending time of the two senders and also the transmission power, different packet collision situations can be created. The description of these tests can be found within the file. 

I also modified frame capture logics according to your comments. Now, if the frame capture is not enabled, the code should generate the same results as the old version. 

I also used the script to correct the code style. However, in wifi-phy.cc script, you may notice some lines of code which are not related to the frame capture implementation are also changed. It is mainly because these lines of code fail the code style tests. I also listed these changes here. 

The reason I made changes to InterferenceHelper::CalculateNoiseInterferenceW is because now we also want to calculate the sinr for the packet arrives later. Let us consider the following example: 

packet A arrives first. The interference power for this packets has been included in  m_firstPower. And then, packet B arrives later. In this case, we also want to calculate the SINR for packet B. The interference power for packet B is m_firstPower + rx power of packet A + rx power of packets arrives between packet A and packet B but not strong enough to be captured. The following changes in this method is generally to support this purpose. Hopefully, this helps you to understand these changes.
Comment 46 Bin Cheng 2017-01-24 19:46:55 EST
Created attachment 2761 [details]
frame-capture-patch v3
Comment 47 sebastien.deronne 2017-01-25 15:34:18 EST
(In reply to Bin Cheng from comment #46)
> Created attachment 2761 [details]
> frame-capture-patch v3

Tom, could you please update the Rietveld issue with the new patch?
Comment 48 sebastien.deronne 2017-01-25 15:54:40 EST
(In reply to Bin Cheng from comment #45)
> (In reply to sebastien.deronne from comment #44)
> > Do you have an updated patch?
> > 
> > My main concerns are:
> > - no real example to show frame capture effects;
> > - interference test could be extended to cover frame capture scenarios;
> > - regression tests are still failing;
> > - changes done in InterferenceHelper are not fully clear.
> > 
> > I'd like we finish this up by the end of the week :-)
> 
> I am sorry that I am quite busy recently and I could not response early. I
> have made a new patch which incorporates almost all the comments on the
> previous version. 

Thanks for your hard work so far.

However, I am still struggling with the failed regression
> test. With this new patch, I still have one test failed,
> devices-mesh-dot11s-regression. I checked the generated pcap files and found
> that it seems the packet generation sequence has been changed, i.e., the
> packets are generated in a different order from the reference pcap files. I
> am still not sure which part of my changes triggered this. If you have any
> suggestion, please let me know. 

If I remove the changes done for CalculateNoiseInterferenceW, regression passes fine.
Could you please double check this part?

The following describes the changes from the
> last version.  
> 
> In this new patch, I created a new file in
> src/wifi/examples/frame-capture-model-validation.cc. This file can generate
> plots which show the curves for the capture model we used. These plots are
> comparable to the curves depicted in fig. 6(b) and 6(d) of this paper
> (http://www.winlab.rutgers.edu/~cb3974/Bin%20Cheng's%20Homepage_files/
> publication/Experience_Simulation_MobiCom_2016.pdf)

Thanks. Could you please add a comment in this example to explain the outputs?

I also would like to see an example in examples/wireless to show how you can configure frame capture modes. Its outputs could demonstrate the benefits of the two frame capture modes compared to frame capture disabled.

> 
> I also modified file test-interference-helper.cc to support an option of
> testing frame capture capability. By changing the packet sending time of the
> two senders and also the transmission power, different packet collision
> situations can be created. The description of these tests can be found
> within the file. 

Thanks.
I see you forced enableFrameCapture and enableMultipleFrameCaptureTests to true regardless of the passed configuration. Could you change that? Also, what is the purpose of enableMultipleFrameCaptureTests? I would instead expect to see two parameters to enable either frame or payload capture mode.

Note that I will move to Nist in a separate commit in our ns-3-dev repo, I think Nist is more commonly used so it should be mainly tested against Nist.

> 
> I also modified frame capture logics according to your comments. Now, if the
> frame capture is not enabled, the code should generate the same results as
> the old version. 

Thanks.
I still see few differences, for example the goto will exit and go directly to it, while you will call the function and eventually few other processes will be performed. Could you please double check this?

> 
> I also used the script to correct the code style. However, in wifi-phy.cc
> script, you may notice some lines of code which are not related to the frame
> capture implementation are also changed. It is mainly because these lines of
> code fail the code style tests. I also listed these changes here. 

It is very nice you did not forget :-)
I and lots of other developers general forget to do it and this is why you find some misalignments. I will run a check-style on the wifi directory and I'll update the repository.

> 
> The reason I made changes to InterferenceHelper::CalculateNoiseInterferenceW
> is because now we also want to calculate the sinr for the packet arrives
> later. Let us consider the following example: 
> 
> packet A arrives first. The interference power for this packets has been
> included in  m_firstPower. And then, packet B arrives later. In this case,
> we also want to calculate the SINR for packet B. The interference power for
> packet B is m_firstPower + rx power of packet A + rx power of packets
> arrives between packet A and packet B but not strong enough to be captured.
> The following changes in this method is generally to support this purpose.
> Hopefully, this helps you to understand these changes.

I'll have a look once regression is fixed, maybe this will be slightly modified in your next patch.

I'll add few more comments in the Rietveld once it is updated.
Please note I would like to have it pushed to the mainstream over the middle of next week, so my full attention will be on this item for the coming days.
Comment 49 Tom Henderson 2017-01-27 01:41:38 EST
> 
> I'll add few more comments in the Rietveld once it is updated.

It is now updated.
Comment 50 Bin Cheng 2017-01-27 19:06:13 EST
Created attachment 2765 [details]
Updated patch to fix the regression test failure
Comment 51 Bin Cheng 2017-01-27 19:08:42 EST
(In reply to sebastien.deronne from comment #48)
> (In reply to Bin Cheng from comment #45)
> > (In reply to sebastien.deronne from comment #44)
> > > Do you have an updated patch?
> > > 
> > > My main concerns are:
> > > - no real example to show frame capture effects;
> > > - interference test could be extended to cover frame capture scenarios;
> > > - regression tests are still failing;
> > > - changes done in InterferenceHelper are not fully clear.
> > > 
> > > I'd like we finish this up by the end of the week :-)
> > 
> > I am sorry that I am quite busy recently and I could not response early. I
> > have made a new patch which incorporates almost all the comments on the
> > previous version. 
> 
> Thanks for your hard work so far.
> 
> However, I am still struggling with the failed regression
> > test. With this new patch, I still have one test failed,
> > devices-mesh-dot11s-regression. I checked the generated pcap files and found
> > that it seems the packet generation sequence has been changed, i.e., the
> > packets are generated in a different order from the reference pcap files. I
> > am still not sure which part of my changes triggered this. If you have any
> > suggestion, please let me know. 
> 
> If I remove the changes done for CalculateNoiseInterferenceW, regression
> passes fine.
> Could you please double check this part?
> 
> The following describes the changes from the
> > last version.  
> > 
> > In this new patch, I created a new file in
> > src/wifi/examples/frame-capture-model-validation.cc. This file can generate
> > plots which show the curves for the capture model we used. These plots are
> > comparable to the curves depicted in fig. 6(b) and 6(d) of this paper
> > (http://www.winlab.rutgers.edu/~cb3974/Bin%20Cheng's%20Homepage_files/
> > publication/Experience_Simulation_MobiCom_2016.pdf)
> 
> Thanks. Could you please add a comment in this example to explain the
> outputs?
> 
> I also would like to see an example in examples/wireless to show how you can
> configure frame capture modes. Its outputs could demonstrate the benefits of
> the two frame capture modes compared to frame capture disabled.
> 
> > 
> > I also modified file test-interference-helper.cc to support an option of
> > testing frame capture capability. By changing the packet sending time of the
> > two senders and also the transmission power, different packet collision
> > situations can be created. The description of these tests can be found
> > within the file. 
> 
> Thanks.
> I see you forced enableFrameCapture and enableMultipleFrameCaptureTests to
> true regardless of the passed configuration. Could you change that? Also,
> what is the purpose of enableMultipleFrameCaptureTests? I would instead
> expect to see two parameters to enable either frame or payload capture mode.
> 
> Note that I will move to Nist in a separate commit in our ns-3-dev repo, I
> think Nist is more commonly used so it should be mainly tested against Nist.
> 
> > 
> > I also modified frame capture logics according to your comments. Now, if the
> > frame capture is not enabled, the code should generate the same results as
> > the old version. 
> 
> Thanks.
> I still see few differences, for example the goto will exit and go directly
> to it, while you will call the function and eventually few other processes
> will be performed. Could you please double check this?
> 
> > 
> > I also used the script to correct the code style. However, in wifi-phy.cc
> > script, you may notice some lines of code which are not related to the frame
> > capture implementation are also changed. It is mainly because these lines of
> > code fail the code style tests. I also listed these changes here. 
> 
> It is very nice you did not forget :-)
> I and lots of other developers general forget to do it and this is why you
> find some misalignments. I will run a check-style on the wifi directory and
> I'll update the repository.
> 
> > 
> > The reason I made changes to InterferenceHelper::CalculateNoiseInterferenceW
> > is because now we also want to calculate the sinr for the packet arrives
> > later. Let us consider the following example: 
> > 
> > packet A arrives first. The interference power for this packets has been
> > included in  m_firstPower. And then, packet B arrives later. In this case,
> > we also want to calculate the SINR for packet B. The interference power for
> > packet B is m_firstPower + rx power of packet A + rx power of packets
> > arrives between packet A and packet B but not strong enough to be captured.
> > The following changes in this method is generally to support this purpose.
> > Hopefully, this helps you to understand these changes.
> 
> I'll have a look once regression is fixed, maybe this will be slightly
> modified in your next patch.
> 
> I'll add few more comments in the Rietveld once it is updated.
> Please note I would like to have it pushed to the mainstream over the middle
> of next week, so my full attention will be on this item for the coming days.

I have uploaded a new patch. This new patch has fixed the regression failure issue. Also, I also added some changes to incorporate your comments. Please let me know if you have other questions.
Comment 52 Bin Cheng 2017-01-27 19:12:10 EST
(In reply to Tom Henderson from comment #49)
> > 
> > I'll add few more comments in the Rietveld once it is updated.
> 
> It is now updated.

Thanks a lot, Tom. I am sorry for bothering you so many times. I am wondering if I can update the patch in the Rietveld myself. I do not want to bother you anymore.
Comment 53 sebastien.deronne 2017-01-28 08:29:50 EST
(In reply to Bin Cheng from comment #52)
> (In reply to Tom Henderson from comment #49)
> > > 
> > > I'll add few more comments in the Rietveld once it is updated.
> > 
> > It is now updated.
> 
> Thanks a lot, Tom. I am sorry for bothering you so many times. I am
> wondering if I can update the patch in the Rietveld myself. I do not want to
> bother you anymore.

Not sure we can since it is under Tom's account.
Comment 54 sebastien.deronne 2017-01-28 10:11:22 EST
(In reply to Bin Cheng from comment #51)
> (In reply to sebastien.deronne from comment #48)
> > (In reply to Bin Cheng from comment #45)
> > > (In reply to sebastien.deronne from comment #44)
> > > > Do you have an updated patch?
> > > > 
> > > > My main concerns are:
> > > > - no real example to show frame capture effects;
> > > > - interference test could be extended to cover frame capture scenarios;
> > > > - regression tests are still failing;
> > > > - changes done in InterferenceHelper are not fully clear.
> > > > 
> > > > I'd like we finish this up by the end of the week :-)
> > > 
> > > I am sorry that I am quite busy recently and I could not response early. I
> > > have made a new patch which incorporates almost all the comments on the
> > > previous version. 
> > 
> > Thanks for your hard work so far.
> > 
> > However, I am still struggling with the failed regression
> > > test. With this new patch, I still have one test failed,
> > > devices-mesh-dot11s-regression. I checked the generated pcap files and found
> > > that it seems the packet generation sequence has been changed, i.e., the
> > > packets are generated in a different order from the reference pcap files. I
> > > am still not sure which part of my changes triggered this. If you have any
> > > suggestion, please let me know. 
> > 
> > If I remove the changes done for CalculateNoiseInterferenceW, regression
> > passes fine.
> > Could you please double check this part?
> > 
> > The following describes the changes from the
> > > last version.  
> > > 
> > > In this new patch, I created a new file in
> > > src/wifi/examples/frame-capture-model-validation.cc. This file can generate
> > > plots which show the curves for the capture model we used. These plots are
> > > comparable to the curves depicted in fig. 6(b) and 6(d) of this paper
> > > (http://www.winlab.rutgers.edu/~cb3974/Bin%20Cheng's%20Homepage_files/
> > > publication/Experience_Simulation_MobiCom_2016.pdf)
> > 
> > Thanks. Could you please add a comment in this example to explain the
> > outputs?
> > 
> > I also would like to see an example in examples/wireless to show how you can
> > configure frame capture modes. Its outputs could demonstrate the benefits of
> > the two frame capture modes compared to frame capture disabled.
> > 
> > > 
> > > I also modified file test-interference-helper.cc to support an option of
> > > testing frame capture capability. By changing the packet sending time of the
> > > two senders and also the transmission power, different packet collision
> > > situations can be created. The description of these tests can be found
> > > within the file. 
> > 
> > Thanks.
> > I see you forced enableFrameCapture and enableMultipleFrameCaptureTests to
> > true regardless of the passed configuration. Could you change that? Also,
> > what is the purpose of enableMultipleFrameCaptureTests? I would instead
> > expect to see two parameters to enable either frame or payload capture mode.
> > 
> > Note that I will move to Nist in a separate commit in our ns-3-dev repo, I
> > think Nist is more commonly used so it should be mainly tested against Nist.
> > 
> > > 
> > > I also modified frame capture logics according to your comments. Now, if the
> > > frame capture is not enabled, the code should generate the same results as
> > > the old version. 
> > 
> > Thanks.
> > I still see few differences, for example the goto will exit and go directly
> > to it, while you will call the function and eventually few other processes
> > will be performed. Could you please double check this?
> > 
> > > 
> > > I also used the script to correct the code style. However, in wifi-phy.cc
> > > script, you may notice some lines of code which are not related to the frame
> > > capture implementation are also changed. It is mainly because these lines of
> > > code fail the code style tests. I also listed these changes here. 
> > 
> > It is very nice you did not forget :-)
> > I and lots of other developers general forget to do it and this is why you
> > find some misalignments. I will run a check-style on the wifi directory and
> > I'll update the repository.
> > 
> > > 
> > > The reason I made changes to InterferenceHelper::CalculateNoiseInterferenceW
> > > is because now we also want to calculate the sinr for the packet arrives
> > > later. Let us consider the following example: 
> > > 
> > > packet A arrives first. The interference power for this packets has been
> > > included in  m_firstPower. And then, packet B arrives later. In this case,
> > > we also want to calculate the SINR for packet B. The interference power for
> > > packet B is m_firstPower + rx power of packet A + rx power of packets
> > > arrives between packet A and packet B but not strong enough to be captured.
> > > The following changes in this method is generally to support this purpose.
> > > Hopefully, this helps you to understand these changes.
> > 
> > I'll have a look once regression is fixed, maybe this will be slightly
> > modified in your next patch.
> > 
> > I'll add few more comments in the Rietveld once it is updated.
> > Please note I would like to have it pushed to the mainstream over the middle
> > of next week, so my full attention will be on this item for the coming days.
> 
> I have uploaded a new patch. This new patch has fixed the regression failure
> issue. Also, I also added some changes to incorporate your comments. Please
> let me know if you have other questions.

Thanks a bunch.
Regression is indeed fixed and I am more confident in the new proposed solution for interference helper. However, some of my comments are not handled yet. Do you have time to provide a (possibly) final patch with all comments handled?
Comment 55 Bin Cheng 2017-01-28 10:44:29 EST
Thanks for the reply. Could you please point out which your comments I have not addressed? I thought I have addressed almost all your comments except adding one more example in example/wireless. My response to your comments can be found on Rietveld as well, in case you have not noticed yet. Actually, the impact of the frame capture, i.e., the comparison between simulations with and without frame capture capability has been covered in the updated test-interference-helper.cc. I have added some explanation to the expected simulation result. And in frame-capture-model-validation, I also mentioned that the impact of the frame capture effect on the simulation results has also been studied in our paper. With all this, I am not sure whether we still need another example in the example/wireless and if so, what else we should include in that example. 

Could you please remind me which your comments I have not addressed? I can try my best to provide a new patch today. But I will be traveling from tomorrow to next Friday. I may not be able to devote too much time on this patch next week.
Comment 56 sebastien.deronne 2017-01-28 12:10:59 EST
(In reply to Bin Cheng from comment #55)
> Thanks for the reply. Could you please point out which your comments I have
> not addressed? I thought I have addressed almost all your comments except
> adding one more example in example/wireless. My response to your comments
> can be found on Rietveld as well, in case you have not noticed yet.
> Actually, the impact of the frame capture, i.e., the comparison between
> simulations with and without frame capture capability has been covered in
> the updated test-interference-helper.cc. I have added some explanation to
> the expected simulation result. And in frame-capture-model-validation, I
> also mentioned that the impact of the frame capture effect on the simulation
> results has also been studied in our paper. With all this, I am not sure
> whether we still need another example in the example/wireless and if so,
> what else we should include in that example. 

I would expect an example where you run for example 3 different networks where one outputs a legacy performance, one outputs performance using preamble capture mode and a last one outputs performance using payload capture mode.

> 
> Could you please remind me which your comments I have not addressed? I can
> try my best to provide a new patch today. But I will be traveling from
> tomorrow to next Friday. I may not be able to devote too much time on this
> patch next week.

I would mainly like to get further details about the error model you introduce and how deep it was validated.

Note that I would also wait for further comments from Tom before pushing, mainly wrt changes in InterferenceHelper.
Comment 57 Bin Cheng 2017-01-28 12:56:08 EST
(In reply to sebastien.deronne from comment #56)
> (In reply to Bin Cheng from comment #55)
> > Thanks for the reply. Could you please point out which your comments I have
> > not addressed? I thought I have addressed almost all your comments except
> > adding one more example in example/wireless. My response to your comments
> > can be found on Rietveld as well, in case you have not noticed yet.
> > Actually, the impact of the frame capture, i.e., the comparison between
> > simulations with and without frame capture capability has been covered in
> > the updated test-interference-helper.cc. I have added some explanation to
> > the expected simulation result. And in frame-capture-model-validation, I
> > also mentioned that the impact of the frame capture effect on the simulation
> > results has also been studied in our paper. With all this, I am not sure
> > whether we still need another example in the example/wireless and if so,
> > what else we should include in that example. 
> 
> I would expect an example where you run for example 3 different networks
> where one outputs a legacy performance, one outputs performance using
> preamble capture mode and a last one outputs performance using payload
> capture mode.
> 
Do you want to run all three different configurations in one example and generate three sets of results at the same time? Otherwise, I still think this has been covered in test-interference-helper.cc. In test-interference-helper, different packet collision scenarios were considered. And generally, the biggest difference between with and without frame capture is for switching to the secondly arrived packet. For without frame capture case, the collided packets are lost. However, for with frame capture case, once the SINR for the secondly arrived packet is strong enough, the secondly arrived packet can be received with a certain probability. 

> > 
> > Could you please remind me which your comments I have not addressed? I can
> > try my best to provide a new patch today. But I will be traveling from
> > tomorrow to next Friday. I may not be able to devote too much time on this
> > patch next week.
> 
> I would mainly like to get further details about the error model you
> introduce and how deep it was validated.

I have added some explanations in frame-capture-model.cc and also in our previous discussion, I have explained this several times. The payload capture model was fitted and calibrated based on the results of a set of hardware lab test conducted by CAMP engineers. The test was essentially for DSRC devices and it was for OFDM 6 Mbps signal on 10 MHz channel. However, during the test, the CAMP engineers could not generate a collided signal at preamble portion, so we were not able to calibrate a model as we did for the payload. The preamble capture model was based on try-and-error. We tried to shift an error model (originally based on the Nist model for decoding OFDM 6 Mbps signal with BPSK modulation. This is the data rate and the modulation method for the preamble portion of most wifi protocols), so that the simulator can generate the best match to the experiment results. As I mentioned before, this model was not able to be validated across different data rates and wifi protocols. However, we believe this model may still be useful in many wifi networks. Without permissions from CAMP, I could not reveal more lab test data and testing details so far. 

I am not sure whether this information is enough for you. If not, please let me what kind of specific details of validation you want to know. Thanks.

> 
> Note that I would also wait for further comments from Tom before pushing,
> mainly wrt changes in InterferenceHelper.
Comment 58 sebastien.deronne 2017-01-28 13:06:08 EST
(In reply to Bin Cheng from comment #57)
> (In reply to sebastien.deronne from comment #56)
> > (In reply to Bin Cheng from comment #55)
> > > Thanks for the reply. Could you please point out which your comments I have
> > > not addressed? I thought I have addressed almost all your comments except
> > > adding one more example in example/wireless. My response to your comments
> > > can be found on Rietveld as well, in case you have not noticed yet.
> > > Actually, the impact of the frame capture, i.e., the comparison between
> > > simulations with and without frame capture capability has been covered in
> > > the updated test-interference-helper.cc. I have added some explanation to
> > > the expected simulation result. And in frame-capture-model-validation, I
> > > also mentioned that the impact of the frame capture effect on the simulation
> > > results has also been studied in our paper. With all this, I am not sure
> > > whether we still need another example in the example/wireless and if so,
> > > what else we should include in that example. 
> > 
> > I would expect an example where you run for example 3 different networks
> > where one outputs a legacy performance, one outputs performance using
> > preamble capture mode and a last one outputs performance using payload
> > capture mode.
> > 
> Do you want to run all three different configurations in one example and
> generate three sets of results at the same time? Otherwise, I still think
> this has been covered in test-interference-helper.cc. In
> test-interference-helper, different packet collision scenarios were
> considered. And generally, the biggest difference between with and without
> frame capture is for switching to the secondly arrived packet. For without
> frame capture case, the collided packets are lost. However, for with frame
> capture case, once the SINR for the secondly arrived packet is strong
> enough, the secondly arrived packet can be received with a certain
> probability. 
> 
> > > 
> > > Could you please remind me which your comments I have not addressed? I can
> > > try my best to provide a new patch today. But I will be traveling from
> > > tomorrow to next Friday. I may not be able to devote too much time on this
> > > patch next week.
> > 
> > I would mainly like to get further details about the error model you
> > introduce and how deep it was validated.
> 
> I have added some explanations in frame-capture-model.cc and also in our
> previous discussion, I have explained this several times. The payload
> capture model was fitted and calibrated based on the results of a set of
> hardware lab test conducted by CAMP engineers. The test was essentially for
> DSRC devices and it was for OFDM 6 Mbps signal on 10 MHz channel. However,
> during the test, the CAMP engineers could not generate a collided signal at
> preamble portion, so we were not able to calibrate a model as we did for the
> payload. The preamble capture model was based on try-and-error. We tried to
> shift an error model (originally based on the Nist model for decoding OFDM 6
> Mbps signal with BPSK modulation. This is the data rate and the modulation
> method for the preamble portion of most wifi protocols), so that the
> simulator can generate the best match to the experiment results. As I
> mentioned before, this model was not able to be validated across different
> data rates and wifi protocols. However, we believe this model may still be
> useful in many wifi networks. Without permissions from CAMP, I could not
> reveal more lab test data and testing details so far. 
> 
> I am not sure whether this information is enough for you. If not, please let
> me what kind of specific details of validation you want to know. Thanks.
> 
> > 
> > Note that I would also wait for further comments from Tom before pushing,
> > mainly wrt changes in InterferenceHelper.

It seems based on 802.11a experiments, but it will be an issue for 802.11n/ac/ax/...
Did you make some trials with those most recent 802.11 versions?
Comment 59 Tom Henderson 2017-01-28 15:43:37 EST
> > 
> > Thanks a lot, Tom. I am sorry for bothering you so many times. I am
> > wondering if I can update the patch in the Rietveld myself. I do not want to
> > bother you anymore.
> 
> Not sure we can since it is under Tom's account.

I don't believe others can update it.  I don't mind; v4 patch is there now.
Comment 60 Bin Cheng 2017-01-28 19:55:48 EST
(In reply to sebastien.deronne from comment #58)
> (In reply to Bin Cheng from comment #57)
> > (In reply to sebastien.deronne from comment #56)
> > > (In reply to Bin Cheng from comment #55)
> > > > Thanks for the reply. Could you please point out which your comments I have
> > > > not addressed? I thought I have addressed almost all your comments except
> > > > adding one more example in example/wireless. My response to your comments
> > > > can be found on Rietveld as well, in case you have not noticed yet.
> > > > Actually, the impact of the frame capture, i.e., the comparison between
> > > > simulations with and without frame capture capability has been covered in
> > > > the updated test-interference-helper.cc. I have added some explanation to
> > > > the expected simulation result. And in frame-capture-model-validation, I
> > > > also mentioned that the impact of the frame capture effect on the simulation
> > > > results has also been studied in our paper. With all this, I am not sure
> > > > whether we still need another example in the example/wireless and if so,
> > > > what else we should include in that example. 
> > > 
> > > I would expect an example where you run for example 3 different networks
> > > where one outputs a legacy performance, one outputs performance using
> > > preamble capture mode and a last one outputs performance using payload
> > > capture mode.
> > > 
> > Do you want to run all three different configurations in one example and
> > generate three sets of results at the same time? Otherwise, I still think
> > this has been covered in test-interference-helper.cc. In
> > test-interference-helper, different packet collision scenarios were
> > considered. And generally, the biggest difference between with and without
> > frame capture is for switching to the secondly arrived packet. For without
> > frame capture case, the collided packets are lost. However, for with frame
> > capture case, once the SINR for the secondly arrived packet is strong
> > enough, the secondly arrived packet can be received with a certain
> > probability. 
> > 
> > > > 
> > > > Could you please remind me which your comments I have not addressed? I can
> > > > try my best to provide a new patch today. But I will be traveling from
> > > > tomorrow to next Friday. I may not be able to devote too much time on this
> > > > patch next week.
> > > 
> > > I would mainly like to get further details about the error model you
> > > introduce and how deep it was validated.
> > 
> > I have added some explanations in frame-capture-model.cc and also in our
> > previous discussion, I have explained this several times. The payload
> > capture model was fitted and calibrated based on the results of a set of
> > hardware lab test conducted by CAMP engineers. The test was essentially for
> > DSRC devices and it was for OFDM 6 Mbps signal on 10 MHz channel. However,
> > during the test, the CAMP engineers could not generate a collided signal at
> > preamble portion, so we were not able to calibrate a model as we did for the
> > payload. The preamble capture model was based on try-and-error. We tried to
> > shift an error model (originally based on the Nist model for decoding OFDM 6
> > Mbps signal with BPSK modulation. This is the data rate and the modulation
> > method for the preamble portion of most wifi protocols), so that the
> > simulator can generate the best match to the experiment results. As I
> > mentioned before, this model was not able to be validated across different
> > data rates and wifi protocols. However, we believe this model may still be
> > useful in many wifi networks. Without permissions from CAMP, I could not
> > reveal more lab test data and testing details so far. 
> > 
> > I am not sure whether this information is enough for you. If not, please let
> > me what kind of specific details of validation you want to know. Thanks.
> > 
> > > 
> > > Note that I would also wait for further comments from Tom before pushing,
> > > mainly wrt changes in InterferenceHelper.
> 
> It seems based on 802.11a experiments, but it will be an issue for
> 802.11n/ac/ax/...
> Did you make some trials with those most recent 802.11 versions?

Unfortunately, we did not have test data for other 802.11 versions. But I think the frame capture is more device or chipset dependent instead of 802.11 protocol dependent. The frame capture is a capability that the device can detect the known preamble sequence among other signal energy on the channel. The more sensitive the device, the easier to capture a new frame. This is my understanding. So far, I could not provide data to support this claim.
Comment 61 sebastien.deronne 2017-01-29 09:04:53 EST
(In reply to Bin Cheng from comment #60)
> (In reply to sebastien.deronne from comment #58)
> > (In reply to Bin Cheng from comment #57)
> > > (In reply to sebastien.deronne from comment #56)
> > > > (In reply to Bin Cheng from comment #55)
> > > > > Thanks for the reply. Could you please point out which your comments I have
> > > > > not addressed? I thought I have addressed almost all your comments except
> > > > > adding one more example in example/wireless. My response to your comments
> > > > > can be found on Rietveld as well, in case you have not noticed yet.
> > > > > Actually, the impact of the frame capture, i.e., the comparison between
> > > > > simulations with and without frame capture capability has been covered in
> > > > > the updated test-interference-helper.cc. I have added some explanation to
> > > > > the expected simulation result. And in frame-capture-model-validation, I
> > > > > also mentioned that the impact of the frame capture effect on the simulation
> > > > > results has also been studied in our paper. With all this, I am not sure
> > > > > whether we still need another example in the example/wireless and if so,
> > > > > what else we should include in that example. 
> > > > 
> > > > I would expect an example where you run for example 3 different networks
> > > > where one outputs a legacy performance, one outputs performance using
> > > > preamble capture mode and a last one outputs performance using payload
> > > > capture mode.
> > > > 
> > > Do you want to run all three different configurations in one example and
> > > generate three sets of results at the same time? Otherwise, I still think
> > > this has been covered in test-interference-helper.cc. In
> > > test-interference-helper, different packet collision scenarios were
> > > considered. And generally, the biggest difference between with and without
> > > frame capture is for switching to the secondly arrived packet. For without
> > > frame capture case, the collided packets are lost. However, for with frame
> > > capture case, once the SINR for the secondly arrived packet is strong
> > > enough, the secondly arrived packet can be received with a certain
> > > probability. 
> > > 
> > > > > 
> > > > > Could you please remind me which your comments I have not addressed? I can
> > > > > try my best to provide a new patch today. But I will be traveling from
> > > > > tomorrow to next Friday. I may not be able to devote too much time on this
> > > > > patch next week.
> > > > 
> > > > I would mainly like to get further details about the error model you
> > > > introduce and how deep it was validated.
> > > 
> > > I have added some explanations in frame-capture-model.cc and also in our
> > > previous discussion, I have explained this several times. The payload
> > > capture model was fitted and calibrated based on the results of a set of
> > > hardware lab test conducted by CAMP engineers. The test was essentially for
> > > DSRC devices and it was for OFDM 6 Mbps signal on 10 MHz channel. However,
> > > during the test, the CAMP engineers could not generate a collided signal at
> > > preamble portion, so we were not able to calibrate a model as we did for the
> > > payload. The preamble capture model was based on try-and-error. We tried to
> > > shift an error model (originally based on the Nist model for decoding OFDM 6
> > > Mbps signal with BPSK modulation. This is the data rate and the modulation
> > > method for the preamble portion of most wifi protocols), so that the
> > > simulator can generate the best match to the experiment results. As I
> > > mentioned before, this model was not able to be validated across different
> > > data rates and wifi protocols. However, we believe this model may still be
> > > useful in many wifi networks. Without permissions from CAMP, I could not
> > > reveal more lab test data and testing details so far. 
> > > 
> > > I am not sure whether this information is enough for you. If not, please let
> > > me what kind of specific details of validation you want to know. Thanks.
> > > 
> > > > 
> > > > Note that I would also wait for further comments from Tom before pushing,
> > > > mainly wrt changes in InterferenceHelper.
> > 
> > It seems based on 802.11a experiments, but it will be an issue for
> > 802.11n/ac/ax/...
> > Did you make some trials with those most recent 802.11 versions?
> 
> Unfortunately, we did not have test data for other 802.11 versions. But I
> think the frame capture is more device or chipset dependent instead of
> 802.11 protocol dependent. The frame capture is a capability that the device
> can detect the known preamble sequence among other signal energy on the
> channel. The more sensitive the device, the easier to capture a new frame.
> This is my understanding. So far, I could not provide data to support this
> claim.

Can't we imagine a more generic model? Or at least an extendable model?
I am not really willing to deliver a model that only applies to very specific conditions.
Comment 62 sebastien.deronne 2017-02-13 15:20:11 EST
Hi, could you provide updates? Thanks.
Comment 63 Bin Cheng 2017-02-13 23:32:23 EST
(In reply to sebastien.deronne from comment #62)
> Hi, could you provide updates? Thanks.

I am still working on a new patch. I may be able to provide a new patch later this week. Sorry for the delay.
Comment 64 Bin Cheng 2017-02-18 17:07:31 EST
Created attachment 2786 [details]
Patch remove per check when determining whether the reception can be continue
Comment 65 Bin Cheng 2017-02-18 17:21:29 EST
(In reply to sebastien.deronne from comment #62)
> Hi, could you provide updates? Thanks.

I just uploaded a new version of the patch. This version tries to address Tom's comments. The main changes are:

1) Remove the PER check when determining if the current reception can be continued. So in the new patch, if the new packet passes the packet switching check, the old packet is dropped, otherwise, the new packet is dropped. 

2) Remove the NotifyRxEndError from SwitchFromRxAbort method. But I keep the state switching in the code, i.e., when the old reception is aborted, the station state switches from RX to IDLE or CCA_BUSY. And when starting to receive the new packet,  the state switches back to RX. The reason I still keep this is because 1) I still believe it would be useful if the user can know when a reception is aborted from station state logs. Add a state switching may help to do so. 2) I realize that removing this state switching may lead to much more changes to the existing code, at least based on my understanding. So I decide not to do all these changes at this time. 

3) The implementation of several methods are updated, e.g., merging CheckPreambleCapture and CheckPayloadCapture to CheckFrameCapture, etc. 

Please let me know if you have more comments on the new patch. I am quite busy recently, but I will try to update the patch if any change is needed. Thanks.
Comment 66 sebastien.deronne 2017-02-19 05:51:38 EST
Tom, could you update the review with this new patch? Thanks!
Comment 67 Tom Henderson 2017-02-19 11:04:56 EST
(In reply to sebastien.deronne from comment #66)
> Tom, could you update the review with this new patch? Thanks!

Updated.  http://codereview.appspot.com/319050043
Comment 68 sebastien.deronne 2017-02-21 14:23:58 EST
Unfortunately I still do not approve the latest patch, please refer to my comments in the review.

IMO most devices in the field will compare the received powers and will jump to the reception of another frame if its power is higher. This makes the implementation much simpler than what is proposed now. Another reason I am not in favor of the frame capture model is that it applies to a specific case and does not make it generic. Also, examples are not OK.

If you miss time, please let me know and I'll take over once we agree on the approach.
Comment 69 sebastien.deronne 2017-02-21 14:29:13 EST
(In reply to sebastien.deronne from comment #68)
> Unfortunately I still do not approve the latest patch, please refer to my
> comments in the review.
> 
> IMO most devices in the field will compare the received powers and will jump
> to the reception of another frame if its power is higher. This makes the
> implementation much simpler than what is proposed now. Another reason I am
> not in favor of the frame capture model is that it applies to a specific
> case and does not make it generic. Also, examples are not OK.
> 
> If you miss time, please let me know and I'll take over once we agree on the
> approach.

I would have something like:

if (Pcurrentsignal < Pnewsignal + margin)
{
  SwitchReception ();
}
Comment 70 Bin Cheng 2017-02-21 21:12:49 EST
(In reply to sebastien.deronne from comment #69)
> (In reply to sebastien.deronne from comment #68)
> > Unfortunately I still do not approve the latest patch, please refer to my
> > comments in the review.
> > 
> > IMO most devices in the field will compare the received powers and will jump
> > to the reception of another frame if its power is higher. This makes the
> > implementation much simpler than what is proposed now. Another reason I am
> > not in favor of the frame capture model is that it applies to a specific
> > case and does not make it generic. Also, examples are not OK.
> > 
> > If you miss time, please let me know and I'll take over once we agree on the
> > approach.
> 
> I would have something like:
> 
> if (Pcurrentsignal < Pnewsignal + margin)
> {
>   SwitchReception ();
> }

I am not sure if it is accurate if using actual power values instead of SINR values to indicate when to do the frame capture. First, Most existing models for frame capture, e.g. the one in the ns-2, are based on SINR values. Second, I think the signal power can be contaminated by the interference power, i.e., when measuring the signal power, the interference power is on the channel as well. The power may measure only once, but the SINR calculation could be cross different signal symbols. Therefore, it is possible that the detected signal power (actual signal power + interference) meets the condition you defined (Pcurrentsignal < Pnewsignal + margin), but its actual SINR is low.  Third, howe do we know the margin you defined here is the same for different Wi-Fi standards?

For our model, one change I can add is to make the model parameters, i.e., the parameters we were changing when doing the fitting, configurable in the code. The default value is for our current model, which is validated based on a lab hardware test and a field test with 400  DSRC transmitters on 10 MHz channel with 6 Mbps data rate. Another researcher can modify these model parameters if they fitted the model a similar dataset but with different bandwidth or different datarate. 

In the example I provided, my idea was to identify when a frame capture occurs based on a log, e..g, NS_LOG_INFO (Simulator::Now () << " " << captureType << " switch to new packet"). These logs may be buried in other logs. By counting the number of these logs in the log file, one can figure out how many frame capture occurred for one particular SINR and then calculate the capture probability for that SINR. Similarly, I also use logs to find out when a preamble was decoded and how many preambles are decoded for one SINR. But I have not figure out a better to identify when the frame capture happens. If you have a better idea, please let me know. Thanks.
Comment 71 Ioannis 2017-02-22 08:25:12 EST
Hi Bin,

I didn't have a look at the code, so I only comment on the main capture procedure.

I would agree with Sebastien on RSS instead of SINR. If SINR is used, then first the node has to calculate SINR for the ongoing reception, where S = RSSold and I = I + RSSnew and then again SINR_2 with S = RSSnew and I = I + RSSold. I have the impression that calculating SINR like that on the fly increases complexity. According to the std, upon receiving a PLCP preamble, the RSSI is reported.

As for the Margin, there is nothing defined in the std. However, there are some references defining a value of ~10 dB for Margin. I guess it depends on the chipset, as you discussed earlier.
Comment 72 Tom Henderson 2017-02-22 10:16:40 EST
Bin,
I also would like the implementation to allow for different frame capture models in the future, some which may be SIR based.  Maybe there is an SNIR factor but I think it remains to be checked how much this matters.

For example, this paper reports an SIR based model (although it does not report explicitly the level above the noise floor); it would be preferable if we allow such a model to be added as an option:
P. Fuxjaeger and S. Ruehrup, "Validation of the NS-3 Interference Model for IEEE802.11 Networks," 2015 8th IFIP Wireless and Mobile Networking Conference (WMNC), Munich, 2015, pp. 216-222.

In the ns-3 architecture, I think that this needs to be delegated outside of the WifiPhy; WifiPhy should just tell the InterferenceHelper about the new arrival, and InterferenceHelper should just tell WifiPhy whether or not the new frame is the new receiving frame, or else tell it the probability; i.e. 

- prob = m_interference.CalculatePreambleCaptureProbability (sinrForNewPkt);
+ prob = m_interference.CalculatePreambleCaptureProbability (currentEvent, newEvent);

One small issue is that currentEvent is bound up in an ns3::Event as an argument and not separately tracked, but we could keep track of it explicitly in a state variable when we schedule m_endPlcpRxEvent; e.g. m_currentRxEvent = event;

In the InterferenceHelper, I would pass these events both to the frame capture model, along with the noise power (including any noise figure).

 - return m_frameCaptureModel->CalculatePreambleCaptureProbability (sinr);
 + return m_frameCaptureModel->CalculatePreambleCaptureProbability (currentEvent, newEvent, noisePowerW);

and since the events have all of the information needed to either compute SIR or  SNIR, the model can do what it wants.

Well, there is one small issue still, that an InterferenceHelper::Event does not track event start time, so we can't learn the offset in time between two events, but that could be added to the class.
Comment 73 Tom Henderson 2017-02-22 10:24:19 EST
> 
> In the example I provided, my idea was to identify when a frame capture
> occurs based on a log, e..g, NS_LOG_INFO (Simulator::Now () << " " <<
> captureType << " switch to new packet"). These logs may be buried in other
> logs. By counting the number of these logs in the log file, one can figure
> out how many frame capture occurred for one particular SINR and then
> calculate the capture probability for that SINR. Similarly, I also use logs
> to find out when a preamble was decoded and how many preambles are decoded
> for one SINR. But I have not figure out a better to identify when the frame
> capture happens. If you have a better idea, please let me know. Thanks.


The ns-3 way to do this is to export a traced callback such as done in the WifiPhyStateHelper: 
                    .AddTraceSource ("RxOk",
                     "A packet has been received successfully.",
                     MakeTraceSourceAccessor (&WifiPhyStateHelper::m_rxOkTrace),

and keep the log statement, so users can either do the quick and dirty counting of log statements, or connect a trace sink and use that (and our provided example should definitely use the trace source).

We need to decide what trace is actually fired when this event occurs.  It seems to me that perhaps we call PhyRxDrop on the dropped frame, and both a PhyRxBegin and a new trace, PhyRxCapture, on the new frame.
Comment 74 sebastien.deronne 2017-02-22 14:14:02 EST
I actually do not bother whether you compare the received powers or the SINR, even though complexity-wise I think most manufactures are going for power comparisons.
So in summary I think the non-generic error model should simply be removed and we should go for power and/or SNIR comparison.

I also agree with Tom that the InterferenceHelper should be more involved in such computations, and that the phy should hold the current event.

A new trace source is indeed something we need to indicate whether a frame switch occurred.
Comment 75 Bin Cheng 2017-02-24 19:53:21 EST
(In reply to Ioannis from comment #71)
> Hi Bin,
> 
> I didn't have a look at the code, so I only comment on the main capture
> procedure.
> 
> I would agree with Sebastien on RSS instead of SINR. If SINR is used, then
> first the node has to calculate SINR for the ongoing reception, where S =
> RSSold and I = I + RSSnew and then again SINR_2 with S = RSSnew and I = I +
> RSSold. I have the impression that calculating SINR like that on the fly
> increases complexity. According to the std, upon receiving a PLCP preamble,
> the RSSI is reported.
> 
> As for the Margin, there is nothing defined in the std. However, there are
> some references defining a value of ~10 dB for Margin. I guess it depends on
> the chipset, as you discussed earlier.

Thanks, Ioannis. I think ~10dB as the margin makes sense.
Comment 76 Bin Cheng 2017-02-24 19:56:25 EST
(In reply to Ioannis from comment #71)
> Hi Bin,
> 
> I didn't have a look at the code, so I only comment on the main capture
> procedure.
> 
> I would agree with Sebastien on RSS instead of SINR. If SINR is used, then
> first the node has to calculate SINR for the ongoing reception, where S =
> RSSold and I = I + RSSnew and then again SINR_2 with S = RSSnew and I = I +
> RSSold. I have the impression that calculating SINR like that on the fly
> increases complexity. According to the std, upon receiving a PLCP preamble,
> the RSSI is reported.
> 
> As for the Margin, there is nothing defined in the std. However, there are
> some references defining a value of ~10 dB for Margin. I guess it depends on
> the chipset, as you discussed earlier.

Thanks, Ioannis. I think ~10dB as the margin makes sense. (In reply to Tom Henderson from comment #73)
> > 
> > In the example I provided, my idea was to identify when a frame capture
> > occurs based on a log, e..g, NS_LOG_INFO (Simulator::Now () << " " <<
> > captureType << " switch to new packet"). These logs may be buried in other
> > logs. By counting the number of these logs in the log file, one can figure
> > out how many frame capture occurred for one particular SINR and then
> > calculate the capture probability for that SINR. Similarly, I also use logs
> > to find out when a preamble was decoded and how many preambles are decoded
> > for one SINR. But I have not figure out a better to identify when the frame
> > capture happens. If you have a better idea, please let me know. Thanks.
> 
> 
> The ns-3 way to do this is to export a traced callback such as done in the
> WifiPhyStateHelper: 
>                     .AddTraceSource ("RxOk",
>                      "A packet has been received successfully.",
>                      MakeTraceSourceAccessor
> (&WifiPhyStateHelper::m_rxOkTrace),
> 
> and keep the log statement, so users can either do the quick and dirty
> counting of log statements, or connect a trace sink and use that (and our
> provided example should definitely use the trace source).
> 
> We need to decide what trace is actually fired when this event occurs.  It
> seems to me that perhaps we call PhyRxDrop on the dropped frame, and both a
> PhyRxBegin and a new trace, PhyRxCapture, on the new frame.

Thanks, Tom. This is a way better solution to indicate the packet switching. I thin adding a new trace is really necessary. I agree with you that PhyRxCapture should be called the reception of the new packet starts.
Comment 77 Bin Cheng 2017-02-24 20:04:27 EST
(In reply to sebastien.deronne from comment #74)
> I actually do not bother whether you compare the received powers or the
> SINR, even though complexity-wise I think most manufactures are going for
> power comparisons.
> So in summary I think the non-generic error model should simply be removed
> and we should go for power and/or SNIR comparison.
> 
> I also agree with Tom that the InterferenceHelper should be more involved in
> such computations, and that the phy should hold the current event.
> 
> A new trace source is indeed something we need to indicate whether a frame
> switch occurred.

I would suggest using SINR as the indicator. I already implemented a function to calculate SINR for any given event. If you like it, we can still keep it. If we are so worried about the non-generic model as the one we proposed, let us then simply use a threshold based solution, either comparing SINR or power.  

Regarding all the new changes we just discussed, e.g., adding new trace source, or changing the model to a threshold based approach, etc., I may need some time to finish them. If you are in hurry and willing to take over, please go ahead without me. I can help once I have more time. 

I am sorry for the delayed responses and please let me if you have any new questions which we need to discuss about.
Comment 78 sebastien.deronne 2017-03-06 16:49:47 EST
Created attachment 2789 [details]
New proposal based on rx power comparison

I made a first rework based on previous discussions.

Few assumptions:
- Consider capture during preamble duration. This could be tuned in the future.
- Consider a default margin of 10 dB, this is configurable through an attribute.

I also need to update documentation and add an example showing benefits of PLC mode.
I'll do that later this week. In the meantime, feel free to post your comments so that I can finish this up.
Comment 79 Tom Henderson 2017-03-07 10:33:04 EST
(In reply to sebastien.deronne from comment #78)
> Created attachment 2789 [details]
> New proposal based on rx power comparison
> 
> I made a first rework based on previous discussions.
> 
> Few assumptions:
> - Consider capture during preamble duration. This could be tuned in the
> future.
> - Consider a default margin of 10 dB, this is configurable through an
> attribute.
> 
> I also need to update documentation and add an example showing benefits of
> PLC mode.
> I'll do that later this week. In the meantime, feel free to post your
> comments so that I can finish this up.

Sebastien, I'm sorry but I think this model is too rigid.  It does not allow consideration of time-arrival delay which may influence the probability, such as shown in the Fuxjaeger reference I listed above.  Can we not delegate this to a frame capture model rather than encode it in WifiPhy?

For example, this kind of statement:
+      if (m_physicalLayerCaptureEnabled
+          && txVector.GetPreambleType () != WIFI_PREAMBLE_NONE
+          && WToDbm (m_currentEvent->GetRxPowerW ()) + m_physicalLayerCaptureMargin < WToDbm (rxPowerW)
+          && m_currentEvent->GetStartTime () + GetPlcpPreambleDuration (m_currentEvent->GetTxVector ()) < Simulator::Now ())

could be instead a query to a frame capture object, which could be an ns3::Object and stored by PointerValue in the WifiPhy attributes, with a default implementation that is simply a threshold?

Also, I did not spot a NotifyRxDrop() in the AbortCurrentReception() case.

Also, you moved all of the logic for checking 'is this a preamble?' to StartRx(), but this logic does not seem to be common for the two cases that call StartRx () (from IDLE or from capture) because you are already checking for preamble separately in the capture logic before calling StartRx() there.
Comment 80 sebastien.deronne 2017-03-08 15:57:35 EST
> Sebastien, I'm sorry but I think this model is too rigid.  It does not allow
> consideration of time-arrival delay which may influence the probability,
> such as shown in the Fuxjaeger reference I listed above.  

I agree the SIR depends on the moment the new incoming frame arrive, but I thought we agreed to not model it for now and let for future improvement.
How do you suggest to handle this? If this can be handled in the remaining time frame, then I'll go for it.

Can we not
> delegate this to a frame capture model rather than encode it in WifiPhy?
> 
> For example, this kind of statement:
> +      if (m_physicalLayerCaptureEnabled
> +          && txVector.GetPreambleType () != WIFI_PREAMBLE_NONE
> +          && WToDbm (m_currentEvent->GetRxPowerW ()) +
> m_physicalLayerCaptureMargin < WToDbm (rxPowerW)
> +          && m_currentEvent->GetStartTime () + GetPlcpPreambleDuration
> (m_currentEvent->GetTxVector ()) < Simulator::Now ())
> 
> could be instead a query to a frame capture object, which could be an
> ns3::Object and stored by PointerValue in the WifiPhy attributes, with a
> default implementation that is simply a threshold?

This sounds better, I will have a look at how I can handle this.

> 
> Also, I did not spot a NotifyRxDrop() in the AbortCurrentReception() case.

This is indeed missing, I will add it.

> 
> Also, you moved all of the logic for checking 'is this a preamble?' to
> StartRx(), but this logic does not seem to be common for the two cases that
> call StartRx () (from IDLE or from capture) because you are already checking
> for preamble separately in the capture logic before calling StartRx() there.

I do not really understand. StarRx is not checking if this is a preamble, but more handling A-MPDUs. I added a condition to only switch reception if the new frame is an MPDU or the first frame in an A-MPDU. So all cases in StartRx will not be hit for frame switch but this avoids code duplication.
Now that I am looking at it, I think condition should anyway be modified to check whether we are currently receiving an A-MPDU for which the preamble was received.
Please detail if I am misunderstanding what you meant.
Comment 81 Tom Henderson 2017-03-08 16:53:50 EST
(In reply to sebastien.deronne from comment #80)
> > Sebastien, I'm sorry but I think this model is too rigid.  It does not allow
> > consideration of time-arrival delay which may influence the probability,
> > such as shown in the Fuxjaeger reference I listed above.  
> 
> I agree the SIR depends on the moment the new incoming frame arrive, but I
> thought we agreed to not model it for now and let for future improvement.
> How do you suggest to handle this? If this can be handled in the remaining
> time frame, then I'll go for it.

I am more concerned about allowing for a model in which the probability of capture depends on how far into the frame the late arriving frame comes (e.g. if it arrives in the last symbol, versus arriving earlier).

But I agree that for the moment, a simple threshold is sufficient.


> Can we not
> > delegate this to a frame capture model rather than encode it in WifiPhy?
> > 
> > For example, this kind of statement:
> > +      if (m_physicalLayerCaptureEnabled
> > +          && txVector.GetPreambleType () != WIFI_PREAMBLE_NONE
> > +          && WToDbm (m_currentEvent->GetRxPowerW ()) +
> > m_physicalLayerCaptureMargin < WToDbm (rxPowerW)
> > +          && m_currentEvent->GetStartTime () + GetPlcpPreambleDuration
> > (m_currentEvent->GetTxVector ()) < Simulator::Now ())
> > 
> > could be instead a query to a frame capture object, which could be an
> > ns3::Object and stored by PointerValue in the WifiPhy attributes, with a
> > default implementation that is simply a threshold?
> 
> This sounds better, I will have a look at how I can handle this.

OK

> 
> > 
> > Also, I did not spot a NotifyRxDrop() in the AbortCurrentReception() case.
> 
> This is indeed missing, I will add it.
> 
> > 
> > Also, you moved all of the logic for checking 'is this a preamble?' to
> > StartRx(), but this logic does not seem to be common for the two cases that
> > call StartRx () (from IDLE or from capture) because you are already checking
> > for preamble separately in the capture logic before calling StartRx() there.
> 
> I do not really understand. StarRx is not checking if this is a preamble,
> but more handling A-MPDUs. I added a condition to only switch reception if
> the new frame is an MPDU or the first frame in an A-MPDU. So all cases in
> StartRx will not be hit for frame switch but this avoids code duplication.
> Now that I am looking at it, I think condition should anyway be modified to
> check whether we are currently receiving an A-MPDU for which the preamble
> was received.
> Please detail if I am misunderstanding what you meant.

Yes, my point was that I didn't think that simply moving the code from the IDLE processing into StartRx was the right thing since it causes checking for preamble in multiple places (within StartRx and also before entering it) in the RX path.
Comment 82 sebastien.deronne 2017-03-12 15:53:47 EDT
(In reply to Tom Henderson from comment #81)
> (In reply to sebastien.deronne from comment #80)
> > > Sebastien, I'm sorry but I think this model is too rigid.  It does not allow
> > > consideration of time-arrival delay which may influence the probability,
> > > such as shown in the Fuxjaeger reference I listed above.  
> > 
> > I agree the SIR depends on the moment the new incoming frame arrive, but I
> > thought we agreed to not model it for now and let for future improvement.
> > How do you suggest to handle this? If this can be handled in the remaining
> > time frame, then I'll go for it.
> 
> I am more concerned about allowing for a model in which the probability of
> capture depends on how far into the frame the late arriving frame comes
> (e.g. if it arrives in the last symbol, versus arriving earlier).
> 
> But I agree that for the moment, a simple threshold is sufficient.
> 
> 
> > Can we not
> > > delegate this to a frame capture model rather than encode it in WifiPhy?
> > > 
> > > For example, this kind of statement:
> > > +      if (m_physicalLayerCaptureEnabled
> > > +          && txVector.GetPreambleType () != WIFI_PREAMBLE_NONE
> > > +          && WToDbm (m_currentEvent->GetRxPowerW ()) +
> > > m_physicalLayerCaptureMargin < WToDbm (rxPowerW)
> > > +          && m_currentEvent->GetStartTime () + GetPlcpPreambleDuration
> > > (m_currentEvent->GetTxVector ()) < Simulator::Now ())
> > > 
> > > could be instead a query to a frame capture object, which could be an
> > > ns3::Object and stored by PointerValue in the WifiPhy attributes, with a
> > > default implementation that is simply a threshold?
> > 
> > This sounds better, I will have a look at how I can handle this.
> 
> OK
> 
> > 
> > > 
> > > Also, I did not spot a NotifyRxDrop() in the AbortCurrentReception() case.
> > 
> > This is indeed missing, I will add it.
> > 
> > > 
> > > Also, you moved all of the logic for checking 'is this a preamble?' to
> > > StartRx(), but this logic does not seem to be common for the two cases that
> > > call StartRx () (from IDLE or from capture) because you are already checking
> > > for preamble separately in the capture logic before calling StartRx() there.
> > 
> > I do not really understand. StarRx is not checking if this is a preamble,
> > but more handling A-MPDUs. I added a condition to only switch reception if
> > the new frame is an MPDU or the first frame in an A-MPDU. So all cases in
> > StartRx will not be hit for frame switch but this avoids code duplication.
> > Now that I am looking at it, I think condition should anyway be modified to
> > check whether we are currently receiving an A-MPDU for which the preamble
> > was received.
> > Please detail if I am misunderstanding what you meant.
> 
> Yes, my point was that I didn't think that simply moving the code from the
> IDLE processing into StartRx was the right thing since it causes checking
> for preamble in multiple places (within StartRx and also before entering it)
> in the RX path.

I could remove the StartRx function, but then we will end up with duplicate code. I agree some blocks will never be used when reception is switched (i.e. only cases where preamble is received will be hit), but still a big part of StartRx will have to be written twice.
Comment 83 Ioannis 2017-03-14 14:31:06 EDT
I did not spot any changes in MacLow. In particular, in DeaggregateAmpduAndReceive where the Block Ack is set. If capture occurs during payload and the new first A-MPDU frame is lost, the Block Ack is not set. For example something like 

else if (preamble != WIFI_PREAMBLE_NONE || !m_sendAckEvent.IsRunning () || (m_addrFrom != firsthdr.GetAddr2 ())) //Capture Effect occurred 
            {
              m_addrFrom = firsthdr.GetAddr2 ();
Comment 84 Tom Henderson 2017-03-14 17:34:27 EDT
> > > Please detail if I am misunderstanding what you meant.
> > 
> > Yes, my point was that I didn't think that simply moving the code from the
> > IDLE processing into StartRx was the right thing since it causes checking
> > for preamble in multiple places (within StartRx and also before entering it)
> > in the RX path.
> 
> I could remove the StartRx function, but then we will end up with duplicate
> code. I agree some blocks will never be used when reception is switched
> (i.e. only cases where preamble is received will be hit), but still a big
> part of StartRx will have to be written twice.

OK, it is not a major concern of mine, so I am fine to leave as is.
Comment 85 sebastien.deronne 2017-03-15 13:54:01 EDT
(In reply to Ioannis from comment #83)
> I did not spot any changes in MacLow. In particular, in
> DeaggregateAmpduAndReceive where the Block Ack is set. If capture occurs
> during payload and the new first A-MPDU frame is lost, the Block Ack is not
> set. For example something like 
> 
> else if (preamble != WIFI_PREAMBLE_NONE || !m_sendAckEvent.IsRunning () ||
> (m_addrFrom != firsthdr.GetAddr2 ())) //Capture Effect occurred 
>             {
>               m_addrFrom = firsthdr.GetAddr2 ();

Ioannis, for the moment we assume capture only occur during preamble, so it should not affect A-MPDU. I am ok to extend and to no limit to preamble duration if you have some proposals.
Comment 86 sebastien.deronne 2017-03-15 14:22:25 EDT
> > > Can we not
> > > > delegate this to a frame capture model rather than encode it in WifiPhy?
> > > > 
> > > > For example, this kind of statement:
> > > > +      if (m_physicalLayerCaptureEnabled
> > > > +          && txVector.GetPreambleType () != WIFI_PREAMBLE_NONE
> > > > +          && WToDbm (m_currentEvent->GetRxPowerW ()) +
> > > > m_physicalLayerCaptureMargin < WToDbm (rxPowerW)
> > > > +          && m_currentEvent->GetStartTime () + GetPlcpPreambleDuration
> > > > (m_currentEvent->GetTxVector ()) < Simulator::Now ())
> > > > 
> > > > could be instead a query to a frame capture object, which could be an
> > > > ns3::Object and stored by PointerValue in the WifiPhy attributes, with a
> > > > default implementation that is simply a threshold?
> > > 
> > > This sounds better, I will have a look at how I can handle this.
> > 
> > OK
> > 

Tom, do you expect m_physicalLayerCaptureEnabled and m_physicalLayerCaptureMargin to be part of WifiPhy or should those be moved to the frame capture object?
Comment 87 Tom Henderson 2017-03-18 18:03:29 EDT
(In reply to sebastien.deronne from comment #86)

> 
> Tom, do you expect m_physicalLayerCaptureEnabled and
> m_physicalLayerCaptureMargin to be part of WifiPhy or should those be moved
> to the frame capture object?


I might first try to define an attribute on WifiPhy, assuming that by default we will want ns-3 to provide a basic capture model going forward.

    .AddAttribute ("FrameCaptureModel",
                   "Ptr to an object that implements the frame capture model",
                   StringValue ("ns3::FrameCaptureModel"),
                   MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
                   MakePointerChecker <FrameCaptureModel>())

then in the code, m_physicalLayerCaptureEnabled is not needed; it can just be a check to see if m_frameCaptureModel is not null.  The margin can then be in the FrameCaptureModel.  

It may make sense to make FrameCaptureModel an abstract base class and provide a default, margin-based "SimpleFrameCaptureModel" that has the attribute "Margin".  This would avoid a base-class margin attribute that might not apply to some future subclass.  This would change the above to:

    .AddAttribute ("FrameCaptureModel",
                   "Ptr to an object that implements the frame capture model",
                   StringValue ("ns3::SimpleFrameCaptureModel"),
                   MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
                   MakePointerChecker <FrameCaptureModel>())
Comment 88 sebastien.deronne 2017-03-26 04:30:54 EDT
(In reply to Tom Henderson from comment #87)
> (In reply to sebastien.deronne from comment #86)
> 
> > 
> > Tom, do you expect m_physicalLayerCaptureEnabled and
> > m_physicalLayerCaptureMargin to be part of WifiPhy or should those be moved
> > to the frame capture object?
> 
> 
> I might first try to define an attribute on WifiPhy, assuming that by
> default we will want ns-3 to provide a basic capture model going forward.
> 
>     .AddAttribute ("FrameCaptureModel",
>                    "Ptr to an object that implements the frame capture
> model",
>                    StringValue ("ns3::FrameCaptureModel"),
>                    MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
>                    MakePointerChecker <FrameCaptureModel>())
> 
> then in the code, m_physicalLayerCaptureEnabled is not needed; it can just
> be a check to see if m_frameCaptureModel is not null.  The margin can then
> be in the FrameCaptureModel.  
> 
> It may make sense to make FrameCaptureModel an abstract base class and
> provide a default, margin-based "SimpleFrameCaptureModel" that has the
> attribute "Margin".  This would avoid a base-class margin attribute that
> might not apply to some future subclass.  This would change the above to:
> 
>     .AddAttribute ("FrameCaptureModel",
>                    "Ptr to an object that implements the frame capture
> model",
>                    StringValue ("ns3::SimpleFrameCaptureModel"),
>                    MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
>                    MakePointerChecker <FrameCaptureModel>())

Tom, I like your proposal and I will go for it.
I just think that the frameCaptureModel should be null by default, right?
So StringValue ("ns3::SimpleFrameCaptureModel") should be changed (empty StringValue?)
Comment 89 sebastien.deronne 2017-03-26 04:43:09 EDT
(In reply to sebastien.deronne from comment #88)
> (In reply to Tom Henderson from comment #87)
> > (In reply to sebastien.deronne from comment #86)
> > 
> > > 
> > > Tom, do you expect m_physicalLayerCaptureEnabled and
> > > m_physicalLayerCaptureMargin to be part of WifiPhy or should those be moved
> > > to the frame capture object?
> > 
> > 
> > I might first try to define an attribute on WifiPhy, assuming that by
> > default we will want ns-3 to provide a basic capture model going forward.
> > 
> >     .AddAttribute ("FrameCaptureModel",
> >                    "Ptr to an object that implements the frame capture
> > model",
> >                    StringValue ("ns3::FrameCaptureModel"),
> >                    MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
> >                    MakePointerChecker <FrameCaptureModel>())
> > 
> > then in the code, m_physicalLayerCaptureEnabled is not needed; it can just
> > be a check to see if m_frameCaptureModel is not null.  The margin can then
> > be in the FrameCaptureModel.  
> > 
> > It may make sense to make FrameCaptureModel an abstract base class and
> > provide a default, margin-based "SimpleFrameCaptureModel" that has the
> > attribute "Margin".  This would avoid a base-class margin attribute that
> > might not apply to some future subclass.  This would change the above to:
> > 
> >     .AddAttribute ("FrameCaptureModel",
> >                    "Ptr to an object that implements the frame capture
> > model",
> >                    StringValue ("ns3::SimpleFrameCaptureModel"),
> >                    MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
> >                    MakePointerChecker <FrameCaptureModel>())
> 
> Tom, I like your proposal and I will go for it.
> I just think that the frameCaptureModel should be null by default, right?
> So StringValue ("ns3::SimpleFrameCaptureModel") should be changed (empty
> StringValue?)

Other possibility is to have the default method that returns always false when we request whether reception should be switched. Please let me know what you prefer.
Comment 90 Tom Henderson 2017-03-26 10:08:00 EDT
> > 
> > Tom, I like your proposal and I will go for it.
> > I just think that the frameCaptureModel should be null by default, right?
> > So StringValue ("ns3::SimpleFrameCaptureModel") should be changed (empty
> > StringValue?)
> 
> Other possibility is to have the default method that returns always false
> when we request whether reception should be switched. Please let me know
> what you prefer.

I would prefer that the long term default configuration for wifi phy have an enabled frame capture model.  Whether we should do that on the eve of a release without any such model in hand yet is another question.
Comment 91 sebastien.deronne 2017-03-26 13:48:15 EDT
(In reply to Tom Henderson from comment #90)
> > > 
> > > Tom, I like your proposal and I will go for it.
> > > I just think that the frameCaptureModel should be null by default, right?
> > > So StringValue ("ns3::SimpleFrameCaptureModel") should be changed (empty
> > > StringValue?)
> > 
> > Other possibility is to have the default method that returns always false
> > when we request whether reception should be switched. Please let me know
> > what you prefer.
> 
> I would prefer that the long term default configuration for wifi phy have an
> enabled frame capture model.  Whether we should do that on the eve of a
> release without any such model in hand yet is another question.

Ok, I understand your point to have SimpleFrameCaptureModel. 
I agree this is expected to be more performant than no frame capture at all, but maybe users might get different results than what they expect because they did not consider this effect? 
I also do not want to enable it at that moment in time since we are close to the release date as you said, how do you advise me to have it disabled for the time being? I see two solutions: have a null pointer by default (not sure how to do it with attributes) or have the base class that always returns false.
Comment 92 Tom Henderson 2017-03-27 17:11:58 EDT
(In reply to sebastien.deronne from comment #91)
> (In reply to Tom Henderson from comment #90)
> > > > 
> > > > Tom, I like your proposal and I will go for it.
> > > > I just think that the frameCaptureModel should be null by default, right?
> > > > So StringValue ("ns3::SimpleFrameCaptureModel") should be changed (empty
> > > > StringValue?)
> > > 
> > > Other possibility is to have the default method that returns always false
> > > when we request whether reception should be switched. Please let me know
> > > what you prefer.
> > 
> > I would prefer that the long term default configuration for wifi phy have an
> > enabled frame capture model.  Whether we should do that on the eve of a
> > release without any such model in hand yet is another question.
> 
> Ok, I understand your point to have SimpleFrameCaptureModel. 
> I agree this is expected to be more performant than no frame capture at all,
> but maybe users might get different results than what they expect because
> they did not consider this effect? 
> I also do not want to enable it at that moment in time since we are close to
> the release date

OK, I would be fine with configured off by default for the next release.

 as you said, how do you advise me to have it disabled for
> the time being? I see two solutions: have a null pointer by default (not
> sure how to do it with attributes) or have the base class that always
> returns false.

Having a null StringValue will not work, but a null PointerValue should work, as long as you check for the pointer being valid before using:

     .AddAttribute ("FrameCaptureModel",
                    "Ptr to an object that implements the frame capture model",
                    PointerValue (0),
                    MakePointerAccessor (&WifiPhy::m_frameCaptureModel),
                    MakePointerChecker <FrameCaptureModel>())


which can be changed to StringValue when we are ready to provide one by default.
Comment 93 sebastien.deronne 2017-03-29 17:18:32 EDT
Created attachment 2814 [details]
new patch addressing Tom's comments

Tom, thanks.
Here is the new patch.
Comment 94 Tom Henderson 2017-03-30 00:33:28 EDT
(In reply to sebastien.deronne from comment #93)
> Created attachment 2814 [details]
> new patch addressing Tom's comments
> 
> Tom, thanks.
> Here is the new patch.

Thanks, just a few comments:

1) I did not spot a call to a drop trace for the aborted frame
2) the method CalculateFrameCapture could perhaps be better named CaptureNewFrame ('calculate' suggests to me that there is some non-boolean value being returned such as a PER)
3) in a few places, s/should be switch/should be switched/
4) class doxygen for SimpleFrameCaptureModel could state more about it (that it is a threshold model)
Comment 95 Tom Henderson 2017-03-31 12:24:51 EDT
suggested regression test case:

Send weak packet A, strong packet B a few microseconds later, to a receiver.  B's power is above the threshold margin with respect to A's power.  With frame capture disabled, both are dropped and B dropped before A is dropped (examine drop trace).  With frame capture enabled, only packet A is dropped.
Comment 96 sebastien.deronne 2017-04-01 04:41:12 EDT
(In reply to Tom Henderson from comment #94)
> (In reply to sebastien.deronne from comment #93)
> > Created attachment 2814 [details]
> > new patch addressing Tom's comments
> > 
> > Tom, thanks.
> > Here is the new patch.
> 
> Thanks, just a few comments:
> 
> 1) I did not spot a call to a drop trace for the aborted frame

I will add.

> 2) the method CalculateFrameCapture could perhaps be better named
> CaptureNewFrame ('calculate' suggests to me that there is some non-boolean
> value being returned such as a PER)

Agreed, I was myself not fully happy with the name :-)

> 3) in a few places, s/should be switch/should be switched/

I will correct.

> 4) class doxygen for SimpleFrameCaptureModel could state more about it (that
> it is a threshold model)

Ok.
Comment 97 sebastien.deronne 2017-04-01 04:42:54 EDT
(In reply to Tom Henderson from comment #95)
> suggested regression test case:
> 
> Send weak packet A, strong packet B a few microseconds later, to a receiver.
> B's power is above the threshold margin with respect to A's power.  With
> frame capture disabled, both are dropped and B dropped before A is dropped
> (examine drop trace).  With frame capture enabled, only packet A is dropped.

I believe the interference helper example can be immediately used for that?
It just needs to be adapted to examine drop trace. Do you agree with that, or do you prefer I add a new test file?
Comment 98 Tom Henderson 2017-04-07 00:17:38 EDT
(In reply to sebastien.deronne from comment #97)
> (In reply to Tom Henderson from comment #95)
> > suggested regression test case:
> > 
> > Send weak packet A, strong packet B a few microseconds later, to a receiver.
> > B's power is above the threshold margin with respect to A's power.  With
> > frame capture disabled, both are dropped and B dropped before A is dropped
> > (examine drop trace).  With frame capture enabled, only packet A is dropped.
> 
> I believe the interference helper example can be immediately used for that?
> It just needs to be adapted to examine drop trace. Do you agree with that,
> or do you prefer I add a new test file?

I would be fine with extending the interference helper, thanks.
Comment 99 sebastien.deronne 2017-04-09 15:19:11 EDT
Created attachment 2829 [details]
new patch that includes a test in the regression

Tom, I made a new patch addressing your latest concerns.
Comment 100 Tom Henderson 2017-04-12 14:25:50 EDT
(In reply to sebastien.deronne from comment #99)
> Created attachment 2829 [details]
> new patch that includes a test in the regression
> 
> Tom, I made a new patch addressing your latest concerns.

It looks good, thanks.  One small clarification that I suggest is that you emphasize in the doxygen of SimpleFrameCaptureModel that this model only operates on the legacy L-STF and L-LTF fields and not on other training fields (e.g. HT-LTF) that exist for newer standards.
Comment 101 sebastien.deronne 2017-04-18 14:12:51 EDT
(In reply to Tom Henderson from comment #100)
> (In reply to sebastien.deronne from comment #99)
> > Created attachment 2829 [details]
> > new patch that includes a test in the regression
> > 
> > Tom, I made a new patch addressing your latest concerns.
> 
> It looks good, thanks.  One small clarification that I suggest is that you
> emphasize in the doxygen of SimpleFrameCaptureModel that this model only
> operates on the legacy L-STF and L-LTF fields and not on other training
> fields (e.g. HT-LTF) that exist for newer standards.

Mmhh, we could avoid such limitation using CalculatePlcpPreambleAndHeaderDuration iso GetPlcpPreambleDuration? Or am I misunderstanding what you mean?
Comment 102 sebastien.deronne 2017-05-03 15:42:37 EDT
Tom, can we conclude on this?
Comment 103 Tom Henderson 2017-05-04 16:05:33 EDT
(In reply to sebastien.deronne from comment #101)
> (In reply to Tom Henderson from comment #100)
> > (In reply to sebastien.deronne from comment #99)
> > > Created attachment 2829 [details]
> > > new patch that includes a test in the regression
> > > 
> > > Tom, I made a new patch addressing your latest concerns.
> > 
> > It looks good, thanks.  One small clarification that I suggest is that you
> > emphasize in the doxygen of SimpleFrameCaptureModel that this model only
> > operates on the legacy L-STF and L-LTF fields and not on other training
> > fields (e.g. HT-LTF) that exist for newer standards.
> 
> Mmhh, we could avoid such limitation using
> CalculatePlcpPreambleAndHeaderDuration iso GetPlcpPreambleDuration? Or am I
> misunderstanding what you mean?

That is another way to handle it.  I was suggesting that it just document that it was the initial L-STF/L-LTF duration, but the threshold could instead apply to the fuller duration, as you suggest.

So I would be fine with either changing to CalculatePlcpPreambleAndHeaderDuration(), or the documentation that I suggested.
Comment 104 sebastien.deronne 2017-05-06 11:17:44 EDT
Tom, OK, thanks for your feedback. I will move to CalculatePlcpPreambleAndHeaderDuration and deliver the code.
Comment 105 sebastien.deronne 2017-05-06 12:03:44 EDT
pushed in changeset 12855:99b57d734bde