1

I'm making a remote controlled machine using a pi pico to drive the motors and read some sensors, and a raspberry pi 4 to send commands to the pi pico via serial and host the web interface.

I'm working on sending and receiving commands from the raspberry and for now I'm stuck with this code:

#include <string.h>

#include "pico/stdlib.h"
#include "hardware/uart.h"
#include "hardware/irq.h"

#define UART_ID uart0
#define BAUD_RATE 19200
#define DATA_BITS 8
#define STOP_BITS 1
#define PARITY    UART_PARITY_NONE
#define UART_TX_PIN 0
#define UART_RX_PIN 1

static int chars_rxed = 0;

char uCommand[32] = {0, 0};

void on_uart_rx() {
    char tmp_string[] = {0, 0};
    while (uart_is_readable(UART_ID)) {
        uint8_t ch = uart_getc(UART_ID);
        tmp_string[0] = ch;
        strcat(uCommand, tmp_string);
        if(uart_is_writable(UART_ID)){
          uart_putc(UART_ID, '-');
          uart_puts(UART_ID, uCommand);
          uart_putc(UART_ID, '-');
        }
        chars_rxed++;
    }
}

int main(){

  uart_init(UART_ID, BAUD_RATE);

  gpio_set_function(UART_TX_PIN, GPIO_FUNC_UART);
  gpio_set_function(UART_RX_PIN, GPIO_FUNC_UART);
  uart_set_hw_flow(UART_ID, false, false);
  uart_set_format(UART_ID, DATA_BITS, STOP_BITS, PARITY);
  uart_set_fifo_enabled(UART_ID, false);

  int UART_IRQ = UART_ID == uart0 ? UART0_IRQ : UART1_IRQ;

  irq_set_exclusive_handler(UART_IRQ, on_uart_rx);
  irq_set_enabled(UART_IRQ, true);
  uart_set_irq_enables(UART_ID, true, false);

  uart_puts(UART_ID, "\nOK\n");
    while (1){
        tight_loop_contents();
        if(uCommand[0] != 0){
          uart_putc(UART_ID, '/');
          uart_puts(UART_ID, uCommand);
          uart_putc(UART_ID, '/');
        }
      }
}

my idea was to take the command sent via serial during the interrupt and place it in a charset, then parse it and execute it externally.

Trying it, I notice that it never enters the if inside the while and it doesn't 'fill' the 'uCommand' charset completely but only a few characters compared to the ones sent.

I hope my question is not off topic.

lasb3tas
  • 75
  • 1
  • 9
  • The function `uart_getc` is a blocking function. This means it will not exit the function has received a character. Are you sure you are sending data? It is also better to send the number of bytes you expect to receive. I've made an example in which an STM32 chip communicates over UART with a python script. You could use that as an example for your situation. https://embeddedproto.com/a-simple-uart-example-with-embedded-proto/ – Bart Aug 11 '21 at 11:10
  • Thank you for your comment Bart. Yes, I'm using Arduino serial prompt for sending and visualize the uart. – lasb3tas Aug 11 '21 at 11:36
  • In that case maybe first write a version which works with blocking uart commands only and than when that works (aka all hardware and wires are connected correct) start with callbacks. – Bart Aug 11 '21 at 11:38
  • Actually, the hardware works. I tried with the UART advanced example and everything is fine. Can't understand why it does not work now, it's weird. – lasb3tas Aug 11 '21 at 11:41
  • Welcome to the world of embedded programming ;-) Do you have a debugger running? If so, send just one character from your arduino, nothing else and wait till you see it come by on the pico pi. – Bart Aug 11 '21 at 11:49
  • I don't have any debugger running. Tried to send a single character, got '-A-' as output also tried to send 3 characters, (ABC) got -AA-. – lasb3tas Aug 11 '21 at 12:00
  • Try to get a debugger running as you can inspect the variables and see where something goes wrong – Bart Aug 11 '21 at 12:10
  • I should be able to use an stm st-link, Am I right? – lasb3tas Aug 11 '21 at 12:16
  • That I do not know – Bart Aug 11 '21 at 12:23
  • I think ST-Links are locked to ST parts - maybe it is possible to work around that. See https://electronics.stackexchange.com/questions/250664/can-i-use-st-link-programmer-for-non-st-chips – Clifford Aug 11 '21 at 16:12
  • You should declare `uCommand` (and any other shared objects) `volatile` at least. The while-loop is waiting for `uCommand[0] != 0` to become true, but the `main` thread does not modify `uCommand[0]` so the compiler is free to "optimise" away the entire block of code if it "knows" it can never be `true`. Similarly `chars_rxed` may get optimised away because you write but never read it. Declaring that `volatile` will also prevent that. If you are not compiling with a `-O` *n* level or with `-O0`, then that is not your problem - though you should do it all the same before it _becomes_ one. – Clifford Aug 11 '21 at 16:17
  • In your update you say that the code appears to work. However don't make is work so hard - tell me how that code differs from the on-working code? – Clifford Aug 11 '21 at 16:36
  • ... Ok I took the trouble to compare the files - I see you added `volatile` - so that explains why it works. Why the code fails when you remove the `uart_is_writable()` is therefore a _different question_. You should remove it so that someone can answer the original question. Or remove the original question since you have already solved it. – Clifford Aug 11 '21 at 16:44
  • Note also that `strcat()` is a bad idea at the best of times and especially so in an interrupt handler. See https://www.joelonsoftware.com/2001/12/11/back-to-basics/. To make your ISR deterministic and efficient keep a track of the end of the buffer and write the data directly to that. `strcat()` has to find the end of the string every time, with increasing time on each interrupt. – Clifford Aug 11 '21 at 16:49
  • Also do not `memset()` the buffer in the `main()` thread. If serial data arrives while you are doing that you have a race condition. Better to set a flag to indicate a buffer reset and let the ISR start from the beginning if the flag is set (and clears the flag). So that the buffer is written in only one thread context. – Clifford Aug 11 '21 at 16:53

1 Answers1

1

You should declare uCommand (and any other shared objects) volatile.

The while-loop is waiting for uCommand[0] != 0 to become true, but the main thread does not modify uCommand[0] so the compiler is free to "optimise" away the entire block of code if it "knows" it can never be true.

Similarly chars_rxed may get optimised away because you write but never read it. Declaring it volatile will also prevent that.

The optimisaton of apparently redundant accesses usually only occurs when -O1 or higher optimisation level is set. Though you should in any event use volatile in such circumstances regardless.

Clifford
  • 88,407
  • 13
  • 85
  • 165