Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

fix rssi computation from rssic if SNR < 0 #1777

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

BenoitDuffez
Copy link
Contributor

@BenoitDuffez BenoitDuffez commented Jul 21, 2022

rssis is supposed to be the useful signal strength
rssic is supposed to be the received signal strength

The blockchain expects the rssi to be the rssis. However when falling back to rssic when rssis is unavailable, the computation is wrong when SNR < 0
Indeed, in that case the useful signal strength is below the noise level, which is rssic. So the rssis = SNR + rssic when SNR < 0
This should help use the appropriate GWMPv2 value from the packet forwarders

rssis is supposed to be the useful signal strength
rssic is supposed to be the received signal strength

The blockchain excepts the rssi to be the rssis. However when falling back to rssic when rssis is unavailable, the computation is wrong when SNR < 0
Indeed, in that case the useful signal strenght is below the noise level, which is rssic. So the rssis = SNR + rssic when SNR < 0
This should help use the appropriate GWMPv2 value from the packet forwarders
@VeryOuch
Copy link

This should be fixed because otherwise certain hotspots (as e.g. Kerlink) may get invalid PoC events due to the RSSI shown incorrectly high in some cases. This illustration shows a Kerlink being declared as an invalid witness of a beacon because the RSSI is too high (RSSI shown as -109 and SNR -16) while other hotspot brands would have shown this as RSSI -125 and SNR -16, which would not have been invalid.
screenshot 349
.

@Vagabond
Copy link
Contributor

When is RSSIS unavailable on GWMPv2?

@BenoitDuffez
Copy link
Contributor Author

rssic/rssis are available on RF implementations of Semtech reference design v2.x
rssic only (rssis unavailable) is on RF implementations of Semtech reference design v1.x (most if not all of Helium hotspots)

@BenoitDuffez
Copy link
Contributor Author

I think this is the continuation of this: #1083 (comment)

@Vagabond
Copy link
Contributor

Where did you find rssis = SNR + rssic when SNR < 0? I've not seen that before

@BenoitDuffez
Copy link
Contributor Author

BenoitDuffez commented Jul 26, 2022

When SNR < 0, the signal is below the noise level, hence rssic is the noise level. Thus, signal level is rssic+snr

@pdvaneijk
Copy link

Just to confirm, so for any Helium gateway that either uses the SX1301/08 or SX1302/03:

if SNR is positive, use rssic

if snr<0, packet power is rssi+snr

The Semtech V2.X gateway is a totally different beast (ADI RF front end, large FPGA + 2-8 SX1301s) and there are no Helium gateways based on the design.

@BenoitDuffez
Copy link
Contributor Author

BenoitDuffez commented Jul 26, 2022

I might get a pit pedantic, but my PR is about JSON format (GWMP version) and not RF ref des. Ref des v1 can use GWMPv2 (all Kerlink do).

Also, on a side note, I'm using a DIY miner with a Kerlink ref des v2 gateway (the 2x1301 arch you mentioned), it works fine (besides the SNR < 0 issue mentioned here). So there's at least one! 😁

@ke6jjj
Copy link
Contributor

ke6jjj commented Jul 26, 2022

For context, everyone, the point here is to pull out the best possible estimation of the received signal power so that the Free-Space Path Loss equation can accurately rule on signals that are impossibly strong for the distance asserted.

@pdvaneijk
Copy link

I might get a pit pedantic, but my PR is about JSON format (GWMP version) and not RF ref des. Ref des v1 can use GWMPv2 (all Kerlink do).

Also, on a side note, I'm using a DIY miner with a Kerlink ref des v2 gateway (the 2x1301 arch you mentioned), it works fine (besides the SNR < 0 issue mentioned here). So there's at least one! 😁

Semtech V2.X gateways use V2 GWMP and Semtech V1 Gateway and Corecell use V1 GWMP so I think we are good

@BenoitDuffez
Copy link
Contributor Author

If you use the semtech packet forwarder, yes. Kerlink hotspots do not, they all use GWMPv2. So this PR is needed.

maps:get(<<"rssic">>, Obj, undefined)
SNR = maps:get(<<"lsnr">>, Obj, 0),
case SNR < 0 of
true -> SNR + maps:get(<<"rssic">>, Obj, undefined);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn’t this line result in an arithmetic operation error if rssic is undefined?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it would, yes. I think a missing rssic is a brutal fatal error, though. So the arithmetic error is appropriate to propagate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's defined to be present in this protocol version

@Vagabond Vagabond merged commit 7f47fbb into helium:master Aug 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants