shell: add commands to manage neighbor cache manually by miri64 · Pull Request #2583 · RIOT-OS/RIOT

miri64 · 2015-03-12T00:51:14Z

~~Depends on #2461 and all its dependencies.~~ (merged)

~~Waiting for #2602~~ (merged)

~~Depends on #2705~~ (merged)

miri64 · 2015-03-14T01:32:09Z

Rebased to current master

miri64 · 2015-03-19T11:12:16Z

Can I haz review?

Lotterleben · 2015-03-24T08:47:08Z

sys/shell/commands/sc_ipv6_nc.c

Doesn't this only work for add? For del, argv[2] would be an ipv6 address... or am I missing something?
If not, I'd propose to just move this line below line 115.

Done: Moved it.

Lotterleben · 2015-03-24T09:21:19Z

Done for now :)

miri64 · 2015-03-24T15:32:56Z

Rebased to #2705 and addressed comments

miri64 · 2015-03-25T19:32:29Z

~~Merge stalled before #2602 is merged.~~ (merged)

miri64 · 2015-03-29T22:12:35Z

Rebased to current #2705 and adapted for shell handler return values.

Lotterleben · 2015-03-31T13:09:45Z

Otherwise looks good to my eyes, but I don't have any strong opinons here, style-wise. @OlegHahm @haukepetersen, any complaints?

miri64 · 2015-03-31T16:17:37Z

Please comment on changes in the PR not in the commits ;-)

I actually would like to add support for #2721, but that I can do later, too.

miri64 · 2015-03-31T16:23:00Z

Fixed.

Lotterleben · 2015-03-31T18:10:41Z

Thanks. Personally, I'd like to not let this PR linger any longer than it has to...

miri64 · 2015-03-31T19:03:46Z

Squash then?

Lotterleben · 2015-03-31T19:05:25Z

Yup :) (I think I need to work on the ambiguousness of my comments :D)

miri64 · 2015-03-31T19:13:51Z

Done, but #2705 needs to get merged before we can merge this one ;-) (I took the liberty to assign you this PR :-))

Lotterleben · 2015-03-31T19:16:48Z

Oooh boy :D I have a long train ride ahead of me tomorrow, will go through it then, okay?

miri64 · 2015-03-31T19:18:50Z

Okay :-)

miri64 · 2015-04-01T23:14:24Z

Rebased to current master, adapt to API change in #2705 and squashed

Lotterleben · 2015-04-01T23:27:45Z

Thank you :) ACK if travis is happy.

shell: add commands to manage neighbor cache manually

OlegHahm · 2015-04-02T14:16:30Z

Sorry to hop in that late: do we really want to provide these commands for every build? They seem to be rather a debugging/development tool, so I would expect them to only get linked with DEVELHELP set or similar.

miri64 · 2015-04-02T14:19:03Z

Isn't that the main purpose for the shell anyways?

OlegHahm · 2015-04-02T14:27:19Z

Is there a English equivalent to German "jein" (it's obviously not "yo", so maybe "nes"?). Anyway, I think you're right that the shell is usually not meant for real, productive deployment, but I would still distinguish between commands that are helpful for normal operation (ifconfig, chan, route...) and helper tools that interfere with normal (network) behavior. The commands from this PR are probably not the only debatable candidates (txtsnd might be another good candidate).

miri64 · 2015-04-02T14:29:09Z

(Also for some reason related to #2725 I did not manage to compile any application on native with the DEVELHELP macro so this is out of the question ;-))

miri64 · 2015-04-02T14:30:23Z

(But I see your point, but as you've said: this is not only related to this PR)

OlegHahm · 2015-04-02T14:30:57Z

Right. I'll open an issue.

OlegHahm · 2015-04-02T14:31:32Z

(Also for some reason related to #2725 I did not manage to compile any application on native with the DEVELHELP macro so this is out of the question ;-))

Sounds weird to me. Since when do we have memory issues on native?

miri64 · 2015-04-02T14:34:20Z

I don't think it's a memory issue (because without DEVELHELP everything works fine), it's just that DEBUG_ENABLE + DEVELHELP are not possible right now ;-) (but maybe I did not configure the stack sizes correctly… I was one of the people who missunderstood …STACKSIZE_PRINTF ;-))

miri64 · 2015-04-02T14:39:07Z

But regardless: putting this into DEVELHELP just adds more ambiguity to this macro… I would prefer to add a shell submodule shell_helper or something like that, that is exactly the same as shell_commands in infrastructure, but only contains this little commands that are more helpers than real functionality.

OlegHahm · 2015-04-02T14:47:54Z

see #2755

miri64 added Area: network Area: Networking State: waiting for other PR State: The PR requires another PR to be merged first NSTF labels Mar 12, 2015

miri64 added this to the Network Stack Task Force milestone Mar 12, 2015

miri64 assigned haukepetersen Mar 12, 2015

miri64 force-pushed the shell/feat/ipv6_nc branch 2 times, most recently from 5e1e069 to 8699c56 Compare March 14, 2015 01:31

miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 14, 2015

miri64 assigned Lotterleben and unassigned haukepetersen Mar 23, 2015

Lotterleben reviewed Mar 24, 2015
View reviewed changes

miri64 force-pushed the shell/feat/ipv6_nc branch from ea1139f to a697517 Compare March 24, 2015 15:32

miri64 mentioned this pull request Mar 25, 2015

Return-based error-handling for shell handlers #2602

Merged

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 25, 2015

miri64 force-pushed the shell/feat/ipv6_nc branch from a697517 to 7ea1fb7 Compare March 29, 2015 22:12

Lotterleben force-pushed the master branch from 1d9f189 to d4e068b Compare March 31, 2015 09:52

OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01

miri64 force-pushed the shell/feat/ipv6_nc branch from ff84d58 to 63a46c8 Compare March 31, 2015 19:13

shell: add commands to manage neighbor cache manually

bc072db

miri64 force-pushed the shell/feat/ipv6_nc branch from 63a46c8 to bc072db Compare April 1, 2015 23:13

miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 1, 2015

miri64 added a commit that referenced this pull request Apr 2, 2015

Merge pull request #2583 from authmillenon/shell/feat/ipv6_nc

56f5a83

shell: add commands to manage neighbor cache manually

miri64 merged commit 56f5a83 into RIOT-OS:master Apr 2, 2015

miri64 deleted the shell/feat/ipv6_nc branch April 2, 2015 10:50

OlegHahm mentioned this pull request Apr 2, 2015

Define helper shell commands #2755

Closed

miri64 mentioned this pull request Apr 2, 2015

ipv6_nc: add iterators #2721

Merged

Conversation

miri64 commented Mar 12, 2015

Uh oh!

miri64 commented Mar 14, 2015

Uh oh!

miri64 commented Mar 19, 2015

Uh oh!

Lotterleben Mar 24, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 Mar 24, 2015

Choose a reason for hiding this comment

Uh oh!

Lotterleben commented Mar 24, 2015

Uh oh!

miri64 commented Mar 24, 2015

Uh oh!

miri64 commented Mar 25, 2015

Uh oh!

miri64 commented Mar 29, 2015

Uh oh!

Lotterleben commented Mar 31, 2015

Uh oh!

miri64 commented Mar 31, 2015

Uh oh!

miri64 commented Mar 31, 2015

Uh oh!

Lotterleben commented Mar 31, 2015

Uh oh!

miri64 commented Mar 31, 2015

Uh oh!

Lotterleben commented Mar 31, 2015

Uh oh!

miri64 commented Mar 31, 2015

Uh oh!

Lotterleben commented Mar 31, 2015

Uh oh!

miri64 commented Mar 31, 2015

Uh oh!

miri64 commented Apr 1, 2015

Uh oh!

Lotterleben commented Apr 1, 2015

Uh oh!

OlegHahm commented Apr 2, 2015

Uh oh!

miri64 commented Apr 2, 2015

Uh oh!

OlegHahm commented Apr 2, 2015

Uh oh!

miri64 commented Apr 2, 2015

Uh oh!

miri64 commented Apr 2, 2015

Uh oh!

OlegHahm commented Apr 2, 2015

Uh oh!

OlegHahm commented Apr 2, 2015

Uh oh!

miri64 commented Apr 2, 2015

Uh oh!

miri64 commented Apr 2, 2015

Uh oh!

OlegHahm commented Apr 2, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants