Skip to content

sixlowpan: minor fixes for non-ipv6 usage#10379

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
cgundogan:pr/sixlowpan_nonipv6
Dec 18, 2018
Merged

sixlowpan: minor fixes for non-ipv6 usage#10379
miri64 merged 1 commit intoRIOT-OS:masterfrom
cgundogan:pr/sixlowpan_nonipv6

Conversation

@cgundogan
Copy link
Copy Markdown
Member

Contribution description

When using the sixlowpan module without IPv6, then
the compiler stumbles across some dependencies that are not met.

Testing procedure

remove gnrc_ipv6 from the Makefile.dep for gnrc_sixlowpan should suffice. Alternatively, I completely removed the gnrc_sixlowpan dependency entry from Makefile.dep and added gnrc_sixlowpan_frag to the ccn-lite-relay example for testing purposes.

Issues/PRs references

@cgundogan cgundogan added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Nov 13, 2018
@cgundogan cgundogan requested a review from miri64 November 13, 2018 08:51
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few suggestions.

(void)context;
(void)page;
#ifdef MODULE_CCNLITE
#ifdef MODULE_CCN_LITE
Copy link
Copy Markdown
Member

@miri64 miri64 Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in here doesn't look CCN-lite related. How about

Suggested change
#ifdef MODULE_CCN_LITE
#ifndef MODULE_GNRC_IPV6

(or using #ifdef MODULE_GNRC_IPV6 and turn the branches around.)

pkt = gnrc_pktbuf_remove_snip(pkt, sixlowpan);
#ifdef MODULE_CCN_LITE
payload->type = GNRC_NETTYPE_CCN;
#else /* MODULE_CCN_LITE */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#else /* MODULE_CCN_LITE */
#elif defined(MODULE_GNRC_IPV6)

(and then UNDEF for the else branch)

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 13, 2018

(Compiling works)

@smlng
Copy link
Copy Markdown
Member

smlng commented Nov 30, 2018

Hiho, this looks easy to be moved forward and merged, right?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 30, 2018

Yepp, just like to have my comments at least discussed shortly :-)

Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 18, 2018
@cgundogan cgundogan force-pushed the pr/sixlowpan_nonipv6 branch from 9c74951 to 79de5ae Compare December 18, 2018 20:33
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on iotlab-m3. I'm still able to ping between two nodes using gnrc_networking. Since ccnl-lowpan isn't integrated yet, I don't know how to test, but I trust that you did.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 18, 2018

And go

@miri64 miri64 merged commit 3ce4b74 into RIOT-OS:master Dec 18, 2018
@cgundogan cgundogan deleted the pr/sixlowpan_nonipv6 branch December 19, 2018 09:14
@aabadie aabadie added this to the Release 2019.01 milestone Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants