Skip to content

gnrc_netif_hdr: provide convenience function to get netif pointer from netif header.#10500

Merged
haukepetersen merged 5 commits intoRIOT-OS:masterfrom
miri64:gnrc_netif_hdr/enh/conv-netif-getter
Dec 5, 2018
Merged

gnrc_netif_hdr: provide convenience function to get netif pointer from netif header.#10500
haukepetersen merged 5 commits intoRIOT-OS:masterfrom
miri64:gnrc_netif_hdr/enh/conv-netif-getter

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Nov 28, 2018 •
edited
Loading

Contribution description

Does what it says in the title + uses it where appropriate.

Testing procedure

Ping6 with gnrc_networking should still work with native and an IEEE 802.15.4 based board. ICMPv6 error messages should still be sent as outlined in task 10 of the release specs.

Issues/PRs references

Depends on #10505 (merged)

gnrc_netif/gnrc_sixlowpan_iphc BLE capability

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking labels Nov 28, 2018
@miri64 miri64 requested a review from haukepetersen November 28, 2018 16:52
haukepetersen
haukepetersen previously approved these changes Nov 28, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

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

works as expected and change looks good -> ACK

@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 Nov 28, 2018
@miri64 miri64 force-pushed the gnrc_netif_hdr/enh/conv-netif-getter branch from 3f911f3 to 779c049 Compare November 28, 2018 17:16
@miri64 miri64 dismissed haukepetersen’s stale review November 28, 2018 17:17

I removed a now unused variable pointed out by murdock and squashed directly. Can you have another look, @haukepetersen?

haukepetersen
haukepetersen previously approved these changes Nov 28, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

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

re-ACK

@miri64 miri64 mentioned this pull request Nov 29, 2018
Merged
@miri64 miri64 force-pushed the gnrc_netif_hdr/enh/conv-netif-getter branch from 779c049 to 7fbcabb Compare November 29, 2018 09:41
@miri64 miri64 added State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 29, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Needed to add #10505 as a dependency.

@smlng smlng removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 29, 2018
@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 Nov 29, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Setting the CI: ready for build label here rather than in #10505 to see if other problems occur with the header inclusion here.

@miri64 miri64 force-pushed the gnrc_netif_hdr/enh/conv-netif-getter branch from 7fbcabb to 78b77a4 Compare November 29, 2018 11:41
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Rebased to current #10505.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

For the record: the ACK'd state was 779c049

@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 29, 2018
@miri64 miri64 force-pushed the gnrc_netif_hdr/enh/conv-netif-getter branch from 78b77a4 to fb2ca7f Compare November 29, 2018 21:06
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Rebased to current master as #10505 was merged.

@miri64 miri64 force-pushed the gnrc_netif_hdr/enh/conv-netif-getter branch from fb2ca7f to 779c049 Compare November 29, 2018 21:07
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

(just trying something ... reversed to ACK'd state)

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 29, 2018
@miri64
Merge branch 'gnrc_netif_hdr/enh/conv-netif-getter' (RIOT-OS#10500) i…
2fcdbda 
…nto gnrc_sixlowpan_iphc/fix/non-ieee802154-support
@miri64 miri64 mentioned this pull request Nov 29, 2018
Merged
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 30, 2018

As expected, Murdock passes the state @haukepetersen ACK'd now. Can someone confirm, that this is indeed the case now? @kaspar030 @bergzand?

@miri64 miri64 dismissed haukepetersen’s stale review November 30, 2018 08:57

Actually, I found another improvement.. So this needs to be re-ACKed :-/

miri64 added a commit to miri64/RIOT that referenced this pull request Nov 30, 2018
@miri64
Merge branch 'gnrc_netif_hdr/enh/conv-netif-getter' (RIOT-OS#10500) …
2b624fd 
…into HEAD
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Nov 30, 2018

May I ask with which tool you created the dependency diagram?

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 30, 2018

GraphViz dot. The same Tool Doxygen used for its dependency diagrams ;) (though I made it a bit more colorful :))

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 30, 2018

(and of course you may ☺️)

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 30, 2018

@kaspar030 you can find the source of the graph above here. The description language is graph driven and pretty easy to understand.

haukepetersen
haukepetersen approved these changes Dec 5, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

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

re-ACK

@miri64 miri64 force-pushed the gnrc_netif_hdr/enh/conv-netif-getter branch from 772ab9b to d94ba6c Compare December 5, 2018 13:08
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 5, 2018

Squashed

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Dec 5, 2018

and go

@haukepetersen haukepetersen merged commit 6551d8a into RIOT-OS:master Dec 5, 2018
@miri64 miri64 deleted the gnrc_netif_hdr/enh/conv-netif-getter branch December 5, 2018 13:30
@aabadie aabadie added this to the Release 2019.01 milestone Dec 19, 2018
@miri64 miri64 mentioned this pull request Dec 27, 2018
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haukepetersen haukepetersen haukepetersen approved these changes

Assignees

No one assigned

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

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

5 participants

@miri64 @kaspar030 @haukepetersen @aabadie @smlng