-
Notifications
You must be signed in to change notification settings - Fork 2.1k
netdev2: provide capability to pass up packet status information #4648
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
Conversation
NACK in it's current state -> IMHO netdev2 is link layer agnostic, and I don't like a solution, in which ethernet devices have to hand over radio specific information (at least conceptual, though in practice you would just pass Now lets see if there is a better solution: if we would keep strictly to the plain concept of Adding an additional function like So how about generalizing this PR just slightly: int (*recv)(netdev2_t *dev, char* buf, int len, void *info); and make the type of typedef struct {
uint8_t rssi; /**< RSSI of a received packet */
uint8_t lqi; /**< LQI of a received packet */
} netdev2_radio_info_t; when receiving a packet? This paradigm could be even extended to sending packets. So by changing the send function to int (*send)(netdev2_t *dev, const struct iovec *vector, int count, void *info); we could give link layer specific information back, as for example the number of retries (if available...). But if this makes sense we should discuss separately... |
I'm with you on that. I already need to call a lot of get, with netdev2 (to get the source address and its length e.g.), I like to keep this to a minimum.
I had it like this first, but than decided it to do it as it is currently in the PR… don't know why I changed it... Will do.
Symmetry is always better :-). I think I will provide this API change, but not the type needed for it for now. |
926ca1f
to
693bc0f
Compare
Done. |
looks ok to me. One thing we should spend a thought about is the naming scheme for the info types (as |
b99b0b8
to
35afa05
Compare
@haukepetersen ping? |
I guess @haukepetersen meant something like: struct netdev2_radio_rx_info {
uint8_t rssi; /**< RSSI of a received packet */
uint8_t lqi; /**< LQI of a received packet */
};
typedef struct netdev2_radio_rx_info netdev2_ieee802154_rx_info_t;
typedef struct netdev2_radio_rx_info netdev2_cc110x_rx_info_t; |
Done and adapted existing drivers. Will introduce |
What's the intention of the |
From #4648 (comment):
|
What happened to the good old: "implement that when there is a (concrete) need for it"? ;-) |
I did not implement anything, I just provided an API to do it ;-) |
Now looking at this PR again I think @OlegHahm has a point -> how about we keep the send function untouched for now and add it when needed? Also I would like @kaspar030 to take a look at this change and get his approval. |
Done |
ACK |
07f76cf
to
b7735c5
Compare
Rebased and squashed. |
*/ | ||
struct netdev2_radio_rx_info { | ||
uint8_t rssi; /**< RSSI of a received packet */ | ||
uint8_t lqi; /**< LQI of a received packet */ |
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.
@OlegHahm how about including the number of retries here? But this might be best targeted in another PR?!
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.
Number of retries in the RX info?
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.
Ups, mixed up contexts. Never mind...
ACK also from my side. |
b7735c5
to
29bd517
Compare
Fixed some errors and squashed immediately |
29bd517
to
bd8d2d3
Compare
Can someone look over the changes and say if they are still okay? |
Looks ok to me. The doc says the info's datatype is "depending on the device type". That means NETDEV2_DEVICE_TYPE? Or how can I find out which type exactly? |
By name ;) |
netdev2: provide capability to pass up packet status information
This proposes an API change to pass up packet status information like RSSI or LQI to upper-layers. The CC11xx driver circumvents this problem this by (seemingly?) bypassing the netdev2 API altogether for GNRC and for my preliminary netdev2 port of IEEE 802.15.4 (#4645) I just appended this information to the packet, as most device drivers do anyway. With the last decision I'm not happy however and this PR tries to improve the overall situation.