Hooray!

Nice to see one more problem with the atrf driver solved!

Though I am not sure, if I completely like the actual solution: allocating a full packet on the stack could be potential dangerous. When rewriting the driver, one of the main ideas was to get rid of this additional layer of buffering. Now it seems that this is not possible when wanting to use LQI values I guess we can not prevent this. So instead of allocating this temporary buffer on the stack, I think I would tend to allocate it statically as part of the device descriptor. This could also simplify the rest of the receive function code a bit...

So instead of allocating this temporary buffer on the stack, I think I would tend to allocate it statically as part of the device descriptor.

Sure. This was the easiest approach to be honest. Would you take over this part @haukepetersen?

This would also fix RSSI readout? In my setup I see that the RSSI is read
out properly in the netif thread, but when the message receives the calling
thread the value is cleared.

Just curious :)
On 24 Sep 2015 16:45, "Daniel Krebs" notifications@github.com wrote:

So instead of allocating this temporary buffer on the stack, I think I
would tend to allocate it statically as part of the device descriptor.

Sure. This was the easiest approach to be honest. Would you take over this
part @haukepetersen https://github.com/haukepetersen?

—
Reply to this email directly or view it on GitHub
#3954 (comment).

This would also fix RSSI readout? In my setup I see that the RSSI is read out properly in the netif thread, but when the message receives the calling thread the value is cleared.

I don't think so. RSSI readout doesn't really change with this PR. Do you use RSSI for something? AFAIK there's no documentation in RIOT what netif->rssi and netif->lqi really mean apart from the bigger the better. This driver only stores the raw uint8_t rssi value, although it translates to -94 + rssi [dBm] IIRC.

Thanks for the reply, but that was a different problem :)

AFAIK there's no documentation in RIOT what netif->rssi and netif->lqi really mean apart from the bigger the better.

Btw that's the same for Linux for one simple reason: at least RSSI is vendor specific. Some vendors assign RSSI values between 0 and 3 while other uses the full width of the data type.

Good job @daniel-k ! Hopefully LQI and RSSI will be useful one day with RPL ;-)
I will test this PR on Monday and we will merge the PR as soon as the release is over.

So instead of allocating this temporary buffer on the stack, I think I would tend to allocate it statically as part of the device descriptor.
I would just add that we should consider which one of the solution take more space at compilation since we're working with constrained device. I don't know the answer but someone here may know.

In the current approach we the only read access out of frame buffer sequence is the LQI read access. Maybe we could change the current approach to something using fb access like

at86rf2xx_fb_read_start(dev); /* CS, SPI acquire and send SPI FB access */
...
at86rf2xx_fb_read(dev, mhr, 2); /* read FCS */
hdr_len = _get_frame_hdr_len(mhr);
at86rf2xx_fb_read(dev, &(mhr[2]), hdr_len - 2); /* read addresses and the like */
...
payload = gnrc_pktbuf_add(hdr, NULL, (pkt_len - hdr_len), dev->proto);
at86rf2xx_fb_read(dev, payload->data, payload->size); /* dump payload in pkt buffer snip */
...
at86rf2xx_fb_read(dev, &(netif->lqi), 1);
at86rf2xx_fb_read_stop(dev); /* chip unselect, SPI release */
dev->event_cb(NETDEV_EVENT_RX_COMPLETE, payload);

@thomaseichinger, ok, your approach seems a bit cleaner, I don't have a strong opinion on which way to go since the result is going to be the same

I really liked @thomaseichinger proposal. It seems to work and it should be quite easy to review now because code-wise not much has changed.

I tested it, I got lot of 255 values and some below. Seems to work but how may I really tell that it works.

The IEEE 802.15.4 standard defines the LQI as a characterization of the strength
and/or quality of a received frame. The use of the LQI result by the network or
application layer is not specified in this standard. The LQI value shall be an integer
ranging from zero to 255, with at least eight unique values. The minimum and maximum
LQI values (0x00 and 0xFF) should be associated with the lowest and highest quality
compliant signals, respectively, and LQI values in between should be uniformly
distributed between these two limits.

How may I make the signal quality worse? Otherwise I ACK this PR and I'll merge it.

@bapclenet
From my observations the LQI will be quite stable at 255 as long as RSSI (raw) is > 0. Increase distance/walls between your boards and try again.

@haukepetersen You okay with the current implementation?

Great let's merged, will come back if we find any problem concerning LQI value if @haukepetersen agrees on the new implementation?

ping @haukepetersen

Sorry for the sleepiness on my side... I am good with this change - though I have no time to test today. But @bapclenet, if you ack still holds, I say lets merge this!

NACK for now, doesn't work for iotlab-m3.

Damn! Deadline for my thesis is next tuesday so I won't be able to fix this until then 👿 Whats the release deadline for 2015.12 again?

Feature freeze is today, I'm investigating.

Ok, found the problem. Please see daniel-k#4.

@daniel-k Please squash.

ACK when squashed and Travis passed.

@thomaseichinger
Squashed. Now waiting for travis.

@thomaseichinger
Any idea why avr and arm7 are failing right away?

Woohoo, Travis is happy. Finally this PR can be merged :)

\o/

Great! Thanks

Cool! Nice job everyone!