SX1276 radio: reported SNR is wrong

[dbarthel-ol]

The SX1276 radio chip has a register containing the SNR after frame reception. This register value must be divided by 4 (taking sign into account) to yield the real SNR value in dB. This is correctly computed in src/radio/sx1276/sx1276.c and temporarily stored in the snr variable (around lines 1505-1516), but the result is not stored back into SX1276.Settings.LoRaPacketHandler.SnrValue. Therefore upper layers get an incorrect value.
I will provide a pull request for that.
I suspect SX1272 is affected similarly.

[Mobiak]

As I agree with you, this issue already came up in issue #75.

[mluis1]

Thanks for your comments. But as already answered in issue #75 the current implementation is correct according to the LoRaWAN specification.

What we usually do is to perform the conversion to dB in the main application as shown in the example below.

static void McpsIndication( McpsIndication_t *mcpsIndication )
{
    if( mcpsIndication->Status != LORAMAC_EVENT_INFO_STATUS_OK )
    {
        return;
    }

    switch( mcpsIndication->McpsIndication )
    {
        case MCPS_UNCONFIRMED:
        {
            break;
        }
        case MCPS_CONFIRMED:
        {
            break;
        }
        case MCPS_PROPRIETARY:
        {
            break;
        }
        case MCPS_MULTICAST:
        {
            break;
        }
        default:
            break;
    }

    // Check Multicast
    // Check Port
    // Check Datarate
    // Check FramePending
    // Check Buffer
    // Check BufferSize
    // Check Rssi
    // Check Snr
    // Check RxSlot
    LoRaMacDownlinkStatus.Rssi = mcpsIndication->Rssi;
    if( mcpsIndication->Snr & 0x80 ) // The SNR sign bit is 1
    {
        // Invert and divide by 4
        LoRaMacDownlinkStatus.Snr = ( ( ~mcpsIndication->Snr + 1 ) & 0xFF ) >> 2;
        LoRaMacDownlinkStatus.Snr = -LoRaMacDownlinkStatus.Snr;
    }
    else
    {
        // Divide by 4
        LoRaMacDownlinkStatus.Snr = ( mcpsIndication->Snr & 0xFF ) >> 2;
    }
    LoRaMacDownlinkStatus.DownlinkCounter++;
    LoRaMacDownlinkStatus.RxData = mcpsIndication->RxData;
    LoRaMacDownlinkStatus.Port = mcpsIndication->Port;
    LoRaMacDownlinkStatus.Buffer = mcpsIndication->Buffer;
    LoRaMacDownlinkStatus.BufferSize = mcpsIndication->BufferSize;

    if( ComplianceTest.Running == true )
    {
        ComplianceTest.DownLinkCounter++;
    }

    if( mcpsIndication->RxData == true )
    {
...
}

[shpasser]

According to the data sheet of sx1276 the SNR value read from the device register is in twos complement format, why not just divide the value by 4?

[zxl-ben]

@shpasser As I agree with you.