Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SX1276, LoRaWAN] Fix confusing return value in LoRaWAN::processJoinAccept(...) #1262

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

VolkerChristian
Copy link
Contributor

@VolkerChristian VolkerChristian commented Oct 9, 2024

The error RADIOLIB_ERR_LORA_HEADER_DAMAGED returned from phyLayer->readData(...) and signaled from a SX1276 is in fact correctly ignored but the state variable is not corrected.

This confuses the caller due to the wrongly returned state value RADIOLIB_ERR_LORA_HEADER_DAMAGED. Especially radiolib-persistence did think it can not join a network.

Maybe this can be a hotfix?
Because I just have a lot of SX1276 on hand and need the library working with them under Arduino IDE and PlatformIO in my class in two weeks. I don't want to ask and instruct every student to fix its RadioLib installation by hand if it's not absolutely necessary. This could confuse some students. ;-)

Best regards
Volker

…ccept

The error RADIOLIB_ERR_LORA_HEADER_DAMAGED board returned fromreadData(...)
and signaled from a SX1276 is in fact correctly ignored but the state
variable is not corrected.

This confuses the caller due to the wrongly returned state value
RADIOLIB_ERR_LORA_HEADER_DAMAGED.

Change: Clear state also.
@jgromes
Copy link
Owner

jgromes commented Oct 9, 2024

Looks good to be merged, thank you for the fix!

Regarding the deployment of the fixed version, it's been just a few days since the last patch (7.0.2), so I'd like to hold on with the releases for a little bit. Especially since we have a couple open issue reports where it's not clear yet whether they are due to some issues in the library.

If I'm not mistaken though, Platformio does allow to directly use a git repository, instead of the version in the package registry. Maybe that could be an option for you?

@jgromes jgromes merged commit a7feeee into jgromes:master Oct 9, 2024
30 checks passed
@VolkerChristian VolkerChristian deleted the Fix_ConfusingReturnValue branch October 9, 2024 19:08
@VolkerChristian
Copy link
Contributor Author

Ah, thank you for the hint - I just tested successfully PlatformIO with my forked RadioLib repository! That solved my problem! No need for a hotfix!

Thank you!

@StevenCellist
Copy link
Collaborator

@VolkerChristian if you want the most minimal clone of a git repository (RadioLib tends to come with quite some .git history), you can try the following command:

git clone --filter=blob:none https://github.com/jgromes/RadioLib.git 

The filter ensures that only the most recent .git history is pulled which can save up to 100s of Mbs of the download (I don't really understand why it sometimes pulls all the history and sometimes doesn't).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants