0
\$\begingroup\$

I have an Atmega328p MCU which is connected to USB port by ch340 chip and I've connected it to GSM module by hardware RX/TX/GND pins. I don't send any command to USB port all commands are send by micro controller.

So I've found this tutorial which has a pretty simple functions to receive USART_RXD interrupt from RX pin. I've break it down into these two functions, one for copying received data from RX to a variable:

void copy_command ()
{
    // The USART might interrupt this - don't let that happen!
    ATOMIC_BLOCK(ATOMIC_FORCEON) {
        // Copy the contents of data_in into command_in
        memcpy(command_in, data_in, 8);

        // Now clear data_in, the USART can reuse it now
        memset(data_in[0], 0, 8);
    }
}

and a function for interrupt handling:

ISR (USART_RX_vect)
{
    // Get data from the USART in register
    data_in[data_count] = UDR0;

    // End of line!
    if (data_in[data_count] == '\n') {
        command_ready = TRUE;
        // Reset to 0, ready to go again
        data_count = 0;
    } else {
        data_count++;
    }
}

The problem is, even I've changed data_in and command_in variable size to 32 character and changed this line memcpy(command_in, data_in, 32); and removed memset(data_in[0], 0, 8);, the output is some how unacceptable.

for example when the module should send RING in terminal it return NG or when I do sizeof command_in it always return 8 even there is nothing with size of 8 in my code. I assume 8 is the size of one character in byte, but isn't command_in suppose to get a string including all RX characters?

How can I fix this to receive full RX response to process?

\$\endgroup\$
3
  • \$\begingroup\$ You must not connect more than one driver to a line; if you have the modem connected to the ATmega's single serial port, then you can connect at most the CH340's receive line to monitor things. You must not also connect its transmit! Typically ATmega328 mobile modem projects end up having to use a software emulated serial port if they want to keep the debug serial port to a development PC... \$\endgroup\$ – Chris Stratton Oct 2 '20 at 16:07
  • \$\begingroup\$ Realistically, fix the hardware, then try some valid Arduino-realm example like the adafruit fona demo. Once you've proven the setup works, you can move beyond Arduino to bare metal code, though beware this kind of thing is very tricky to get right, especially if one is not already deeply familiar with interrupt consistency issues, c-style strings/buffers, etc... \$\endgroup\$ – Chris Stratton Oct 2 '20 at 16:10
  • \$\begingroup\$ I agree with Justme, this is a question about how to program in C \$\endgroup\$ – user253751 Oct 2 '20 at 16:47
0
\$\begingroup\$

This is more a how to program in C than an electronics question, even if programming happens on an AVR.

Sizeof returns size of the array, it does not know the contents of it, so full array is copied from source to destination. If you want to know the length of the string, use strlen (but as it works with null-terminated strings, you have to null terminate the string yourself, by adding the extra null character), or even better, since you already keep count of received char index, it also is the count of chars received, so store that into command_ready variable to know how much is received.

The other problem is, that if there is a transmission that continues directly after the '\n' character, it overwrites the first letter of just received command. That's why a ring buffer should be used, or dual buffer, or stop reception until command executed, to guarantee that an extra byte does not overwrite first character.

One further problem is the line memset(data_in[0], 0, 8); which most likely is not what you want. It does not clear the data_in array. It takes the first character data from data_in[0], and then uses that data as an address from where to write eight zeroes. Compiler should warn about using an integer as pointer without a cast, as now you are basically zeroing random memory addresses based on what data was received.

So basically, the tutorial has issues and should be taken as a tutorial, not as a working example.

\$\endgroup\$
1
  • \$\begingroup\$ strlen() uses NULL termination for strings. The OP is using a newline. Might want to address that. \$\endgroup\$ – Aaron Oct 2 '20 at 16:07
0
\$\begingroup\$

Please note that the code in this post is kind of pseudo code, regard it as such.

That tutorial is questionable in many ways. In particular, one should not naively use memcpy and memset because they will needlessly destroy everything that is performance. And since they are called from inside an atomic lock to prevent against race conditions, they prevent interrupts from adding new data and stalls everything going on in the MCU in general.

Instead of memcpy, you should declare two RAM buffers ("double buffering"):

volatile static uint8_t* data1 [SIZE];
volatile static uint8_t* data2 [SIZE];

where SIZE is whatever's appropriate in your case (8 or 32). Then create pointers and a flag:

volatile static uint8_t* incoming_buf;
volatile static uint8_t* complete_buf;
volatile static bool     data_available;

...
incoming_buf = data1;
complete_buf = data2;
data_available = false;

Your ISR always uses the incoming_buf pointer:

ISR (USART_RX_vect)
{
   if(data_available) 
   {
     // defensive programming, this should never happen if real-time calculations are correct 
     // (the main program is fast enough to process incoming data)
     return ;
   }

   incoming_buf[data_count] = UDR0;
   ...
   if (incoming_buf[data_count] == '\n') 
   {
     data_available = true;
     data_count = 0;
     ...
   }

And then the atomic access only swaps the two pointers:

if(data_available)
{
  ATOMIC_BLOCK(ATOMIC_FORCEON) 
  {
    volatile static uint8_t* tmp = incoming_buf;
    incoming_buf = complete_buf;
    complete_buf = tmp;
  }
}
data_available = false;

Now, assuming your main program is fast enough to handle the data before the next UART buffer is filled up, you can just use complete_buf in the main program, without worrying about the ISR overwriting anything. No need for memcpy or pointless memset to zero.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.