search

jgromes / RadioLib

Universal wireless communication library for embedded devices
https://jgromes.github.io/RadioLib/
MIT License
1.56k stars 389 forks source link

SX126x::SetFrequency errors are unnoticed #78

Closed BarryPSmith closed 4 years ago

BarryPSmith commented 4 years ago

Describe the bug sx1262::setFrequencyRaw doesn't check the return code from setRfFrequency and always returns ERR_NONE.

To Reproduce I noticed this trying to figure out why my radios aren't talking to each other with the latest code. A set frequency (0x86) command returned 0xAA 0xC7 0x00 0xEB where with an old version of radiolib it was returning only 0xA2. I only found this out looking through the verbose output. I don't know how to reliably reproduce it, but it's clear in the source code.

Expected behavior If the device returns an error when asked to execute a command, propagate that error to the caller.

Screenshots If applicable, add screenshots to help explain your problem.

Additional info (please complete):

jgromes commented 4 years ago

Thanks, that's a major oversight. Obvisouly the ERR_NONE has no place there: https://github.com/jgromes/RadioLib/blob/58b56ec3a2828b2b21cbc523b221180ea7063d36/src/modules/SX126x/SX126x.cpp#L1252-L1256

There's also a similar issue in setTCXO: https://github.com/jgromes/RadioLib/blob/58b56ec3a2828b2b21cbc523b221180ea7063d36/src/modules/SX126x/SX126x.cpp#L1038-L1047

Will fix both asap.

jgromes commented 4 years ago

Should be fixed in 4729b8518be7d27732e500ffd002c005e33e9ee7, but let me know once you test it out - I wasn't getting frequency errors previously.

BarryPSmith commented 4 years ago

Thanks! The frequency errors were caused by the TCXO being set. I've now got it disabled. I'll take your word that it passes the errors up to the caller :-) - it was too frustrating for me to want to go back to that.

jgromes commented 4 years ago

It should pass both errors now ;)

The bigger issue (related to SX126x status in general) is that it's not clear from the datasheet which command are the status bytes related to. I think that the status byte you get from any command is actually the status of the previous command you sent - since the chip needs some time to execute the command. This might warrant further investigation - though I'd rather avoid having to add call to getStatus after every SPI command to get the accurate command result ...