RSDK-9814 Increase packet buffer size to 10 #24
Conversation
| packets := unsafe.Slice((*C.struct_lgw_pkt_rx_s)(unsafe.Pointer(p)), maxRxPkt) | ||
| for i := range numPackets { | ||
| // received a LORA packet | ||
| var payload []byte |
There was a problem hiding this comment.
This produces a []bytes(nil), is this what you want instead of a []bytes{}? both have a length of 0, but one has a nil array.
I think your number of packets check is defensive enough, but just wanted to point this out in case we need to be defensive payload checking and logging for a nil case there as well.
There was a problem hiding this comment.
If it's handled elsewhere just add a comment about where a nil check for the payload is handled, do we have a test for a nil handlepacket payload?
There was a problem hiding this comment.
I think the code right below this should handle this sufficiently, if the packet size is zero we skip calling handlePacket, otherwise we append bytes to the payload so it won't be nil.
There was a problem hiding this comment.
So we'll never have a packet non-zero size with a nil payload?
There was a problem hiding this comment.
the size that is sent in the packet is the payload size, so it should be zero if the payload is nil. So something would have to be wrong with the packet to not match. doesn't hurt to have a nil check just in case, will add.
gateway/gateway.go
Outdated
| g.logger.Errorf("error receiving lora packet") | ||
| default: | ||
| // convert from c array to go slice | ||
| packets := unsafe.Slice((*C.struct_lgw_pkt_rx_s)(unsafe.Pointer(p)), maxRxPkt) |
There was a problem hiding this comment.
[supernit] do we need the extra variable? wondering if we can just do this
| packets := unsafe.Slice((*C.struct_lgw_pkt_rx_s)(unsafe.Pointer(p)), maxRxPkt) | |
| packets := unsafe.Slice((*C.struct_lgw_pkt_rx_s)(unsafe.Pointer(p)), int(C.MAX_RX_PKT)) |
There was a problem hiding this comment.
yeah I can just do that
| default: | ||
| // convert from c array to go slice | ||
| packets := unsafe.Slice((*C.struct_lgw_pkt_rx_s)(unsafe.Pointer(p)), maxRxPkt) | ||
| for i := range numPackets { |
There was a problem hiding this comment.
[q] so before we didn't need this because we set MAX_RX_PKT = 8.
now that MAX_RX_PKT = 10, we technically need to loop thru the packets?
There was a problem hiding this comment.
before we weren't using the MAX_RX_PKT in the C receive function we were doing return lgw_receive(1, packet);, so only one packet was received.
When debug was on in the gateway, the line
WARNING: not enough space allocated, fetched 2 packet(s), 1 will be left in RX buffershowed occasionally. Increased the buffer to 10 to ensure we don't miss any packets, no longer see the warning when manually testing.