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

add support for Pi Pico + sx1262 module to LoRaWAN examples #1086

Closed
wants to merge 7 commits into from

Conversation

K4KDR
Copy link

@K4KDR K4KDR commented May 4, 2024

Hello! After finding success w/ the LoRaWAN examples using my Arduino UNO + sx1278 hardware, I wanted to be able to utilize the excellent Waveshare SX1262 LoRa Node Module for Raspberry Pi Pico (https://www.waveshare.com/pico-lora-sx1262-868m.htm) that is a direct header plug-in to the Pi Pico microcontroller.

pico+sx1262

A previous issue (#729) identified the syntax required to use the sx1262 module.

After modifying my local code to successfully uplink a LoRaWAN payload using the Pi Pico / sx1262 combination, I wanted to share the code if by chance you would like to integrate this hardware option into the list of choices in the LoRaWAN examples.

Thanks!

-Scott, K4KDR

K4KDR added 7 commits May 4, 2024 17:28
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
add support for Pi_Pico_1262_LoRa module (fix from from jgromes#729).    LoRa hardware is -->  https://www.waveshare.com/pico-lora-sx1262-868m.htm  (choose appropriate model for your regional frequency bandplan)
@HeadBoffin
Copy link
Collaborator

I've not put comments on the other files, but they translate over.

@HeadBoffin
Copy link
Collaborator

Sorry, rude of me not to say, thanks for this, very useful consolidation of the prior issues.

Copy link
Owner

@jgromes jgromes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution Scott! However, I'm asking myself whether we need to be adding more example LoRa boards to the LoRaWAN examples ... surely there are still many more we are missing and new ones will be released in the future. By including these, we are basically building a database of pin maps. Fair enough, but why should that be in the RadioLib examples - and copied three times over?

My point is, this seems very similar to what @Velocet proposed in #1065. So maybe instead of kicking the can down the road we should move these pin maps somewhere else (e.g. a new repository in https://github.com/radiolib-org)?

@@ -3,6 +3,8 @@

#include <RadioLib.h>

#define Pi_Pico_1262_LoRa // set to match hardware in-use (see pin maps below!)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this - so far, these examples worked by automatically detecting the type of board. This changes that behavior. Is there any macro that is defined for that board that we could use?

Comment on lines +54 to +55
// fix from https://github.com/jgromes/RadioLib/issues/729
SX1262 radio = new Module(3, 20, 15, 2, SPI1, RADIOLIB_DEFAULT_SPI_SETTINGS);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it can be called a "fix" when you are using the library as intended - what exactly is being fixed here?

Having to specify which SPI bus is used to connect to the module is a very basic information that even has its own section on the Wiki and in the troubleshooting guide. It's not something I want to hide from the user, if they leave everything at the default value and then insert surprised Pikachu face when it doesn't work then that suggests they know very little about the hardware they are using.

@K4KDR
Copy link
Author

K4KDR commented May 5, 2024

Great points - I learn so much every time you answer a question or add code to a project... THANK YOU!

One example is that I had no idea that these examples were doing any kind of hardware auto-detect! When I saw all the #if defined statements, I just assumed that the code was expecting the user to "just know" that the correct way to utilize that was to add the appropriate #define statement for their hardware. There is line just before that section that mentions Auto select MCU <-> radio connections , but that meant nothing to me since I had never heard of any such thing.

Once I found the correct syntax to use the Pi Pico + sx1262 combination, I just wanted to share that for your review. But it makes complete sense to leave references to work-arounds for specific hardware to Issues or Discussions in the repository where they can easily be found by keyword searches.

Thanks for taking the time to look at what I am doing and providing YET MORE learnings. Much appreciated!

@HeadBoffin
Copy link
Collaborator

Once I found the correct syntax to use the Pi Pico + sx1262 combination, I just wanted to share that for your review. But it makes complete sense to leave references to work-arounds for specific hardware to Issues or Discussions in the repository where they can easily be found by keyword searches.

There will be a Pico worked example, it's just that someone else also submitted one at the same time and we need to get the auto-pinmap thing figured. So you work will emerge as a full example, just not today!

@StevenCellist
Copy link
Collaborator

@K4KDR while this PR is pending a pin-table or other way to get the pinmaps in a better state than keeping multiple config.h files synchronized, I already want to thank you for your code here to get the Pico + SX1262 hat going for radiolib-org/radiolib-persistence#4. Thanks! :)

@jgromes
Copy link
Owner

jgromes commented May 28, 2024

@K4KDR sorry for keeping you waiting on this one - it's because I'm not entirely sure we want to keep the pin maps in the LoRaWAN examples, or rather move them to some other place. There's a discussion dedicated to this topic in #1106.

@jgromes
Copy link
Owner

jgromes commented Jun 3, 2024

Closing as radiolib-org/RadioBoards#2 was merged.

@jgromes jgromes closed this Jun 3, 2024
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.

None yet

4 participants