at86rf2xx: detect broadcast and set NETIF flag when receiving#4078
Conversation
daniel-k
added
Type: enhancement
drivers/at86rf2xx/at86rf2xx_netdev.c
Outdated
There was a problem hiding this comment.
@DipSwitch tmp is used in line 205 (or 213 with this change).
thomaseichinger
added
the
CI: ready for build
drivers/at86rf2xx/at86rf2xx_netdev.c
Outdated
There was a problem hiding this comment.
Not that this code is optimised by any means, but as a proposal, you could set the flag here and remove it below if not applicable. Saves us one byte on stack, one assignment and one if-statement. \o/ 😊
There was a problem hiding this comment.
Less code to maintain. I like it!
|
Code looks good, one proposal for consideration above. Added the CI label. |
daniel-k
force-pushed
the
pr/at86rf2xx_detect_broadcast
branch
from
fe8ced1 to
0b160a4
Compare
|
@thomaseichinger |
drivers/at86rf2xx/at86rf2xx_netdev.c
Outdated
There was a problem hiding this comment.
@daniel-k That's the wrong way around. Isn't it? In this case it is not a broadcast address... I thought about setting the flag where you set broadcast = true; before and hdr->flags &= ~GNRC_NETIF_HRD_FLAGS_BROADCAST; here.
There was a problem hiding this comment.
That's why you really want to have code reviews facepalm. Fixed in 78192eb
daniel-k
force-pushed
the
pr/at86rf2xx_detect_broadcast
branch
from
43f66e9 to
78192eb
Compare
|
Ok changes look good. I can't get a rf233 and a rf231 transceiver talk to each other with txtsnd though. This is unrelated to this change as it is the same with master from yesterday. @daniel-k can do you get link layer packet dumps? I see they are receiving, did I miss something? |
|
Ok, this is independent from this PR. Code looks good, Travis is green, ACK and go. |
at86rf2xx: detect broadcast and set NETIF flag when receiving
5 participants
The driver should set the broadcast flag (GNRC_NETIF_HDR_FLAGS_BROADCAST) for incoming packets.