search

G4lile0 / tinyGS

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

Incorrect SPI low-level access method for SX126x #62

Closed jgromes closed 11 months ago

jgromes commented 3 years ago

Hey guys,

While updating my station, I noticed tinyGS requires RadioLib godmode to build. Digging further, I noticed the following:

https://github.com/G4lile0/tinyGS/blob/2128f1410cfd229b9b5478a122446e393b41a0a2/tinyGS/src/Radio/Radio.cpp#L172-L175

Couple of thoughts on that:

  1. Please don't use godmode in production code, it's supposed to be a development feature. I'm guessing this is some sort of low-level direct hardware access, but as a rule of thumb, if you find yourselves in a situation you need to use it, it might be better to raise an issue in RadioLib repository.
  2. You're actually using incorrect SPI method for SX126x. The SPI interface is completely different from SX127x, so you have to use SX126x::writeRegister() instead of Module::SPIsetRegValue(). Also most of SX126x SPI registers are undocmented, and things like modem configuration have to be done through a command-based interface.

Overall, using godmode to achieve this seems like a massive overkill, since it will make literally every single method in the library accessible from the user sketch. I think a macro to selectively enable couple low-level hardware methods would be better here.

G4lile0 commented 3 years ago

Hi Jan, You are right the SPI reg it is experimental and intended only for the sx127x, I just commented the code for the Sx126x . We used the godmode for some options that aren't accessible from the RadioLib library. For example some FSK satellites use preamble polarity AA and RadioLib is fixed to preamble polarity 55. We first need to identify the missing parameters then we will raise an issue in RadioLib repository.

Also we plan to improve the management of the different modules under tinyGS, right now we support SX1278, Sx1276, patching the SX1278 code on RadioLib; soon will address each HW module with the right RadioLib module, to manage them without patching the library, and also will allow us to increase the number of supported modules (for example the SX1280).

jgromes commented 3 years ago

Thanks for the info, I just really wanted to make it clear using godmode in production is a bad idea. Let me know if there's anything in RadioLib I can help with!