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

Fix args to begin() in SX12xx settings examples #688

Merged
merged 1 commit into from
Mar 5, 2023

Conversation

brghena
Copy link
Contributor

@brghena brghena commented Mar 3, 2023

The call to radio.begin() in the SX126x and SX128x Settings example code were missing arguments. Since all the arguments have default assigned values, the compiler wasn't complaining, but the values were being applied to incorrect parameters based on the comments.

For example, previously the comment for the SX126x Settings example said that radio2 was being configured with output power of 2 dBm and preamble length of 20 symbols. Actually it was being configured with an output power of 20 dBm and preamble was left default (8).

I fixed the calls to begin() in the SX126x and SX128x Settings examples. The SX127x Settings example was already fine. I did not check the non-Semtech transceiver examples.

Open question: For the SX128x Settings Example, the "sync word" was left out and there was no comment stating an expected value. I used the default value of 0x12 (private network) there, but I'm not certain if that matches your intent. The SX126x Settings example uses 0x34 (lorawan public network) and SX127x uses 0x14 (no meaning I'm aware of).

Also, thanks for this awesome library! 💜

@jgromes jgromes merged commit 50574d6 into jgromes:master Mar 5, 2023
@jgromes
Copy link
Owner

jgromes commented Mar 5, 2023

Nice catch, thanks! To be perfectly honest, I usually think of the XYZ_Settings examples as more of an API guide than an actual example intended to be run as-is. But still, there shouldn't be mistakes there.

Regarding the SX128x sync word, I'm not entirely sure either, as it operates at 2.4 GHz where LoRaWAN comaptibility is not a factor. Furthermore, the datasheet doesn't seem to state any options other than the default 0x12.

SX127x sync word shouldn't be set to 0x14 in the example - probably the intention was to set to 0x34, same as SX126x. I'll fix that as well.

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.

2 participants