Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gnrc_uhcpc: initialize rpl root on prefix configuration #8173

Merged
merged 2 commits into from
Nov 30, 2017

Conversation

bergzand
Copy link
Member

This PR adds functionality to gnrc_uhcpc to initialize a border router as RPL dodag root when the RPL module is included. When a new prefix is retrieved over uhcpc the old instance is deleted and a new dodag root with the new prefix is created.

Testing: GNRC_NETIF_IPV6_ADDRS_NUMOF was set to 3 as a work around for #8172.

@bergzand bergzand added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation GNRC Area: network Area: Networking labels Nov 29, 2017
@jnohlgard
Copy link
Member

Nice!

if (inst) {
gnrc_rpl_instance_remove(inst);
}
gnrc_rpl_root_init(gnrc_wireless_interface, (ipv6_addr_t*)prefix, false, false);
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I don't like the idea of using gnrc_wireless_interface as the instance id. Instance IDs can be global or local, depening on the range (global for 0 <= x <= 127). In the unlikely event, that gnrc_wireless_interface is a very high number, this would lead to semantic side effects of initializing a local instance instead of a global one. I guess a global instance makes the most sense here.

I'd propose:

gnrc_rpl_root_init(0, (ipv6_addr_t*)prefix, true, false);

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, I misinterpreted some arguments there. @cgundogan How about reusing GNRC_RPL_DEFAULT_INSTANCE here and on line 96?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, using GNRC_RPL_DEFAULT_INSTANCE is probably also nice (: 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@cgundogan cgundogan self-assigned this Nov 30, 2017
@cgundogan
Copy link
Member

@bergzand could you also add #USEMODULE += gnrc_rpl to the Makefile of the border router example? And maybe a little explanation in front of that that it is going to start a RPL dodag once a prefix was initialized?

@bergzand
Copy link
Member Author

I'd prefer to do that in a separate PR if you don't mind. As it influences the build size of the example, it could make this PR unnecessary complicated if somebody disagrees or it pushes the build size beyond the capabilities of one of the boards.

@cgundogan
Copy link
Member

@bergzand you missed that # in front of my proposal (:

@bergzand
Copy link
Member Author

@cgundogan Ah 😑

Added the suggestion and a short explanation to the makefile.

@cgundogan
Copy link
Member

@bergzand that's magnificent! Please squash.

@bergzand
Copy link
Member Author

squashed

Copy link
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

ACK

@cgundogan cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 30, 2017
@cgundogan cgundogan merged commit 06782c4 into RIOT-OS:master Nov 30, 2017
@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
@bergzand bergzand deleted the pr/gnrc_uhcpc_rpl branch April 16, 2018 16:21
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 Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants