Program flow stopping from some odd interrupts.

Go To Last Post
27 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi all, 

I have some code that is only utilizing UART0 and UART1 on a atmega644pa.

I am sending commands to a cellular modem and utilizing an interrupt and ring buffer to contain the responses of the modem.

UART0 would be my debug port to my laptop and UART1 goes to the cellular modem.

 

My code was long and unoptimized because I noticed that bytes got dropped on the receive UART1 port of the AVR.

 

For example "OK" came out as "O" or "K" every once in awhile.

Additionally after N amount of commands to the modem the AVR "breaks" and freezes in its program flow or tries to RESET infinitely.

 

After painstakingly searching for the error I determined it was other things (a better word instead?) in my code causing the UART1_RX interrupt to slow down for the bytes to be dropping.

after deleting code snippets to determine the errors, I found that without some snippets the bytes stopped dropping.

 

But the "freezing" and "infinite resets" remained. The code starts with some initialization of pins and then goes into a loop of sending the same 5 commands to the modem. Eventually the AVR breaks.
Below is some code snippets of the ring buffer and loop of the commands being sent to the modem.

(I am pretty sure I setup my UARTs correctly so I don't want to burden this thread with tons of needless code)

Like I said, the commands are sent and their responses are sent back fine but somewhere in there, after iteration 7 there's a chance for an interrupt to break the AVR.

This doesn't present itself when the modem is connected to the AVR. It is only connected on the UART1 pins and some GPIO pins for turning it on/off

 

//ring buffer
#define BUFFER_SIZE 512
volatile int rxn=0;
volatile char rx[BUFFER_SIZE];
volatile uint8_t rxFlag = 0;

ISR(USART1_RX_vect){
  while (!(UCSR1A & (1<<RXC1)));
  if (rxn==BUFFER_SIZE)
    rxn=0;
  rx[rxn++] = UDR1; // increment rxn and return new value.
  rxFlag=1; // flag the receipt of data.
}

void ring_flush(){
  for(int i=0; i<BUFFER_SIZE; i++){
    rx[i]='\0';
  }
}

//MODEM functions:

void sendModem2(const char *buf, uint8_t p, uint16_t delayMs, uint8_t echo){
      int counter, counter2;
      counter=rxn;
      if((PINB & (1<<PINB0))==0){// read cts if we are good to send data
        PORTB &= ~(1 << PINB1);  //LOW RTS
        transmit_string_USART1(buf,p,false);
      }
      else{
        transmit_string_USART0(PSTR("Not sent"), true ,true);  // if modems cts is closed
      }
      DDRB &= ~(1 << PINB1); // RTS Input
      _delay_ms(delayMs);    // wait for modem to receive command
      counter2 = rxn;

//get response between counter and counter2
      char str[1024]={'\0'};
        for (int i=counter ; i!=(counter2) ; i=(i+1)%BUFFER_SIZE){
          char cAsStr[] = { rx[i], '\0' };
          strcat(str,cAsStr);
        //transmit_USART0(rx[i]);
        }
//print it to USART0
      if(echo){
        transmit_string_USART0(buf,p,true);
        for (int i=0; i<1024; i++){
          transmit_USART0(str[i]);
      }
//flush ring buffer
      ring_flush();
      }
}

void init(){
  init_USART0();
  init_USART1();
  sei();
  if(MCUSR & (1<<PORF )) transmit_string_USART0(PSTR("\nPower-on reset!\n"), true, true);
  if(MCUSR & (1<<EXTRF)) transmit_string_USART0(PSTR("\nExternal reset!\n"), true, true);
  if(MCUSR & (1<<BORF )) transmit_string_USART0(PSTR("\nBrownout reset!\n"), true, true);
  if(MCUSR & (1<<WDRF )) transmit_string_USART0(PSTR("\nWatchdog reset!\n"), true, true);
  if(MCUSR & (1<<JTRF )) transmit_string_USART0(PSTR("\nJTAG reset!\n"), true, true);

  MCUSR = 0;
  DDRD |= 1 << PIND4;
  DDRD |= 1 << PIND6;
  DDRB |= 1 << PINB3;
  DDRB |= 1 << PINB4;
  DDRB &= ~(1 << PINB0);
  DDRC |= 1 << PINC2;
  DDRC |= 1 << PINC3;
  PORTC &= ~(1 << PINC3);     //LOW
  PORTC |= 1 << PINC2;        //HIGH
 }

//Main function
int main(){
  while (1) {
    init();
    PORTB |= 1 << PINB3;                    //-> turns ON Modem IO_REF
    PORTB |= 1 << PINB4;                    //->makes Modem_ON/OFF pin connected to ground momentarily
    _delay_ms(1000);                        //-> makes Modem_ON/OFF pin floating
    PORTB &= ~(1 << PINB4);
	transmit_string_USART0(PSTR("Starting Cellular Modem (30sec)"), true, true);
	  for(int i =0; i<30; i++){  // Wait > 30 seconds for initialization*/
	    _delay_ms(1000);
	  }

	 int i =0;
	 while(1){ // AVR FREEZES HERE at i > 7 (not limited at 7)
		  printNum_USART0(i,0,true);
		  sendModem2(PSTR("ATE0\r"),true,500,false);
		  sendModem2(PSTR("AT\r"),true,500,true);
		  sendModem2(PSTR("AT\\Q3\r"),true,500,true);
		  sendModem2(PSTR("AT+CMEE=2\r"),true,500,true);
		  sendModem2(PSTR("AT+CMEE=2\r"),true,500,true);
		  sendModem2(PSTR("AT+CESQ\r"),true,500,true);
	  i++;
	}
  }
  return(0);
}

The output of the code is :

External reset!

Starting Cellular Modem (30sec)
0
AT

OK
AT\Q3

OK
AT+CMEE=2

OK
AT+CMEE=2

OK
AT+CESQ: 99,99,255,255,19,50

OK
1
AT

OK
......

......
7
AT

OK
AT\Q3

OK
AT+CMEE=2

OK
26
26
26
26
26
26
26
...
26
24
24
24
24
...
24
24
24
24

Watchdog reset!

Watchdog reset!

Watchdog reset!

......

The last parts with the 26 and 24 is the number of the Interrupt that triggered. I have included the ISRs that I used below to find those.

24 is the ANALOG_COMP interrupt

26 is the EE_READY interrupt

and then the avr proceeds to keep resetting by executing the main code over and over again.

 

 

Here are the ISRS:

 

//General ISRS

ISR(BADISR_vect){
	for (;;) UDR0='!';
}

ISR(WDT_vect) { //Watchdog Interrupt Service. This is executed when watchdog timed out
	printNum_USART0(9,3,true);
}

ISR(RESET_vect) {
	printNum_USART0(1,3,true);
}

ISR(INT0_vect) {
	printNum_USART0(2,3,true);
}

ISR(INT1_vect) {
	printNum_USART0(3,3,true);
}

ISR(INT2_vect) {
	printNum_USART0(4,3,true);
}

ISR(PCINT0_vect) {
	printNum_USART0(5,3,true);
}

ISR(PCINT1_vect) {
	printNum_USART0(6,3,true);
}

 ISR(PCINT2_vect) {
	printNum_USART0(7,3,true);
}

ISR(PCINT3_vect) {
	printNum_USART0(8,3,true);
}

ISR(TIMER2_COMPA_vect) {
	printNum_USART0(10,3,true);
}

ISR(TIMER2_COMPB_vect) {
	printNum_USART0(11,3,true);
}

ISR(TIMER2_OVF_vect) {
	printNum_USART0(12,3,true);
}

ISR(TIMER1_CAPT_vect) {
	printNum_USART0(13,3,true);
}

ISR(TIMER1_COMPA_vect) {
	printNum_USART0(14,3,true);
}

ISR(TIMER1_COMPB_vect) {
	printNum_USART0(15,3,true);
}

ISR(TIMER1_OVF_vect) {
	printNum_USART0(16,3,true);
}

ISR(TIMER0_COMPA_vect) {
	printNum_USART0(17,3,true);
}

ISR(TIMER0_COMPB_vect) {
	printNum_USART0(18,3,true);
}

ISR(TIMER0_OVF_vect) {
	printNum_USART0(19,3,true);
}

ISR(SPI_STC_vect) {
	printNum_USART0(20,3,true);
}

ISR(USART0_RX_vect) {
	printNum_USART0(21,3,true);
}

ISR(USART0_UDRE_vect) {
	printNum_USART0(22,3,true);
}

// ISR(USART0_TX_vect) {
// }

ISR(ANALOG_COMP_vect) {
	printNum_USART0(24,3,true);
}

ISR(ADC_vect) {
	printNum_USART0(25,3,true);
}

ISR(EE_READY_vect) {
	printNum_USART0(26,3,true);
}

ISR(TWI_vect) {
	printNum_USART0(27,3,true);
}

ISR(SPM_READY_vect) {
	printNum_USART0(28,3,true);
}

ISR(USART1_UDRE_vect) {
	printNum_USART0(30,3,true);
}

Can someone tell me what the heck is happening here? When I remove the modem from the UART the freezing and resetting error never presents itself.

Last Edited: Wed. Jan 29, 2020 - 09:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Serial input lines idle high, so if left unconnected will float and may be the source of your mystery, place a 10k pull up or enable internal pull up on RX0 pin and see if that helps.

Jim

 

 

(Possum Lodge oath) Quando omni flunkus, moritati.

"I thought growing old would take longer"

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, so your program has problems with USART0 and yor show everything but the code where you init  USART0???

Also you seem to be in love with "|=" when initializing register is a BAD practice, best to just set a register (DDRn, PORTn, USART's ect) with a simple reg = value;

This way all bits are set as you expect without other side effects and unknown settings. 

What AVR are you using, at what xtal frequency?

 

Jim

 

 

(Possum Lodge oath) Quando omni flunkus, moritati.

"I thought growing old would take longer"

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What is the purpose of while (!(UCSR1A & (1<<RXC1))); inside ISR(USART1_RX_vect){ ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why do you test RXC in the receive isr? The fact you got an interrupt tells you this already.

Also your index management is suspect. Increment the index then test for wrap around - not the other way. The index should always have a valid index to the array.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am not understanding. The Serial UART1 line is always connected to the modem when the error occurs.

 

Do you mean by setting the  UART1 RX pin to input prior to UART intialization?

DDRD &= ~(1 << PIND2); // RX1 Input

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well my program is having trouble with something! USART1 is what is talking to the modem.

Like I said I assumed I set the UARTS up right but here is the uart intialization

#include <avr/io.h> 
#include <avr/sleep.h>
#include <avr/wdt.h>
#include <avr/power.h>
#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <util/delay.h>
#include <stdlib.h>
#include <inttypes.h>
#include <math.h>
#include <string.h>

//UART
#define BAUD0 115200
#define BAUD1 115200

#define UBRR0_VALUE0 (((F_CPU) + 8UL * (BAUD0)) / (16UL * (BAUD0)) -1UL)
#define UBRR0_VALUE1 (((F_CPU) + 4UL * (BAUD1)) / (8UL * (BAUD1)) -1UL)
#define UBRR1_VALUE0 (((F_CPU) + 8UL * (BAUD1)) / (16UL * (BAUD1)) -1UL)
#define UBRR1_VALUE1 (((F_CPU) + 4UL * (BAUD1)) / (8UL * (BAUD1)) -1UL)


//USART0 and USART1 functions
void init_USART0() {
  //unsigned int number = F_CPU/16/BAUD0 - 1;
  UBRR0H = (unsigned char)(UBRR0_VALUE1>>8);
  UBRR0L = (unsigned char)UBRR0_VALUE1;
  UCSR0B = (1<<RXEN0)|(1<<TXEN0)|(1 << RXCIE0);// Enable receiver and transmitter 
  UCSR0C = (1<<USBS0)|(3<<UCSZ00);// Set frame format: 8data, 2stop bit 
  UCSR0A |= (1<<U2X0);
}

void transmit_USART0(char c) { 
  while (!(UCSR0A & (1<<UDRE0))); //Wait for empty transmit buffer 
  UDR0 = c;
}

void transmit_string_USART0(const char *buf, uint8_t p, uint8_t ln){
  if(p){
    while (pgm_read_byte(buf) != 0x00){
      transmit_USART0(pgm_read_byte(buf++));
    }
  }
  else{
    while (*buf){
      transmit_USART0(*buf++);
    }
  }
  if (ln) transmit_USART0('\n');
}

void printNum_USART0(float data, int presc, uint8_t ln){
  char buffer [10];
  dtostrf(data,presc+2,presc,buffer);
  transmit_string_USART0(buffer,false,ln);
}

void USART0_Flush(void){
  unsigned char dummy;
  while ( UCSR0A & (1<<RXC0) ) dummy = UDR0;
}

void init_USART1() {
  //unsigned int number = F_CPU/16/BAUD1 - 1;
  UBRR1H = (unsigned char)(UBRR1_VALUE1>>8);
  UBRR1L = (unsigned char)UBRR1_VALUE1;
  UCSR1B = (1<<RXEN1)|(1<<TXEN1)|(1 << RXCIE1);// Enable receiver and transmitter 
  UCSR1C = (1<<USBS1)|(3<<UCSZ10);// Set frame format: 8data, 2stop bit 
  UCSR1A |= (1<<U2X1);
}

void transmit_USART1(char c) { 
  while (!(UCSR1A & (1<<UDRE1))); //Wait for empty transmit buffer 
  UDR1 = c;
}

void transmit_string_USART1(const char *buf, uint8_t p, uint8_t ln){
  if(p){
    while (pgm_read_byte(buf) != 0x00){
      transmit_USART1(pgm_read_byte(buf++));
    }
  }
  else{
    while (*buf){
      transmit_USART1(*buf++);
    }
  }
  if (ln) transmit_USART1('\n');
}

unsigned char USART1_Receive(void){
  while (!(UCSR1A & (1<<RXC1)));
  return UDR1;
}

#define BUFFER_SIZE 512
volatile int rxn=0; 
volatile char rx[BUFFER_SIZE]; 
volatile uint8_t rxFlag = 0; 
ISR(USART1_RX_vect){
  while (!(UCSR1A & (1<<RXC1)));
  if (rxn==BUFFER_SIZE)
    rxn=0;
  rx[rxn++] = UDR1; // increment rxn and return new value.   
  rxFlag=1; // notify main of receipt of data.
}


void ring_flush(){
  for(int i=0; i<BUFFER_SIZE; i++){
    rx[i]='\0';
  }
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

My bad I assumed checking for RXC was needed. I am still learning after a year in dealing with AVRs!

 

Also your index management is suspect. Increment the index then test for wrap around - not the other way. The index should always have a valid index to the array.

How do you mean? I implemented that ring buffer from the ring buffer tutorial somewhere here on avrfreaks. I understand what you are saying but why not test for wrap around first?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Probably me being ignorant! I just fixed that but it has no bearing on the error.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

atmega644pa at 16MHz,

 

For 115200 baud rate there is a 2% error. I changed the frequency to 18.4320 MHz but the previous problem of bits dropping was still due to other code errors not the frequency change.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 DDRD |= 1 << PIND4;
  DDRD |= 1 << PIND6;
  DDRB |= 1 << PINB3;
  DDRB |= 1 << PINB4;

Why have you forgotten to define your pin names? Nothing like accidentally setting PB6 instead of PB8 & wondering why the cola selection tastes like root beer.

Why take 6 or 7 code lines to say what should be said in one line?   Avoid ambiguity--simply say DDRD= what is needed    DDRD= (1<<HOTVALVE) | (1<<COLDVALVE), or whatever you need

Concise, and clear are your free detective friends 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ISR(USART1_RX_vect){
  while (!(UCSR1A & (1<<RXC1)));

I noticed this along with the other people.   This while statement is what you would see in a TX routine, but not an RX. 

When a byte fully arrives at the USART1, the RXC1 flag gets set.  IF the USART1_RX interrupt is enabled, the AVR suspends the main code, and goes to the ISR.  When it does this, the AVR clears the RXC1 flag, and moves the new byte from the USART1 shift register to the UDR1.  This instruction causes the running code to wait inside the while loop until another newer byte arrives on the RX.   Then the code reads the first RX in the USART1 data register, and the second RX gets placed in the USART1 register.   This normally appears to be synchronized, but the UDR1 is holding the byte that arrived before the ISR was invoked.  The latest RX byte is still in the USART1 shift register.

   Remove the while statement and adjust the buffer pointer in the manner that Kartman suggested.  Check how the operation changes.

Last Edited: Wed. Jan 29, 2020 - 09:55 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Leaving the index with an illegal value is just opening the door for defects. Others might pipe in with their comments. Of course, it might have little to do with the core problem you’ve reported.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don’t think _delay_ms() likes a variable parameter. Read the documentation to be sure.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That is what I thought but it does compile fine. I will implement the fix for it .

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The 'missing' characters might be caused because in sendModem2() you have

Quote:
if(echo){
transmit_string_USART0(buf,p,true);
for (int i=0; i<1024; i++){
transmit_USART0(str[i]);
}
//flush ring buffer
ring_flush();
which would null any received characters which arrive within about 9ms after sending the command.

for ( int i= 0; i < strlen( str ); i ++ ){ would be better.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why would it null it 9ms before?

I understand the point of strlen(str) but basically str is an array full of Nulls every time a command is sent. 

so basically when I print 0 to 1024 that would get the whole array.

I don't see the 9ms nulling .

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

My mistake., that 9ms should be 89ms. (I forgot that there are 1+8+1 time intervals per character)


As i see it, in sendModem2() you send a command via USART1.
If echo is true you then send 1024 characters from str[], one at a time via USART0 and that loop will take 89ms to complete.
Meanwhile, if any characters are received on USART1 the USART1_Receive() interrupt will store them into the buffer rx[]
When your echo has finished sending 1024 characters it then calls ring_flush() which sets everything in rx[] to null.

Last Edited: Thu. Jan 30, 2020 - 04:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why are you getting a watchdog reset? (your output indicates).  You must be dealing with the watchdog somewhere to get a watchdog to fire in the first place, and once fired you most likely have nothing in place (that we can see) to disable it. Which would explain the duplicate watchdog messages- once fired, you are making it to the init again where it prints again (within default 15ms timeout, and at 115200baud there is enough time), then go on to wait in a delay for 1 second, where the watchdog timed out and fired again. Repeat.

 

Do yourself a favor, and troubleshoot all your other problems with the watchdog off. 

 

If you do not touch the watchdog (appears not, but maybe code shown is not the real thing), and if not fused on (it cannot be, else you would never make it past the first delay), then I guess you must be doing something special to get a wdrf flag set and unused irq's to fire. Maybe you are really running wild in the io memory area and hitting every other bit in sight (writing to a buffer, ending up in io mem by mistake). The watchdog does not take anything special to turn on, and writing a bunch of random bytes to the io area could easily get the watchdog set, and a peripheral irq enabled (or two in this case). 

 

I'll add, although there are a number of problems to choose from it seems, your two copies of rxn (counter, counter2) are not protected when reading and the isr could have changed the value in between the reading of rxn (2 bytes to read), leaving either counter value outside the 0-511 range and you will be stuck in that loop all the while doing a strcat each time through. I'm not sure if that could get the writes into the io area as I think/thought any wraps beyond ram wrapped back to 0x100, but I don't know.

 

//get response between counter and counter2
      char str[1024]={'\0'};
        for (int i=counter ; i!=(counter2) ; i=(i+1)%BUFFER_SIZE){
          char cAsStr[] = { rx[i], '\0' };
          strcat(str,cAsStr);
        //transmit_USART0(rx[i]);
        }

 

 

 

Last Edited: Thu. Jan 30, 2020 - 08:24 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for looking at the code!

curtvm wrote:

If you do not touch the watchdog (appears not, but maybe code shown is not the real thing), and if not fused on (it cannot be, else you would never make it past the first delay), then I guess you must be doing something special to get a wdrf flag set and unused irq's to fire. Maybe you are really running wild in the io memory area and hitting every other bit in sight (writing to a buffer, ending up in io mem by mistake). The watchdog does not take anything special to turn on, and writing a bunch of random bytes to the io area could easily get the watchdog set, and a peripheral irq enabled (or two in this case). 

Yeah I am not touching the watchdog whatsoever. I understand what you are saying and I could disable it in the watchdog ISR. The code posted above may not be the future code I want it to evolve into, but it was compiled and uploaded by itself and I verified it had the watchdog error and other interrupts firing.

 

curtvm wrote:

I'll add, although there are a number of problems to choose from it seems, your two copies of rxn (counter, counter2) are not protected when reading and the isr could have changed the value in between the reading of rxn (2 bytes to read), leaving either counter value outside the 0-511 range and you will be stuck in that loop all the while doing a strcat each time through. I'm not sure if that could get the writes into the io area as I think/thought any wraps beyond ram wrapped back to 0x100, but I don't know.

 

How would that leave counter and counter2 outside of 0-511 If rxn could only be in that range? Forgive me because I do not understand your last sentence either.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for looking at the code!

curtvm wrote:

If you do not touch the watchdog (appears not, but maybe code shown is not the real thing), and if not fused on (it cannot be, else you would never make it past the first delay), then I guess you must be doing something special to get a wdrf flag set and unused irq's to fire. Maybe you are really running wild in the io memory area and hitting every other bit in sight (writing to a buffer, ending up in io mem by mistake). The watchdog does not take anything special to turn on, and writing a bunch of random bytes to the io area could easily get the watchdog set, and a peripheral irq enabled (or two in this case). 

Yeah I am not touching the watchdog whatsoever. I understand what you are saying and I could disable it in the watchdog ISR. The code posted above may not be the future code I want it to evolve into, but it was compiled and uploaded by itself and I verified it had the watchdog error and other interrupts firing.

 

curtvm wrote:

I'll add, although there are a number of problems to choose from it seems, your two copies of rxn (counter, counter2) are not protected when reading and the isr could have changed the value in between the reading of rxn (2 bytes to read), leaving either counter value outside the 0-511 range and you will be stuck in that loop all the while doing a strcat each time through. I'm not sure if that could get the writes into the io area as I think/thought any wraps beyond ram wrapped back to 0x100, but I don't know.

 

How would that leave counter and counter2 outside of 0-511 If rxn could only be in that range? Forgive me because I do not understand your last sentence either.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see, I think the dropping error was actually something else. The reason it does not persist now is because I took out the other snippet of code which was breaking it.

Somewhere in that extra snippet I was stopping interrupts which would explain why my characters were dropped phase of the moon dependently. 

Yet the main issue of freezing has yet to be identified by me.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
//get response between counter and counter2
      char str[1024]={'\0'};
        for (int i=counter ; i!=(counter2) ; i=(i+1)%BUFFER_SIZE){
          char cAsStr[] = { rx[i], '\0' };
          strcat(str,cAsStr);
        //transmit_USART0(rx[i]);
        }

You appear to be writing code as if this were an infinite resource PC. I know you said 644 so it has a fair amount of RAM but I wouldn't just scatter [512] and [1024] byte buffers all over the place on an 8 bit micro with limited SRAM.

 

When you do build what  is the reported size for static allocation in .bss and .data - IOW how much is left for stack frame autos? (having said that I do see sensible use of PSTR() so hopefully RAM usage does not start too high).

 

PS unless this is C++ code (doesn't look like it) then I'd switch your pgm_read_*() stuff to use __flash data instead - just makes the code clearer and easier to follow and maintain.

Last Edited: Fri. Jan 31, 2020 - 12:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, I could probably optimize it. When I build its about 4k to 8k bytes of flash. I forget how to make things into .bss and .data, I am using avrdude and avr gcc

 

PS unless this is C++ code (doesn't look like it) then I'd switch your pgm_read_*() stuff to use __flash data instead - just makes the code clearer and easier to follow and maintain.

I am not sure what you mean here, because I don't know the particular difference in reading a byte using avr pgmspace versus _flash data. Is there a better function i don't know about?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I did avr-size of my current .elf that includes some of the changes mentioned in this thread.

I also changed the buffer size down to 64 and

the string variable "char str[70]={'\0'}" to 70:

 

AVR Memory Usage
----------------
Device: atmega644pa

Program:    5742 bytes (8.8% Full)
(.text + .data + .bootloader)

Data:         67 bytes (1.6% Full)
(.data + .bss + .noinit)

 

 

   text    data     bss     dec     hex filename
      0    5742       0    5742    166e output/codes/ecobmodemTest.hex

Last Edited: Fri. Jan 31, 2020 - 04:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>How would that leave counter and counter2 outside of 0-511 If rxn could only be in that range?

 

I didn't fully work out the details, but counter/counter2 (int's) can be corrupt rxn values because the isr changes rxn, which can happen in between the counter reads. Maybe there is no way the corrupt read leaves those values anything but 0-511, but you are also leaving the isr when rxn is at 512 (checking for overflow at the start of the next isr) so if counter2 ends up with the 512, i will always be != counter2 (unless counter also got the 512) and you are stuck in that loop doing a strcat. Maybe nothing is going on there and I am seeing something that is not there, but non-atomic reads on rxn are a problem no matter what.

 

>Forgive me because I do not understand your last sentence either.

 

Lets say you were stuck in a loop doing a strcat. The buffer is being appended repeatedly. Eventually you get to the end of ram. Where does the data go when trying to write past the end of ram? From experiments long ago, you will end up 'wrapping' around to an address that is 0x100+ and does not wrap into register/io space. That would debunk my theory of the strcat loop as you don't get into io space which means that cannot be setting the wde bit.

 

Somehow you must be hitting the wde bit to get the watchdog activated, and at least a few others to get unused irq's enabled.

 

If you have a debugger, put it to use. If not, then I would start a new project and assemble it using pieces from your existing project. One small piece of code at a time, testing each addition, fixing problems as you go. Maybe you discover your original problem, maybe you don't. Skip the strcat and the large buffers, as they are not needed. 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If not, then I would start a new project and assemble it using pieces from your existing project

Starting over is often the best way...it goes 10x faster since you've already explored 9 ways that didn't work & that info is fresh in your mind.  By now, you also have a much clearer picture of everything you need to do & what ended up being frivolous or not useful.  You are also able to better be on alert for hidden problem situations you discovered in round one.   Would you rather have a house that has been remodeled 10 times or a new house built from the learnings of 10 remodels?  Usually, the latter.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Fri. Jan 31, 2020 - 09:33 PM