ARMmbed / mbed-semtech-lora-rf-drivers

Semtech's LoRa RF drivers for mbed OS
Other
31 stars 25 forks source link

SNR not exposed correctly #33

Closed maxgerhardt closed 4 years ago

maxgerhardt commented 5 years ago

The LoRaWAN stack now features the lorawan.get_rx_metadata(lorawan_rx_metadata& metadata); function with which RX metadata infos like SNR, RSSI and datarate are conveyed to the user after a reception.

However, when printing the SNR returned by that function, I get things like

        lorawan_rx_metadata rxMetadata;
        lorawan.get_rx_metadata(rxMetadata);

        time_t currTime;
        time(&currTime);
        printf("RX %d byte RSSI %d SNR %d %llu\n", (int) retcode,
               (int) rxMetadata.rssi,
               (int) rxMetadata.snr,
               currTime);
RX 51 byte RSSI -46 SNR 34 1546955257
RX 2 byte RSSI -45 SNR 30 1546955260
RX 2 byte RSSI -26 SNR 34 1546955374

The gateway reports an SNR of about 7-10 for my values, but the returned value here is 30-34, about 4 times larger. So I looked at the code responsible for computing the SNR and noticed the following:

https://github.com/ARMmbed/mbed-semtech-lora-rf-drivers/blob/16958f814d505cfbbedfa16d9bf8b9dff0e0442b/SX1276/SX1276_LoRaRadio.cpp#L1916-L1985

The code reads out the REG_LR_PKTSNRVALUE register from the SX1276 into the _rf_settings.lora_packet_handler.snr_value structure. The value in this structure is later returned by get_rx_metadata. The code then goes on to check if the highest bit (sign bit) is set. If yes, it divides by 4 and inverts the sign. If no, it just divides by 4. The value of that computation is saved in the temporary variable snr.

However, this value is not written back to _rf_settings.lora_packet_handler.snr_value and thus the user will get the raw register value instead of the decoded value. This explains why I'm getting about 4 times the value as I expected.

I can re-do these computation in my code, but is this a mistake that should be fixed in the driver code? @hasnainvirk

ciarmcom commented 5 years ago

ARM Internal Ref: IOTCELL-1742

AnttiKauppila commented 4 years ago

Closed due to inactivity

maxgerhardt commented 4 years ago

..an issue I opened in which the developers never responded is closed by them for inactivity? Huh ó_Ò? So I guess this is a WONTFIX?

AnttiKauppila commented 4 years ago

“At this time, due to our prioritisation of work on customer projects, we are unable to devote the time to fix this issue. As an open source project, we welcome fixes to Mbed OS, so if would like to contribute a fix yourself then we would very much welcome that, please see our contribution guidelines here - https://os.mbed.com/docs/mbed-os/v5.13/contributing/workflow.html#contributions

maxgerhardt commented 4 years ago

@AnttiKauppila

image