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

[LoRaWAN] Move TX power logic to function #884

Merged
merged 2 commits into from
Nov 27, 2023
Merged

[LoRaWAN] Move TX power logic to function #884

merged 2 commits into from
Nov 27, 2023

Conversation

StevenCellist
Copy link
Collaborator

Some applications might want to alter the Tx power, e.g. for testing LoRaWAN network coverage. Also, this moves the setOutputPower logic to one single function for better readability.

@jgromes
Copy link
Owner

jgromes commented Nov 27, 2023

Thanks, this looks good. Just one point, would it be possible to change the method name to setOutputPower? That's used for the transceivers as well.

@StevenCellist
Copy link
Collaborator Author

Understandable, but within the LoRaWAN scheme it's specified as Tx Power and would prevent some mistakes between function calls.
But completely your call, it's not much of a difference. Let me know :)

@jgromes
Copy link
Owner

jgromes commented Nov 27, 2023

Hmm, that's actually a good point - event the transmit/receive methods are called uplink and downlink, so let's keep the current name. Though in that case, setTxPower is missing from keywords (keep in mind the files uses true tabs, not spaces).

@StevenCellist
Copy link
Collaborator Author

Ah, that was the problem.. got it and done

@jgromes jgromes merged commit b2b73ab into jgromes:master Nov 27, 2023
29 checks passed
@jgromes
Copy link
Owner

jgromes commented Nov 27, 2023

Perfect, thank you very much!

BTW the only reason I have not yet released a new update with your LoRaWAN changes is that the STM32 framework for Arduino is broken for the STM32WL series. There should be an update coming soon.

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