Add SNR property according to datasheet

[AdamLeyshon]

Hello, I have implemented what I believe to be the correct function according to the datasheet for the RFM95W
(RFM95W-V2.0-1.pdf from HOPERF)

The register is at 0x19, it 8 bits wide and is a signed value stored as a single byte using two's complement and should be divided by 4 to get the dB value.

Tested on my own AdaFruit RFM95W Bonnet for Raspberry Pi and AdaFruit RFM95W Breakout board connected to a Raspberry Pi Pico running the latest Circuit Python (6.2 beta)

I receive values between 6 and 9 when the transmitting station is near by,

Unfortunately I have nothing to compare this value against to determine if what the hardware says is correct.

Please let me know if the implementation is incorrect.

[brentru]

@jerryneedell do you have a chance to look at this?

[jerryneedell]

@brentru sure -- will take a look.

[jerryneedell]

This looks good to me. One suggestion:
For the RSSI - we have 2 ways to read it.
the .rssi property is the instantaneous RSSI stored
the .last_rssi is saved immediately after the packet is received.
I just ran some tests and I get the same reading fo riot, but as I recall, there were times when they were different . I don't know if the same applies to SNR.

If you want, we could add .last_snr in the same way -- I tried it and as with the RSSI, I get the same reading

here are the changes I made:

pi@gjnpilora:~/projects/blinka/rfm9x_ack $ diff adafruit_rfm9x.py ~/projects/adafruit_github/Adafruit_CircuitPython_RFM9x/adafruit_rfm9x.py 
289,293d288
<         self.last_snr = 0.0
<         """The SNR of the last received packet. Stored when the packet was received.
<            The instantaneous SNR value may not be accurate once the
<            operating mode has been changed.
<         """
790,791d784
<         # save last RSSI reading
<         self.last_snr = self.snr

As to the value returned, I have no way to determine its correctness but I think you have implemented it correctly.

Let me know what you think of adding the .last_snr properly as well.

[jerryneedell]

With the added values I get the same results
the RSSI/SNR are the .last_rssi/.last_snr
the RSSIx/SNRx are the .rssi/.snr

I'll see if I can reproduce a case with some deviation

Received (raw header): ['0x0', '0x1', '0x3', '0x0']
Received (raw payload): bytearray(b"I\'m awake (feathers2): count 1 2 0 battery 3.811V")
RSSI: -61
RSSIx: -61
SNR: 6.75
SNRx: 6.75

[AdamLeyshon]

Sound like a good idea to implement a "last" SNR as well to prevent the value changing in the register after receiving a packet but before we read the value after processing the current packet. 28 Jan 2021 20:21:19 jerryneedell <[email protected]>:

…

@jerryneedell requested changes on this pull request. This looks good to me. One suggestion: For the RSSI - we have 2 ways to read it. the .rssi property is the instantaneous RSSI stored the .last_rssi is saved immediately after the packet is received. I just ran some tests and I get the same reading fo riot, but as I recall, there were times when they were different . I don't know if the same applies to SNR. If you want, we could add .last_snr in the same way -- I tried it and as with the RSSI, I get the same reading here are the changes I made: ***@***.***:~/projects/blinka/rfm9x_ack $ diff adafruit_rfm9x.py ~/projects/adafruit_github/Adafruit_CircuitPython_RFM9x/adafruit_rfm9x.py 289,293d288 < self.last_snr = 0.0 < """The SNR of the last received packet. Stored when the packet was received. < The instantaneous SNR value may not be accurate once the < operating mode has been changed. < """ 790,791d784 < # save last RSSI reading < self.last_snr = self.snrAs to the value returned, I have no way to determine its correctness but I think you have implemented it correctly. Let me know what you think of adding the .last_snr properly as well. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub[#59 (review)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/ABXBTVVXZ775W2TY64TDNELS4HBLLANCNFSM4WXT3VFA]. [https://github.com/notifications/beacon/ABXBTVUGQOTOVE3DAKRP7QDS4HBLLA5CNFSM4WXT3VFKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOEJ7GI5A.gif]

[jerryneedell]

Great. If you can implement that and push a new commit, it should be ready to go.
Thanks for adding this.

[jerryneedell]

Here is more information on the history of "last_rssi" implementation adafruit/Adafruit_CircuitPython_RFM69#22
It was originally implemented on the RFM69 where it apparently had some impact. It is not clear if it matters for the RFM9x but it also "does no harm" so I think we may as well continue with it for RSSI and SNR.

[AdamLeyshon]

I've implemented the requested changes for adding last_snr attribute to the class.

[jerryneedell]

Thank you! -- Looks good to me! Tested on Raspberry Pi ZeroW with RFM9x Bonnet.

[jerryneedell]

@brentru OK to merge this?

[brentru]

@jerryneedell Looks OK to me, go ahead :)

[AdamLeyshon]

Thanks all, Hope to contribute more in future :)

[jerryneedell]

@AdamLeyshon Thank you! We look forward to future contributions!

[jerryneedell]

BTW -- I also made a new release - v 2.1.2 -- should be available via PyPy soon and in the library bundle tonight.