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

Issue 215470043: 802.11n hybrid aggregation

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by S. Deronne
Modified:
4 years, 2 months ago
Reviewers:
Tom Henderson, gbadawy
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

802.11n hybrid aggregation

Patch Set 1 #

Total comments: 16
Unified diffs Side-by-side diffs Delta from patch set Stats (+321 lines, -10 lines) Patch
A examples/wireless/simple-hybrid-aggregation.cc View 1 chunk +192 lines, -0 lines 6 comments Download
M examples/wireless/wscript View 1 chunk +4 lines, -1 line 0 comments Download
M src/wifi/model/edca-txop-n.cc View 1 chunk +12 lines, -0 lines 0 comments Download
M src/wifi/model/mac-low.h View 4 chunks +15 lines, -2 lines 6 comments Download
M src/wifi/model/mac-low.cc View 10 chunks +98 lines, -7 lines 4 comments Download

Messages

Total messages: 18
Tom Henderson
My inline comments ask about three issues: - can the public API be simplified? - ...
4 years, 3 months ago (2015-03-21 16:57:11 UTC) #1
S. Deronne
https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc File src/wifi/model/mac-low.cc (right): https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc#newcode2697 src/wifi/model/mac-low.cc:2697: { On 2015/03/21 16:57:11, Tom Henderson wrote: > inconsistent ...
4 years, 3 months ago (2015-03-28 19:23:52 UTC) #2
S. Deronne
https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.h File src/wifi/model/mac-low.h (right): https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.h#newcode1255 src/wifi/model/mac-low.h:1255: */ On 2015/03/21 16:57:11, Tom Henderson wrote: > missing ...
4 years, 3 months ago (2015-03-28 19:35:55 UTC) #3
S. Deronne
https://codereview.appspot.com/215470043/diff/1/examples/wireless/simple-hybrid-aggregation.cc File examples/wireless/simple-hybrid-aggregation.cc (right): https://codereview.appspot.com/215470043/diff/1/examples/wireless/simple-hybrid-aggregation.cc#newcode70 examples/wireless/simple-hybrid-aggregation.cc:70: Config::SetDefault ("ns3::WifiRemoteStationManager::RtsCtsThreshold", StringValue ("999999")); On 2015/03/21 16:57:11, Tom Henderson ...
4 years, 3 months ago (2015-03-28 20:08:27 UTC) #4
S. Deronne
On 2015/03/21 16:57:11, Tom Henderson wrote: > - the term 'hybrid aggregation' seems to be ...
4 years, 3 months ago (2015-03-29 13:49:58 UTC) #5
S. Deronne
> Why can't these parameters be exposed as attributes like other parameters above, > such ...
4 years, 3 months ago (2015-04-06 12:53:23 UTC) #6
gbadawy
https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc File src/wifi/model/mac-low.cc (right): https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc#newcode2872 src/wifi/model/mac-low.cc:2872: WifiMacHeader::ADDR1, hdr->GetAddr1 (), tstamp); shouldn't the while loop check ...
4 years, 3 months ago (2015-04-06 17:57:04 UTC) #7
S. Deronne
https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc File src/wifi/model/mac-low.cc (right): https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc#newcode2872 src/wifi/model/mac-low.cc:2872: WifiMacHeader::ADDR1, hdr->GetAddr1 (), tstamp); On 2015/04/06 17:57:04, gbadawy wrote: ...
4 years, 3 months ago (2015-04-06 20:16:16 UTC) #8
S. Deronne
> - can we avoid adding base class methods to MacLowTransmissionListener? It has been decided ...
4 years, 3 months ago (2015-04-08 21:00:39 UTC) #9
gbadawy
On Mon, Apr 6, 2015 at 4:16 PM, <sebastien.deronne@gmail.com> wrote: > > https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.cc > File ...
4 years, 3 months ago (2015-04-09 13:36:02 UTC) #10
S. Deronne
On 2015/04/09 13:36:02, gbadawy wrote: > On Mon, Apr 6, 2015 at 4:16 PM, <mailto:sebastien.deronne@gmail.com> ...
4 years, 3 months ago (2015-04-09 16:05:23 UTC) #11
gbadawy
On Thu, Apr 9, 2015 at 12:05 PM, <sebastien.deronne@gmail.com> wrote: > On 2015/04/09 13:36:02, gbadawy ...
4 years, 3 months ago (2015-04-09 21:01:57 UTC) #12
S. Deronne
> I know maybe I didn't explain myself clearly but in reality these MSDUs are ...
4 years, 3 months ago (2015-04-11 09:09:17 UTC) #13
S. Deronne
- can you add something about this feature in the wifi.rst? Will be updated in ...
4 years, 3 months ago (2015-04-11 20:44:26 UTC) #14
S. Deronne
On 2015/04/11 09:09:17, S. Deronne wrote: > > I know maybe I didn't explain myself ...
4 years, 2 months ago (2015-05-04 16:24:41 UTC) #15
S. Deronne
On 2015/03/21 16:57:11, Tom Henderson wrote: > My inline comments ask about three issues: > ...
4 years, 2 months ago (2015-05-04 16:26:36 UTC) #16
S. Deronne
https://codereview.appspot.com/215470043/diff/1/examples/wireless/simple-hybrid-aggregation.cc File examples/wireless/simple-hybrid-aggregation.cc (right): https://codereview.appspot.com/215470043/diff/1/examples/wireless/simple-hybrid-aggregation.cc#newcode47 examples/wireless/simple-hybrid-aggregation.cc:47: // belonging to BestEffort Access Class (AC_BE). Some comments ...
4 years, 2 months ago (2015-05-04 17:02:57 UTC) #17
S. Deronne
4 years, 2 months ago (2015-05-04 18:36:50 UTC) #18
https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.h
File src/wifi/model/mac-low.h (right):

https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.h#newc...
src/wifi/model/mac-low.h:139: virtual Ptr<MsduAggregator> GetMsduAggregator
(void);
On 2015/03/21 16:57:11, Tom Henderson wrote:
> const?

Done.

https://codereview.appspot.com/215470043/diff/1/src/wifi/model/mac-low.h#newc...
src/wifi/model/mac-low.h:142: virtual Mac48Address MapSrcAddressForAggregation
(const WifiMacHeader &hdr);
I let this as a future improvement since it is not only involving two-level
aggregation changes.
Sign in to reply to this message.

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