Skip to content

ng_net: add global configuration options#2399

Merged
haukepetersen merged 1 commit intoRIOT-OS:masterfrom
haukepetersen:ng_netconf
Feb 10, 2015
Merged

ng_net: add global configuration options#2399
haukepetersen merged 1 commit intoRIOT-OS:masterfrom
haukepetersen:ng_netconf

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

this file keeps a global list of options that are available for the get/set methods in netdev and netapi. Network modules and device drivers can choose which of these options they actually support.

This list is also not complete, I expect a number of options to be added over time...

@haukepetersen haukepetersen added NSTF Area: network Area: Networking labels Feb 6, 2015
@OlegHahm OlegHahm added this to the Network Stack Task Force milestone Feb 6, 2015
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.

Can't we make this a bit more specific? s/i.e./e.g./

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure.

@OlegHahm OlegHahm added NSTF and removed NSTF labels Feb 6, 2015
@haukepetersen haukepetersen added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Feb 6, 2015
@haukepetersen
Copy link
Copy Markdown
Contributor Author

addressed @OlegHahm's comments

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.

Does it make sense to put the word netdev somewhere here in between? The doc states it's for network devices, so I figured putting this into the name would make this more obvious.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure, it's not dependent on netdev, so I would leave it out.

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.

I'm with @haukepetersen, in theory you can apply all this states to a protocol's state machine, too.

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.

theoretically speaking we should then adjust the documentation to be something else than

the state of a network device

For me, this sounds very restricting to a network device..

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.

👍

@LudwigKnuepfer LudwigKnuepfer changed the title ng_net: added global configuration options ng_net: add global configuration options Feb 7, 2015
@miri64 miri64 self-assigned this Feb 8, 2015
@miri64 miri64 mentioned this pull request Feb 8, 2015
36 tasks
@haukepetersen
Copy link
Copy Markdown
Contributor Author

adjusted documentation for states

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.

As stated before: I think the word "module" is misleading here (Think of multi-modular (in the RIOT sense) protocol implementations as IPv6 or even 6LoWPAN (fragmentation and IPHC)). I would just use "state of a network device or protocol implementation".

@haukepetersen
Copy link
Copy Markdown
Contributor Author

addressed documentation text for the state enum

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 9, 2015

ACK, please squash.

@cgundogan
Copy link
Copy Markdown
Member

ACK, clear and comprehensible. I guess further PRs regarding the new network stack have this PR as a dependency. I don't see any more show-stoppers here. IMHO when travis builds => go

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.

NG_NET_CONF_H_H ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

why H_H? Too my knowledge we don't have a rule for naming these in RIOT?! And as I used to put the underscore in front (which was in violation of the C standard as I learned), I put one after the name now. Don't think that hurts and it helps to prevent naming clashes

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.

Sorry, I meant NG_NET_CONF_H_ - the trailing H was just a brainfart, the leading NG_ was the actual content of the comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry, I overlooked that one. will fix.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

rebased and squashed.

@haukepetersen haukepetersen removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Feb 10, 2015
@haukepetersen
Copy link
Copy Markdown
Contributor Author

fixed header macro name as pointed out by @LudwigOrtmann and squashed - will merge when Travis is happy.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

Travis is happy -> go.

haukepetersen added a commit that referenced this pull request Feb 10, 2015
ng_net: add global configuration options
@haukepetersen haukepetersen merged commit d46d654 into RIOT-OS:master Feb 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants