-
Notifications
You must be signed in to change notification settings - Fork 2k
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
ng_net: add global configuration options #2399
Conversation
NETCONF_OPT_ADDRESS, /**< get/set link layer address in host byte | ||
order */ | ||
NETCONF_OPT_ADDRESS_LONG, /**< long link layer address in host byte | ||
order (i.e. IEEE802.15.4 EUI-64) */ |
There was a problem hiding this comment.
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./
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure.
addressed @OlegHahm's comments |
NETCONF_STATE_RX, /**< device is in receive mode */ | ||
NETCONF_STATE_TX, /**< device is in transmit mode */ | ||
/* add other states if needed */ | ||
} ng_netconf_state_t; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
adjusted documentation for states |
|
||
/** | ||
* @brief Option parameter to be used with *NETCONF_OPT_STATE* to set or get | ||
* the state of a network module |
There was a problem hiding this comment.
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".
addressed documentation text for the state enum |
ACK, please squash. |
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 |
*/ | ||
|
||
#ifndef NET_CONF_H_ | ||
#define NET_CONF_H_ |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
5363e3d
to
b3d2886
Compare
rebased and squashed. |
b3d2886
to
c5c9c3d
Compare
fixed header macro name as pointed out by @LudwigOrtmann and squashed - will merge when Travis is happy. |
Travis is happy -> go. |
ng_net: add global configuration options
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...