ns-3 - LrWpanMac - Extended Addressing

This small patch provides extended addressing to LrWpanMac. Now McpsDataRequest can create frames with extended (64-bit) addresses.

[JRewienski, 2017-11-09 12:02:20]

Hello everyone,

This small patch provides extended addressing to LrWpanMac. Now McpsDataRequest
can create frames with extended (64-bit) addresses.

Any suggestions / reviews would be much appreciated.

PS. Sorry for posting it 3 times!

[Tommaso Pecorella, 2017-11-12 14:45:48]

On 2017/11/09 12:02:20, JRewienski wrote:
> Hello everyone,
> 
> This small patch provides extended addressing to LrWpanMac. Now
McpsDataRequest
> can create frames with extended (64-bit) addresses.
> 
> Any suggestions / reviews would be much appreciated.
> 
> PS. Sorry for posting it 3 times!

Hi,

the patch looks good.
Could you please add (or modify) the tests so we can test also the extended
addresses ?

Thanks,

T.

[JRewienski, 2017-11-12 20:27:26]

On 2017/11/12 14:45:48, Tommaso Pecorella wrote:
> On 2017/11/09 12:02:20, JRewienski wrote:
> > Hello everyone,
> > 
> > This small patch provides extended addressing to LrWpanMac. Now
> McpsDataRequest
> > can create frames with extended (64-bit) addresses.
> > 
> > Any suggestions / reviews would be much appreciated.
> > 
> > PS. Sorry for posting it 3 times!
> 
> Hi,
> 
> the patch looks good.
> Could you please add (or modify) the tests so we can test also the extended
> addresses ?
> 
> Thanks,
> 
> T.

Hi,

Thanks for your reply,

I've modified the ACK test to also test the extended addressing. I hope it's ok,
as this is just a small new feature.

JR

[Tommaso Pecorella, 2017-11-14 23:35:52]

I added one small comment only.

Other than that it's a +1 (i.e., fix the comment and merge).

https://codereview.appspot.com/338800043/diff/20001/src/lr-wpan/model/lr-wpan...
File src/lr-wpan/model/lr-wpan-mac.cc (right):

https://codereview.appspot.com/338800043/diff/20001/src/lr-wpan/model/lr-wpan...
src/lr-wpan/model/lr-wpan-mac.cc:322: macHdr.SetDstAddrMode
(params.m_dstAddrMode);
This case is explicitly forbidden by the standard.
Use a NS_ABORT_MSG instead of continuing.

I know that a few lines above there is a similar:
  switch (params.m_srcAddrMode)
    {
    case NO_PANID_ADDR:
      macHdr.SetSrcAddrMode (params.m_srcAddrMode);
      macHdr.SetNoPanIdComp ();
      break;
    case ADDR_MODE_RESERVED:
      macHdr.SetSrcAddrMode (params.m_srcAddrMode);
      break;

but this is a bug too.
I opened a bug: https://www.nsnam.org/bugzilla/show_bug.cgi?id=2822

[JRewienski, 2017-11-17 20:10:29]

On 2017/11/14 23:35:52, Tommaso Pecorella wrote:
> I added one small comment only.
> 
> Other than that it's a +1 (i.e., fix the comment and merge).
> 
>
https://codereview.appspot.com/338800043/diff/20001/src/lr-wpan/model/lr-wpan...
> File src/lr-wpan/model/lr-wpan-mac.cc (right):
> 
>
https://codereview.appspot.com/338800043/diff/20001/src/lr-wpan/model/lr-wpan...
> src/lr-wpan/model/lr-wpan-mac.cc:322: macHdr.SetDstAddrMode
> (params.m_dstAddrMode);
> This case is explicitly forbidden by the standard.
> Use a NS_ABORT_MSG instead of continuing.
> 
> I know that a few lines above there is a similar:
>   switch (params.m_srcAddrMode)
>     {
>     case NO_PANID_ADDR:
>       macHdr.SetSrcAddrMode (params.m_srcAddrMode);
>       macHdr.SetNoPanIdComp ();
>       break;
>     case ADDR_MODE_RESERVED:
>       macHdr.SetSrcAddrMode (params.m_srcAddrMode);
>       break;
> 
> but this is a bug too.
> I opened a bug: https://www.nsnam.org/bugzilla/show_bug.cgi?id=2822

Hey!

I've changed the line you have suggested. Thanks for catching the bug!

JR

[Tom Henderson, 2017-12-05 03:06:02]

Looks fine to merge once AUTHORS, RELEASE_NOTES, and CHANGES.html are also
patched.

[JRewienski, 2017-12-08 15:18:44]

On 2017/12/05 03:06:02, Tom Henderson wrote:
> Looks fine to merge once AUTHORS, RELEASE_NOTES, and CHANGES.html are also
> patched.

Hey,

I've patched AUTHORS, RELEASE_NOTES, and CHANGES.html. Hope it complies the
standard.

I'm currently out of the country, so it took me some time to set everything up.
:)

[Tommaso Pecorella, 2017-12-09 00:14:27]

I pushed this in changeset:   13202:03deb728a0df

You can close the code review, and thanks again for the feature.

T.