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

netdev2: rename to netdev and remove gnrc_netdev #6610

Merged
merged 1 commit into from Mar 15, 2017

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 15, 2017

With some minor hand-edits I used the following chain of commands:

git rm sys/include/net/gnrc/netdev.h
git grep --name-only -i netdev2 | \
        xargs sed -i -e 's/^\(NETDEV\)2\(.*\)\( [("]\)/\1\2 \3/g' \
                     -e 's/\(netdev\)2\(.*\)\( \/\*\*<\)/\1\2 \3/I' \
                     -e 's/\(netdev\)2/\1/gI'
git add -p
git commit --amend
git ls-tree --full-tree -r HEAD --name-only | \
        grep "netdev2" | xargs -I'{}' dirname '{}' | uniq | \
        grep "netdev2" | while read dir; do
                new_dir="$(echo "$dir" | sed "s/netdev2/netdev/g")"
                git mv -f "$dir" "$new_dir"
        done
git commit --amend
git ls-tree --full-tree -r HEAD --name-only | \
        grep "netdev2" | while read file; do
                new_file="$(echo "$file" | sed "s/netdev2/netdev/g")"
                git mv -f "$file" "$new_file"
        done
git commit --amend
git grep --name-only "\<drivers_netdev_netdev\>" | \
        xargs sed -i "s/\<drivers_netdev_netdev\>/drivers_netdev_api/g"
git add -p
git commit --amend

@miri64 miri64 added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers Area: network Area: Networking labels Feb 15, 2017
@miri64 miri64 added this to the Release 2017.04 milestone Feb 15, 2017
@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 Feb 15, 2017
@miri64
Copy link
Member Author

miri64 commented Feb 15, 2017

found and fixed minor issue (inclusion of (original) net/gnrc/netdev.h led to problems ;-))

@miri64 miri64 force-pushed the netdev2/enh/rename-to-netdev branch from efca06f to 7062100 Compare February 16, 2017 13:14
@miri64
Copy link
Member Author

miri64 commented Feb 16, 2017

Rebased (fun!)

@miri64
Copy link
Member Author

miri64 commented Feb 27, 2017

Rebased.

With some minor hand-edits I used the following chain of commands:

```sh
git rm sys/include/net/gnrc/netdev.h
git grep --name-only -i netdev2 | \
        xargs sed -i -e 's/^\(NETDEV\)2\(.*\)\( [("]\)/\1\2 \3/g' \
                     -e 's/\(netdev\)2\(.*\)\( \/\*\*<\)/\1\2 \3/I' \
                     -e 's/\(netdev\)2/\1/gI'
git add -p
git commit --amend
git ls-tree --full-tree -r HEAD --name-only | \
        grep "netdev2" | xargs -I'{}' dirname '{}' | uniq | \
        grep "netdev2" | while read dir; do
                new_dir="$(echo "$dir" | sed "s/netdev2/netdev/g")"
                git mv -f "$dir" "$new_dir"
        done
git commit --amend
git ls-tree --full-tree -r HEAD --name-only | \
        grep "netdev2" | while read file; do
                new_file="$(echo "$file" | sed "s/netdev2/netdev/g")"
                git mv -f "$file" "$new_file"
        done
git commit --amend
git grep --name-only "\<drivers_netdev_netdev\>" | \
        xargs sed -i "s/\<drivers_netdev_netdev\>/drivers_netdev_api/g"
git add -p
git commit --amend
```
@miri64 miri64 force-pushed the netdev2/enh/rename-to-netdev branch from d1c130c to 29842bb Compare March 15, 2017 08:33
@miri64
Copy link
Member Author

miri64 commented Mar 15, 2017

Rebased. I would like to avoid another ;-)

@kaspar030 kaspar030 added 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 Mar 15, 2017
@kaspar030
Copy link
Contributor

Well, it compiles... I'd say, merge once Murdock1 agrees?

@miri64 how hard would it be to seperate removal of gnrc_netdev, automatic changes and the changes made by hand into seperate commits?

@emmanuelsearch
Copy link
Member

By the way: would this be a good candidate to test Coccinelle use?

@emmanuelsearch
Copy link
Member

+1 for merging after Murdock agrees

@kaspar030
Copy link
Contributor

By the way: would this be a good candidate to test Coccinelle use?

Maybe. But the work is done I guess. ;)

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@emmanuelsearch
Copy link
Member

Yes, the work is done, but it could still be a good benchmark for the usefulness of the tool in RIOT?

@kaspar030
Copy link
Contributor

Yes, the work is done, but it could still be a good benchmark for the usefulness of the tool in RIOT?

Try and let us know!

@kaspar030
Copy link
Contributor

kaspar030 commented Mar 15, 2017

Do we need a second ACK for this? I admit I did not look at all those 1700 changed lines.

@miri64
Copy link
Member Author

miri64 commented Mar 15, 2017

@miri64 how hard would it be to seperate removal of gnrc_netdev, automatic changes and the changes made by hand into seperate commits?

it would be basically doing everything all over again, since I do not remember which change I did how :-/

By the way: would this be a good candidate to test Coccinelle use?

TBH, I totally forgot about this tool again :( Thanks for the reminder. But since there were a few handcrafted changes that might have been able automatized with a more complex tool than sed (e.g. removing #include "net/gnrc/netdev.h" completely, since net/gnrc/netdev2.h now became that and is completely different), I think it would have been the better choice.

@miri64
Copy link
Member Author

miri64 commented Mar 15, 2017

Do we need a second ACK for this? I admit I did not look at all those 1700 changed lines.

I would feel better if someone else than me would at least test, if gnrc_networking is still working ;-).

@kaspar030
Copy link
Contributor

I would feel better if someone else than me would at least test, if gnrc_networking is still working ;-).

Confirmed gnrc_networking is still working (tried ping). Let's go!

@kaspar030 kaspar030 merged commit 119fc70 into RIOT-OS:master Mar 15, 2017
@kaspar030
Copy link
Contributor

@miri64 Would you mind letting devel@ know about the change?

@miri64 miri64 deleted the netdev2/enh/rename-to-netdev branch March 15, 2017 10:40
@miri64
Copy link
Member Author

miri64 commented Mar 15, 2017

Not at all, will write a mail.

@emmanuelsearch
Copy link
Member

@miri64 great to finally have straightened this part of the API out!

@miri64 miri64 mentioned this pull request Mar 16, 2017
@drcref
Copy link

drcref commented Mar 17, 2017

Networking is partly broken in my 2 samr21-xpro boards since this PR was merged.

When both are running the gnrc_networking example, they can ping each other only on their link local addresses. Even if they have global addresses assigned (or autoconfigured) they can't ping each other using them. This rename might have introduced issues to NDP which breaks the neighbor cache of the nodes.

In another scenario, one xpro is acting as a border router (based on the gnrc_border_router example, modified to use the enc28j60 ethernet adapter instead of ethos for uplink) and the other as a node with gnrc_networking. Networking with global addresses is now broken there too. The border router can't talk to the upstream network router or the 802.15.4 nodes except using link-local addresses.

@miri64
Copy link
Member Author

miri64 commented Mar 17, 2017

I can't confirm the border router case (samr21-xpro with ethos ethernet). For the first case: Did you set the forwarding in the FIB as well? Auto-config won't cut that (that was the case before this PR was merged too).

@drcref
Copy link

drcref commented Mar 17, 2017

Yes I did. I've been testing the border router case, now it works again after I reverted just before this PR.

@miri64
Copy link
Member Author

miri64 commented Mar 17, 2017

As I said: I can't confirm (I don't have the enc28j60 chip though :-/). Both scenarios work in the default set-up (i.e. with ethos border router). Tested scenario 1 on both samr21-xpro and native.

@drcref
Copy link

drcref commented Mar 17, 2017

I will investigate it a bit further when I have some time, it might be an issue with enc28j60 as well. Thanks for checking on your end!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers 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: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants