2

So I am working on getting the gps data when the button is pressed using external interrupt. My problem is , it is working without the external interrupt but when I added it, it's not getting the data. Is something wrong with my code? I am using quectel l80-m39 for gps connected to the rx of atmega328p. Provided below is my code. The button is connected to PD3 of atmega.

#define F_CPU 8000000UL // Defining the CPU Frequency

#include <avr/io.h>     // Contains all the I/O Register Macros
#include <util/delay.h> // Generates a Blocking Delay
#include <avr/interrupt.h>

#define USART_BAUDRATE 9600 // Desired Baud Rate
#define BAUD_PRESCALER (((F_CPU / (USART_BAUDRATE * 16UL))) - 1)

void USART_Init()
{
    // Set Baud Rate
    UBRR0H = (unsigned char)(BAUD_PRESCALER >> 8);
    UBRR0L = (unsigned char)BAUD_PRESCALER;

    // Enable Receiver
    UCSR0B |= (1 << RXEN0);

    // Set data frame format: asynchronous mode,no parity, 1 stop bit, 8 bit size
    UCSR0C |= (1 << UCSZ01) | (1 << UCSZ00);
}

unsigned char USART_ReceivePolling(void)
{
    while (!(UCSR0A & (1 << RXC0)));
        return UDR0;
}


void readgps()
{
    uint8_t lati_value[15];
    unsigned char LocalData;
    int i = 0;
    LocalData = USART_ReceivePolling();
    if (LocalData == '$')
    {
        LocalData = USART_ReceivePolling();
        if (LocalData == 'G')
        {
            LocalData = USART_ReceivePolling();
            if (LocalData == 'P')
            {
                LocalData = USART_ReceivePolling();
                if (LocalData == 'G')
                {
                    LocalData = USART_ReceivePolling();
                    if (LocalData == 'G')
                    {
                        LocalData = USART_ReceivePolling();
                        if (LocalData == 'A')
                        {
                            LocalData = USART_ReceivePolling();
                            if (LocalData == ',')
                            {
                                LocalData = USART_ReceivePolling();
                                while (LocalData != ',')
                                {
                                    LocalData = USART_ReceivePolling();
                                }
                                LocalData = USART_ReceivePolling();
                                while (LocalData != ',')
                                {
                                    lati_value[i] = LocalData;
                                    ++i;
                                    LocalData = USART_ReceivePolling();
                                }
                                PORTD |= (1 << PD2);
                                _delay_ms(100);
                            }
                        }
                    }
                }
            }
        }
    } else 
    {
        PORTD &= ~(1 << PD2);
    }
}

int main(void)
{
    EICRA |= (1 << ISC10) | (1 << ISC11); // interrupt on rising edge of INT1
    EIMSK |= (1 << INT1);                 // external interrupt enable on INT1
    
    sei();
    while (1)
    {

    }
}

ISR(INT1_vect)
{
    if (PIND & (1 << 3))
    {
        DDRD = 0b01110110;
        USART_Init();
        readgps();
    }
} 
Armandas
  • 2,276
  • 1
  • 22
  • 27

1 Answers1

1

USART_ReceivePolling() is blocking the ISR. You should never block the ISR. In the ISR just set a flag and check it in the main loop. Something like this (unchanged code removed):

// ...

// Flag for reading the GPS
volatile unsigned char g_read_gps = 0;

// ...

int main(void)
{
    // ...

    while (1)
    {
        if (g_read_gps) {
            readgps();
            g_read_gps = 0;
        }
    }
}

ISR(INT1_vect)
{
    if (PIND & (1 << 3))
    {
        g_read_gps = 1;
    }
}
Armandas
  • 2,276
  • 1
  • 22
  • 27
  • I added the flag but still not doing it. ``` int main(void) { EICRA |= (1 << ISC10) | (1 << ISC11); // interrupt on rising edge of INT1 EIMSK |= (1 << INT1); // external interrupt enable on INT1 sei(); while (1) { if (f_gps) { f_gps = 0; DDRD = 0b01110110; USART_Init(); readgps(); } } } ISR(INT1_vect) { if (PIND & (1 << 3)) { f_gps = 1; } } ``` – Jane Malibiran Feb 02 '23 at 04:49
  • @JaneMalibiran I assumed that the interrutps already work, but I think you may not be setting up the interrupts correctly. You should start with a minimal example first, then when that works, move onto reading the UART. See here: https://ouilogique.com/interruptions/ – Armandas Feb 02 '23 at 05:55
  • 1
    It is now working. I am wrong with how I should get data when the interrupt is executed. It should continue to loop USART_ReceivePolling() while the GPGGA part is not yet received. Thank you so much. – Jane Malibiran Feb 02 '23 at 06:06
  • How exactly is it blocking the ISR? Is reading the flag + reading the data register causing the flag to get cleared? If so you should mention this in your answer. It also seems like calling some init code from an ISR isn't the best idea. – Lundin Feb 02 '23 at 09:51
  • @Lundin What I mean is that the receive function spins in a loop inside the interrupt waiting for UART data, preventing the interrupt from completing quickly. – Armandas Feb 02 '23 at 10:38
  • @Armandas Ah, right. Yeah the general best practice is to keep ISRs minimal. However, it can make sense to have an ISR trigger on the first byte of an incoming message and then poll for additional ones, in case UART data is coming in with a fast baudrate. This locks up the whole MCU of course, but it's simpler to code than some ring buffer scheme. – Lundin Feb 02 '23 at 10:43
  • 1
    I suggest moving `g_read_gps = 0;` to _after_ `readgps()`. Switch bounce will make it almost certain that it will immediately get set to 1 and `readgps()` will be executed twice. The GNSS sentence read will be long enough that the switch will stop bouncing by the time it is done. – Clifford Feb 02 '23 at 13:47
  • 1
    And really: `volatile unsigned char g_read_gps = 0;` is required here. – Clifford Feb 02 '23 at 13:48