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

Issue 144050: Full Compressed Block Ack support

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 3 months ago by Mirko Banchi
Modified:
10 years, 11 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Patch Set 1 #

Total comments: 70

Patch Set 2 : first review #

Total comments: 4

Patch Set 3 : remove uneeded methods in BlockAckManager and fix comments' style #

Total comments: 8

Patch Set 4 : second review #

Patch Set 5 : third review #

Total comments: 22

Patch Set 6 : remove circular dependence among objects #

Patch Set 7 : build fix #

Total comments: 6

Patch Set 8 : fourth review #

Total comments: 9

Patch Set 9 : fifth review #

Patch Set 10 : align with ns-3-dev #

Patch Set 11 : possible final patch #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+5123 lines, -49 lines) Patch
A examples/wireless/wifi-blockack.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +141 lines, -0 lines 0 comments Download
M examples/wireless/wscript View 1 chunk +3 lines, -0 lines 0 comments Download
A src/devices/wifi/block-ack-agreement.h View 1 2 3 4 5 6 7 8 1 chunk +68 lines, -0 lines 0 comments Download
A src/devices/wifi/block-ack-agreement.cc View 1 2 3 4 5 6 7 8 1 chunk +119 lines, -0 lines 0 comments Download
A src/devices/wifi/block-ack-manager.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +297 lines, -0 lines 0 comments Download
A src/devices/wifi/block-ack-manager.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +617 lines, -0 lines 0 comments Download
A src/devices/wifi/block-ack-test-suite.cc View 8 9 10 1 chunk +214 lines, -0 lines 0 comments Download
A src/devices/wifi/ctrl-headers.h View 1 2 3 4 1 chunk +159 lines, -0 lines 0 comments Download
A src/devices/wifi/ctrl-headers.cc View 1 2 3 4 1 chunk +681 lines, -0 lines 0 comments Download
M src/devices/wifi/edca-txop-n.h View 1 2 3 4 5 6 7 8 9 10 7 chunks +50 lines, -0 lines 0 comments Download
M src/devices/wifi/edca-txop-n.cc View 1 2 3 4 5 6 7 8 9 10 20 chunks +429 lines, -28 lines 1 comment Download
M src/devices/wifi/mac-low.h View 1 2 3 4 5 6 7 8 9 10 17 chunks +188 lines, -1 line 0 comments Download
M src/devices/wifi/mac-low.cc View 1 2 3 4 5 6 7 8 9 10 20 chunks +604 lines, -3 lines 0 comments Download
M src/devices/wifi/mac-tx-middle.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/devices/wifi/mac-tx-middle.cc View 1 chunk +13 lines, -0 lines 0 comments Download
M src/devices/wifi/mgt-headers.h View 1 2 3 4 5 6 7 8 9 7 chunks +123 lines, -0 lines 1 comment Download
M src/devices/wifi/mgt-headers.cc View 1 2 3 4 5 6 7 8 9 6 chunks +467 lines, -0 lines 1 comment Download
A src/devices/wifi/originator-block-ack-agreement.h View 8 1 chunk +115 lines, -0 lines 0 comments Download
A src/devices/wifi/originator-block-ack-agreement.cc View 1 chunk +82 lines, -0 lines 0 comments Download
M src/devices/wifi/qadhoc-wifi-mac.h View 3 chunks +7 lines, -1 line 1 comment Download
M src/devices/wifi/qadhoc-wifi-mac.cc View 1 2 3 4 5 6 7 8 9 10 5 chunks +117 lines, -1 line 0 comments Download
M src/devices/wifi/qap-wifi-mac.h View 1 2 3 4 5 6 7 8 9 3 chunks +6 lines, -0 lines 0 comments Download
M src/devices/wifi/qap-wifi-mac.cc View 1 2 3 4 5 6 7 8 9 10 5 chunks +119 lines, -1 line 2 comments Download
A src/devices/wifi/qos-blocked-destinations.h View 1 chunk +48 lines, -0 lines 0 comments Download
A src/devices/wifi/qos-blocked-destinations.cc View 1 chunk +66 lines, -0 lines 0 comments Download
M src/devices/wifi/qos-utils.h View 1 1 chunk +7 lines, -0 lines 0 comments Download
M src/devices/wifi/qos-utils.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +13 lines, -0 lines 0 comments Download
M src/devices/wifi/qsta-wifi-mac.h View 3 chunks +6 lines, -0 lines 0 comments Download
M src/devices/wifi/qsta-wifi-mac.cc View 1 2 3 4 5 6 7 8 9 10 5 chunks +117 lines, -0 lines 0 comments Download
M src/devices/wifi/wifi-mac.h View 1 2 3 4 5 6 7 8 9 2 chunks +11 lines, -0 lines 0 comments Download
M src/devices/wifi/wifi-mac.cc View 1 2 chunks +67 lines, -0 lines 0 comments Download
M src/devices/wifi/wifi-mac-header.h View 3 chunks +7 lines, -0 lines 0 comments Download
M src/devices/wifi/wifi-mac-header.cc View 1 2 3 4 5 6 7 8 9 6 chunks +41 lines, -6 lines 0 comments Download
M src/devices/wifi/wifi-mac-queue.h View 1 2 3 4 5 6 7 8 3 chunks +25 lines, -2 lines 0 comments Download
M src/devices/wifi/wifi-mac-queue.cc View 1 2 3 4 5 6 7 8 3 chunks +85 lines, -6 lines 0 comments Download
M src/devices/wifi/wscript View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 31
faker.moatamri
Reviews http://codereview.appspot.com/144050/diff/1/2 File examples/wireless/wifi-blockack.cc (right): http://codereview.appspot.com/144050/diff/1/2#newcode21 Line 21: //This is a simple example in order ...
11 years, 2 months ago (2009-11-03 14:10:49 UTC) #1
Andrey Mazo
Valgrind seems to be unhappy with this code and shows memory leaks: ==30779== LEAK SUMMARY: ...
11 years, 2 months ago (2009-11-03 23:19:58 UTC) #2
Andrey Mazo
I've tried to set some more Ptr's to zero in several DoDispose ()'s, but it ...
11 years, 2 months ago (2009-11-04 00:14:03 UTC) #3
Andrey Mazo
On 2009/11/03 23:19:58, Andrey Mazo wrote: > Valgrind seems to be unhappy with this code ...
11 years, 2 months ago (2009-11-04 11:10:15 UTC) #4
Andrey Mazo
On 2009/11/04 13:39, Mirko Banchi wrote: >On 2009/11/04 00:14:03, Andrey Mazo wrote: >> I've tried ...
11 years, 2 months ago (2009-11-04 11:16:29 UTC) #5
Andrey Mazo
On 2009/11/04 13:35, Mirko Banchi wrote: >On 2009/11/03 23:19:58, Andrey Mazo wrote: >> http://codereview.appspot.com/144050/diff/1/7#newcode277 >> ...
11 years, 2 months ago (2009-11-04 11:40:57 UTC) #6
Nicola Baldo
In general the patch seems good to me, though I have to say that due ...
11 years, 2 months ago (2009-11-04 17:27:41 UTC) #7
Andrey Mazo
On 2009/11/04 11:10:15, Andrey Mazo wrote: > On 2009/11/03 23:19:58, Andrey Mazo wrote: > > ...
11 years, 2 months ago (2009-11-04 22:04:46 UTC) #8
and.kirill
The code looks very nice, and a really huge job is done, but the main ...
11 years, 2 months ago (2009-11-10 17:15:26 UTC) #9
Nicola Baldo
Good work. The comments below refer to a number of minor issues found here and ...
11 years, 2 months ago (2009-11-13 12:58:49 UTC) #10
Mathieu Lacage
Hi, First, I apologize for taking so long to properly review this code. With that ...
11 years, 2 months ago (2009-11-17 11:59:13 UTC) #11
Mathieu Lacage
I like the direction of the changes. I will wait until you have split the ...
11 years, 2 months ago (2009-11-23 15:01:40 UTC) #12
Mirko Banchi
On 2009/11/23 15:01:40, Mathieu Lacage wrote: > I like the direction of the changes. I ...
11 years, 2 months ago (2009-11-24 11:24:07 UTC) #13
Mathieu Lacage
On Tue, Nov 24, 2009 at 12:24 PM, <mk.banchi@gmail.com> wrote: > Reviewers: faker.moatamri, Andrey Mazo, ...
11 years, 2 months ago (2009-11-24 11:33:00 UTC) #14
Mirko Banchi
>Yes, I have noticed this but it's really just a matter to add this >extra ...
11 years, 2 months ago (2009-11-24 12:43:09 UTC) #15
Mirko Banchi
Hi Mathieu and all, a fourth review patch set is available. What is new: -OriginatorBlockAckAgreement ...
11 years, 2 months ago (2009-11-24 18:06:44 UTC) #16
Mathieu Lacage
Ok, a couple of small details, as well as some high-level questions. http://codereview.appspot.com/144050/diff/10041/11008 File src/devices/wifi/block-ack-agreement.h ...
11 years, 2 months ago (2009-11-25 09:17:29 UTC) #17
Mirko Banchi
On 2009/11/25 09:17:29, Mathieu Lacage wrote: > Ok, a couple of small details, as well ...
11 years, 2 months ago (2009-11-25 11:51:22 UTC) #18
Mathieu Lacage
On Wed, Nov 25, 2009 at 12:51 PM, <mk.banchi@gmail.com> wrote: > On 2009/11/25 09:17:29, Mathieu ...
11 years, 2 months ago (2009-11-25 15:01:27 UTC) #19
Mirko Banchi
On 2009/11/25 15:01:27, Mathieu Lacage wrote: > On Wed, Nov 25, 2009 at 12:51 PM, ...
11 years, 2 months ago (2009-11-26 15:29:35 UTC) #20
Mirko Banchi
Hi Mathieu, i have aligned my block ack implementation with ns-3-dev. Now implementation for action ...
11 years, 1 month ago (2009-12-02 18:06:53 UTC) #21
Mathieu Lacage
Mirko, Sorry for this long delay in getting back to you. The overall patch looks ...
11 years, 1 month ago (2009-12-14 12:58:40 UTC) #22
Mirko Banchi
On 2009/12/14 12:58:40, Mathieu Lacage wrote: > Mirko, > > Sorry for this long delay ...
11 years, 1 month ago (2009-12-14 14:15:01 UTC) #23
Mathieu Lacage
On Mon, Dec 14, 2009 at 3:15 PM, <mk.banchi@gmail.com> wrote: > Kirill have already moved ...
11 years, 1 month ago (2009-12-14 15:09:07 UTC) #24
Tom Henderson
Some comments from a superficial review of this patchset are included. Otherwise, the main comment ...
10 years, 12 months ago (2010-01-30 23:21:14 UTC) #25
Mathieu Lacage
On Sun, Jan 31, 2010 at 12:21 AM, <tomh.org@gmail.com> wrote: > http://codereview.appspot.com/144050/diff/18001/18018#newcode20 > src/devices/wifi/mgt-headers.h:20: * ...
10 years, 12 months ago (2010-02-02 12:40:19 UTC) #26
Tom Henderson
On 2010/02/02 12:40:19, Mathieu Lacage wrote: > On Sun, Jan 31, 2010 at 12:21 AM, ...
10 years, 12 months ago (2010-02-02 16:55:26 UTC) #27
Tom Henderson
> However, you are right; I checked that our code-submissions.html page says to do > ...
10 years, 12 months ago (2010-02-02 16:57:08 UTC) #28
Mathieu Lacage
On Tue, Feb 2, 2010 at 5:55 PM, <tomh.org@gmail.com> wrote: > We had a discussion ...
10 years, 11 months ago (2010-02-04 17:04:52 UTC) #29
tomh_tomh.org
On Thu, 4 Feb 2010 18:04:46 +0100, Mathieu Lacage > > > I don't know ...
10 years, 11 months ago (2010-02-04 17:20:03 UTC) #30
Mirko Banchi
10 years, 11 months ago (2010-02-06 17:50:30 UTC) #31
On 2010/02/04 17:20:03, tomh_tomh.org wrote:
> 
> It just caught my eye because it was within a couple of lines of each
> other.  Again, not a big issue for me.
> 
> I think the bigger cleanup issue with the merge is that there is >missing
> doxygen and lots of MY_DEBUG statements still sprinkled in the
> code, which
> I would like to see fixed.  
> 

I think that use of MY_DEBUG macro is very helpful to figure out what happens in
a simulation because it prints also mac addresses. Without its use, when the
number of stations in a simulation increases, maybe the output could become
unreadable. However yes, it's possible to remove MY_DEBUG macro and use directly
NS_LOG_DEBUG (m_low->GetAddress ()<<" "<<...) instead.
Sign in to reply to this message.

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