G4lile0 / tinyGS

📡 Open Ground Station Network 🛰
GNU General Public License v3.0
922 stars 176 forks source link

Update SX127x.cpp #198

Closed estbhan closed 10 months ago

estbhan commented 11 months ago

It was noted that, in FSK mode, it was not possible to receive a packet with last beta versions. The logic within the main "else" in the method "float SX127x::getRSSI", seemmed to indicate that, if "skipReception" is false the board should be set to start receiving, and then set to standby mode just before returning the RSSI value. This seemed to stop or avoid FSK reception in some way. The logic has been changed in order to set to standby only if the skipReception flag is true. In principle with this change, FSK packets are received and Noise Floor measurement is active and changing in the TinyGS station dashboard.

jgromes commented 11 months ago

@estbhan I don't know how exactly is getRSSI being used in tinyGS source code, but here's my two cents on the issue.

The purpose of the skipReceive argument is to skip putting the device to Rx mode prior to taking the RSSI measurement. The logic here is that if the device is in standby, you need to first put it to Rx to get meaningful RSSI result. However, if the SX127x is already in Rx more when you're calling getRSSI, you shouldn't put it into receive mode again. When exiting, if you did put the device manually into Rx by setting skipReceive to false, you should put it back to standby.

So I would argue the logic in RadioLib is correct, and if tinyGS uses getRSSI while the radio is already receiving, it should set the argument to true.

On a slightly different note, even if this gets merged, it will remain in the RadioLib upstream. So maybe it would be better if tinyGS used RadioLib as a submodule or Platformio package.

estbhan commented 11 months ago

Dear Jan, thanks for your comments, they are very much appreciated. I understand now the purpose of the code in your function.

I suggest to reject this commit, and I will work in identifying all the calls to the function getRSSI to see whether the value of the argument skipReceive passed is coherent with the function logic, I mean, skipReceive is false when the board is in standby mode, and skipReceive is true when the board is already in reception.

Thanks again and Best Regards Esteban

G4lile0 commented 10 months ago

Thanks Esteban and Jan, I just updated the beta release with the latest RadioLib and perform some changes with the tinyGS code that I think that fix the issue. Thanks!

jgromes commented 10 months ago

@G4lile0 instead of having to update a local copy of RadioLib, perhaps it would be better to include it as a platformio dependency? It's quite easy just by adding a lib_deps option to tinyGS platformio.ini file, you can also specify either the latest master or a specific version, e.g.:

[env]
lib_deps =
  jgromes/RadioLib @ 6.1.0
G4lile0 commented 10 months ago

You are right, as we doesn't have a patched RadioLib version since a long time, it doesn't make sense to include the local copy.. I just updated it.. for the beta branch we will use the latest commit using: [env] lib_deps = https://github.com/jgromes/RadioLib/archive/refs/heads/master.zip

Thanks