jgromes / RadioLib

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

Pico example will not run when flashed #1191

Closed gherlein closed 1 week ago

gherlein commented 3 weeks ago

While building the NonArduino/Pico example, I see this:

/home/gherlein/src/pico-projects/forks/RadioLib/src/Module.cpp: In member function 'void Module::SPIreadRegisterBurst(uint32_t, size_t, uint8_t*)':
/home/gherlein/src/pico-projects/forks/RadioLib/src/Module.cpp:116:19: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
  116 |       *(cmdPtr++) = (this->spiConfig.cmds[RADIOLIB_MODULE_SPI_COMMAND_READ] >> 8*i) & 0xFF;
      |       ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Describe the bug

Pico example builds but will not run. I will have time later today or tomorrow to see if I can chase this to ground.

Added note:
The Pico SDK says you need to initialize stdio like this: stdio_init_all(); but the code does not have that.

To Reproduce

Build the example code and install it on a Pico. Nothing happens.

I added the stdio_init_all(); to see if it was just missing stdio connection. No joy. I also added some code to just blink the LED to even see if it was running at all (code attached) and running that resulted in no blinks. #ifdef'd the radio parts to see if my assumptions were right. You do need the stdio_init_all(); and the LED blinks as expected.

So the problem is definitely in the radio code causing it to not run.

/*
  RadioLib Non-Arduino Raspberry Pi Pico library example

  Licensed under the MIT License

  Copyright (c) 2024 Cameron Goddard

  Permission is hereby granted, free of charge, to any person obtaining a copy
  of this software and associated documentation files (the "Software"), to deal
  in the Software without restriction, including without limitation the rights
  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  copies of the Software, and to permit persons to whom the Software is
  furnished to do so, subject to the following conditions:

  The above copyright notice and this permission notice shall be included in all
  copies or substantial portions of the Software.

  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  SOFTWARE.
*/

// define pins to be used
#define SPI_PORT spi0
#define SPI_MISO 4
#define SPI_MOSI 3
#define SPI_SCK 2

#define RFM_NSS 26
#define RFM_RST 22
#define RFM_DIO0 14
#define RFM_DIO1 15

#include <pico/stdlib.h>

// include the library
#include <RadioLib.h>

// include the hardware abstraction layer
#include "PicoHal.h"

// create a new instance of the HAL class
PicoHal* hal = new PicoHal(SPI_PORT, SPI_MISO, SPI_MOSI, SPI_SCK);

// now we can create the radio module
// NSS pin:  26
// DIO0 pin:  14
// RESET pin:  22
// DIO1 pin:  15
SX1276 radio = new Module(hal, RFM_NSS, RFM_DIO0, RFM_RST, RFM_DIO1);
const uint LED_PIN = 25;

int main() {

  stdio_init_all();
  gpio_init(LED_PIN);
  gpio_set_dir(LED_PIN, GPIO_OUT);

#define RADIO
#ifdef RADIO  
  // initialize just like with Arduino
  printf("[SX1276] Initializing ... ");
  int state = radio.begin();
  if (state != RADIOLIB_ERR_NONE) {
    printf("failed, code %d\n", state);
    return(1);
  }
  printf("success!\n");
#endif

  // loop forever
  for(;;) {
#ifdef RADIO
    // send a packet
    printf("[SX1276] Transmitting packet ... ");
    state = radio.transmit("Hello World!");
    if(state == RADIOLIB_ERR_NONE) {
      // the packet was successfully transmitted
      printf("success!\n");

      // wait for a second before transmitting again
      hal->delay(1000);

    } else {
      printf("failed, code %d\n", state);
    }
#endif
    gpio_put(LED_PIN, 0);
    sleep_ms(250);
    gpio_put(LED_PIN, 1);
    puts("Hello World\n");
    sleep_ms(1000);

  }

  return(0);
}

Expected behavior

Program will run.

Additional info (please complete):

Stock RPi Pico board, radio module is SX1262-based: https://www.amazon.com/gp/product/B09LV2W64R/ref=ppx_yo_dt_b_search_asin_title?ie=UTF8&psc=1

StevenCellist commented 3 weeks ago

The first thing that I see, is that your code uses SX1276 radio while you say yourself that you use a SX1262.. so changing that will definitely help!

Also, you may profit from enabling the DEBUG_BASIC flag in BuildOptUser.h to see additional output regarding an issue like this.

gherlein commented 3 weeks ago

I should have said the code will not run even if the radio is unpowered, so theoretically it should have just errored out.

Yes, I hadn't caught that the example is for a different radio. Would not have worked anyway.

StevenCellist commented 3 weeks ago

Well, with your addition of stdio_init_all() the LED blinks, so there may be a chance that the radio will initialize of the correct one is selected. But again, the debug flags may show additional information leading up to the point of failure, so please check those out (BASIC, and if that doesn't help any, add the SPI flag too).

gherlein commented 3 weeks ago

I changed the radio to SX1262 like this:

SX1262 radio = new Module(hal, RFM_NSS, RFM_DIO0, RFM_RST, RFM_DIO1);

I set the debug flag as requested. It appears there is an Arduino-ism if I try to do that:

/home/gherlein/src/pico-projects/forks/RadioLib/src/Module.cpp: In member function 'void Module::init()': /home/gherlein/src/pico-projects/forks/RadioLib/src/BuildOpt.h:32:33: error: 'Serial' was not declared in this scope 32 | #define RADIOLIB_DEBUG_PORT Serial | ^~

So I set it to stdout.

I don't think it will help, since nothing comes across the serial port at all.

When compiling I still get this

In member function 'void Module::SPIreadRegisterBurst(uint32_t, size_t, uint8_t)', inlined from 'void Module::SPIreadRegisterBurst(uint32_t, size_t, uint8_t)' at /home/gherlein/src/pico-projects/forks/RadioLib/src/Module.cpp:109:6: /home/gherlein/src/pico-projects/forks/RadioLib/src/Module.cpp:116:19: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 116 | (cmdPtr++) = (this->spiConfig.cmds[RADIOLIB_MODULE_SPI_COMMAND_READ] >> 8i) & 0xFF; | ~~^~~~~~~~~~~~~~~~~~

warning: writing 1 byte into a region of size 0

That does not look good. I'll poke at this if I get time.

jgromes commented 3 weeks ago

@gherlein

The Pico SDK says you need to initialize stdio like this: stdio_init_all(); but the code does not have that.

It does - it's in the PicoHal::init() method, where it belongs. When you removed the radio parts, you have also removed tha call to that init method, and hence, the stdio init.

That does not look good. I'll poke at this if I get time.

I think that warning is a false positive because the gcc version you're using cannot determine the size of memory cmdPtr points to (6 bytes). No other tools we use for static analysis reports this sort of issue and neither do other versions of gcc even with -Wall and -Wextra enabled. But if you find a different way to implement this without that warning I would be happy for a PR ;)

However, I don't think that's your problem. If it were, I would expect the program to crash in the radio.begin() method, since the code in question handles SPI transactions. Instead, your program does not seem to print even the starting "[SX1276] Initializing ... " text.

I don't have any experience with the RPi Pico board, the example was contributed (see git blame), however, one thing that strikes me as a bit strange looking at it is that the main function calls return with exit code as opposed to an infinite loop I would expect on a microcontroller. IS that something specific to RPi Pico?

kroghm96 commented 1 week ago

Hi @gherlein, I'm having the same issue. I've scoped my issue to be in: state = this->phyLayer->checkDataRate(*dataRate); inside the LoRaWAN.cpp file. But don't really know how to fix it. Have you figured out a solution?

gherlein commented 1 week ago

Hi @gherlein, I'm having the same issue. I've scoped my issue to be in: state = this->phyLayer->checkDataRate(*dataRate); inside the LoRaWAN.cpp file. But don't really know how to fix it. Have you figured out a solution?

No, I pivoted to trying to grok what the radio really needed and worked on https://github.com/gherlein/pico-pong and also trying a base library Semtech published in 2017. No joy. It was frustrating so I moved on to playing with some other radios. I do think RadioLib is the base I would go back to when I start trying to use the sx1262 again. HOWEVER, after bloodying my head on this I might just give in and buy some sz1276-based radios instead. Seems like WAY MORE folks are using those even though they are only 100 mbps and 10km range (verses 300 mbps and 15km range). If the drivers "just work" then I can totally see why folks would prefer that radio.

As was commented above, the example has some things about it that make me wonder if it ever really worked (like returning instead of an infinite loop).

gherlein commented 1 week ago

For example, if one chooses the sx1276 radio there is this example https://github.com/ArmDeveloperEcosystem/lorawan-library-for-pico that's only 3 years old and it comes from ARM. When I play with LoRa again I'm likely to test that too. But I was hoping to use RadioLib for it's portability to Arduino etc.

kroghm96 commented 1 week ago

Ahh, yeah I'm using the sx1276, so can't relate to the added struggle of the sx1262. I just got my sx1276 working with RadioLib and PicoHal. Found out my wiring was wrong, but I imagine you already wired it correctly so don't know how much that helps.

jgromes commented 1 week ago

@gherlein if you no longer pursue this then it would have been fair to let us know so that we can close this issue - I will do that now.

For what it's worth, Pi Pico has been used and reported to be working in several issues and discussions here, so I have to presume the problem is in the hardware or usage.

gherlein commented 1 week ago

I was planning on taking one more swing at this later today with a fresh mind, but no worries to close it. I think my choice of radio was poor.