On TM4C123GH6PM Launch Pad, is my ADC process conflicting with main? Changing LED

Question

The code should do as follows, initialize adc so it can communicate with the external temp sensor, wired to breadboard. It is doing so, and I am getting accurate temperatures.

Based on that temperature, it should change the onboard LED to a corresponding color. I put in the blinking code just to make sure the onboard LED was functioning, which is does.

The board will blink without any of the ADC code, so I am assuming that is my issue, but it's hard to tell what is specifically causing it.

#include <stdint.h>
#include <stdbool.h>
#include <string.h>
#include <stdio.h>
#include "inc/hw_memmap.h"
#include "inc/hw_types.h"
#include "driverlib/debug.h"
#include "driverlib/sysctl.h"
#include "driverlib/adc.h"
#include "driverlib/interrupt.h"
#include "inc/tm4c123gh6pm.h"
#include "driverlib/gpio.h"

//bit masks for leds
#define RED_LED_MASK 0x02  
#define BLUE_LED_MASK 0x04
#define GREEN_LED_MASK 0x08

char state[10];
uint32_t ui32ADC0Value;
volatile uint32_t ui32TempValueC;
volatile float temp;
volatile float voltage;

//ADC0 initialization
void ADC0_Init(void) {
    SysCtlClockSet(SYSCTL_SYSDIV_5 | SYSCTL_USE_PLL | SYSCTL_OSC_MAIN | SYSCTL_XTAL_16MHZ); // configure the system clock to be 40MHz
    SysCtlPeripheralEnable(SYSCTL_PERIPH_ADC0); // activate the clock of ADC0
    SysCtlDelay(2); // insert a few cycles after enabling the peripheral to allow the clock to be fully activated.

    ADCSequenceDisable(ADC0_BASE, 3); // disable ADC0 before the configuration is complete
    ADCSequenceConfigure(ADC0_BASE, 3, ADC_TRIGGER_PROCESSOR, 0); // will use ADC0, SS1, processor-trigger, priority 0
        
    // ADC0 SS3 Step 3, sample from external temp sensor
    ADCSequenceStepConfigure(ADC0_BASE, 3, 0, ADC_CTL_CH0 | ADC_CTL_IE | ADC_CTL_END); 
    ADCIntClear(ADC0_BASE, 3);
    IntPrioritySet(INT_ADC0SS3, 0x00); // configure ADC0 SS1 interrupt priority as 0
    IntEnable(INT_ADC0SS3); // enable interrupt 31 in NVIC (ADC0 SS1)
    ADCIntEnableEx(ADC0_BASE, ADC_INT_SS3); // arm interrupt of ADC0 SS1

    ADCSequenceEnable(ADC0_BASE, 3); // enable ADC0
}
        
//interrupt handler
void ADC0_Handler(void) {
    ADCIntClear(ADC0_BASE, 3); // clear interrupt flag of ADC0 SS3
    ADCProcessorTrigger(ADC0_BASE, 3); // Software trigger the next ADC sampling
    ADCSequenceDataGet(ADC0_BASE, 3, &ui32ADC0Value); // Load the captured data from FIFO
    voltage = (ui32ADC0Value * 3.3) / 4096;
    temp = (voltage - 0.5) * 100;
    ui32TempValueC = temp;
    temp = (ui32TempValueC * 9.0 / 5.0) + 32.0;

    if (temp < 68) {
            strcpy(state, "COLD");
    } else if (temp > 70) {
            strcpy(state, "HOT");
    } else {
            strcpy(state, "IDLE");
    }

}

void PortFunctionInit(void) {
    // dummy variable
    volatile uint32_t ui32Loop;   
    
    // ENABLE THE PORT CLOCK
    SYSCTL_RCGC2_R |= SYSCTL_RCGC2_GPIOF; 
    
    // dummy read to insert a few 
    ui32Loop = SYSCTL_RCGC2_R; 
    
    GPIO_PORTF_LOCK_R = 0x4C4F434B;   // unlock 
    GPIO_PORTF_CR_R |= 0x01; // allow changes 
    
    GPIO_PORTF_AFSEL_R |= 0x00; // 
    
    GPIO_PORTF_DEN_R |= RED_LED_MASK; // enable pin1 on port f
    GPIO_PORTF_DIR_R |= RED_LED_MASK; // make output pin

    GPIO_PORTF_DEN_R |= BLUE_LED_MASK; // enable pin2 on port f
    GPIO_PORTF_DIR_R |= BLUE_LED_MASK; // make output pin
    
    GPIO_PORTF_DEN_R |= GREEN_LED_MASK;// enable pin3 on port f
    GPIO_PORTF_DIR_R |= GREEN_LED_MASK; // make output pin
}

int main(void) {
    SysCtlPeripheralEnable(SYSCTL_PERIPH_GPIOE);
    GPIOPinTypeADC(GPIO_PORTE_BASE, GPIO_PIN_3);
    
    // initialize the GPIO port
    PortFunctionInit();
        
    ADC0_Init();
    IntMasterEnable(); // globally enable interrupt
    ADCProcessorTrigger(ADC0_BASE, 3);
    
/*  while (1) {
        GPIO_PORTF_DATA_R |= GREEN_LED_MASK;
        SysCtlDelay(2000000);
        GPIO_PORTF_DATA_R ^= GPIO_PORTF_DATA_R;
        SysCtlDelay(2000000);
    }
*/
    while (1) {
        GPIO_PORTF_DATA_R &= ~(RED_LED_MASK | BLUE_LED_MASK | GREEN_LED_MASK);

        if (strcmp(state, "COLD") == 0) {
            GPIO_PORTF_DATA_R |= RED_LED_MASK;
        } 
        else if (strcmp(state, "HOT") == 0) {
            GPIO_PORTF_DATA_R |= BLUE_LED_MASK;
        } 
        else if (strcmp(state, "IDLE") == 0) {
            GPIO_PORTF_DATA_R |= GREEN_LED_MASK;
        } 
    }
}

Working with a Texas Instruments TM4C123GH6PM Launch Pad, in Keil. I have connected an external temperature sensor(I know it has one on it, it is for a school project), and eventually will connect a motion sensor as well.

The idea is when the temperature is in a certain range, the LED on the board will change colors accordingly. The problem is, I think the ADC process is blocking, or interrupting main from executing. But I can't figure out how to prevent, or fix this issue.

This is for a school project, and the lab this year has felt lacking when it comes to understanding initializations and such, so if it is glaringly obvious I apologize and would appreciate the help.

When I run the code with this in the main loop:

    while (1) {
        GPIO_PORTF_DATA_R |= GREEN_LED_MASK;
        
        SysCtlDelay(2000000);
    
        GPIO_PORTF_DATA_R ^= GPIO_PORTF_DATA_R;
    
        SysCtlDelay(2000000);
    }

Then the LED blinks just fine when all the ADC functions are commented out. When I bring them back in, the LED will turn on, but just stay green.

When I run the actual code I want to run:

    while(1) {
        GPIO_PORTF_DATA_R &= ~(RED_LED_MASK | BLUE_LED_MASK | GREEN_LED_MASK);

        if (strcmp(state, "COLD") == 0) {
            GPIO_PORTF_DATA_R |= RED_LED_MASK;
        } 
        else if (strcmp(state, "HOT") == 0) {
            GPIO_PORTF_DATA_R |= BLUE_LED_MASK;
        } 
        else if (strcmp(state, "IDLE") == 0) {
            GPIO_PORTF_DATA_R |= GREEN_LED_MASK;
        } 
    }

Then the LED doesn't even turn on, and when tracking through debugger, the program never really steps into the if statement, even when state matches any of the conditions. I have verified this using watch, for state and temp.

I am at a loss embedded C is not my strong suit. Any help would be appreciated. I am assuming it has to do with the ADCSequenceStepConfigure line. Is there something I have not configured correctly in that step?

It is using SS3 to gather continuous samples, is that what is interrupting my main function from completing?

Is there an issue with the stringCmp way of doing it?

/*while(1) {
    strcpy(state, "COLD");
    GPIO_PORTF_DATA_R &= ~(RED_LED_MASK | BLUE_LED_MASK | 
GREEN_LED_MASK);
    test = 0;
    
    if (strcmp(state, "COLD") == 0) {
        test = 1;
        GPIO_PORTF_DATA_R |= RED_LED_MASK;
    } 
    else if (strcmp(state, "HOT") == 0) {
        test = 2;
        GPIO_PORTF_DATA_R |= BLUE_LED_MASK;
    } 
    else {
        // Add more conditions if needed
    }
} testing string compare with strcpy with state*/

Answer 1

First of all eradicate all on the unnecessary conversions to voltage, Celsius, and Fahrenheit and the associated floating point operations. Without enabling the FPU, the floating point operations will be prohibitively slow for an interrupt, and with the FPU enabled the floating point operation in the interrupt handler will screw up any on-going floating-point operation pre-empted in the main() context.

There is no need for the ISR to have knowledge of "human" units. The thresholds can be pre-calculated in ADC units. In this case:

// Temperature ADC thresholds
#define TEMP_LO_THRESHOLD 869   // 68degF in ADC units
#define TEMP_HI_THRESHOLD 883   // 70degF in ADC units

Noting that that is a rather narrow range and may be subject to noise. If all you are interested in is a 2 degree range, having hardware with a range -50 to +280 degrees C is not perhaps a good design. You have very low resolution at the temperature of interest.

Then you have to understand that the ADC conversion probably takes a few microseconds to complete, even at the lowest possible ADC clock rate available. As such re-enabling the interrupt in the interrupt handler is probably not a good idea, the conversion is likely to be complete before the ISR completes, so will simply immediately re-enter. At best it will dedicate rather more CPU cycles to the ISR than the application warrants.

One solution is to have a timer running at the desired sample rate with the timer triggering the ADC conversion (more on that below), but a simple solution that will likely be adequate in this case would be to trigger the conversion in the main() context on each iteration of the loop. That way you guarantee that that loop processing will get CPU time.

Furthermore, the use of a human-readable string to convey state to be used only by a machine is inappropriate for several reasons - not least that it is non-atomic and grossly inefficient. You should use a simple and atomic integer type for this. Further for readability you can use an enumeration:

volatile enum
{
    IDLE,
    COLD,
    HOT
} state = IDLE ;

So now you might have an ISR like:

void ADC0_Handler(void) 
{
    uint32_t ui32ADC0Value = 0 ;

    ADCIntClear( ADC0_BASE, 3 ) ; 
    ADCSequenceDataGet( ADC0_BASE, 3, &ui32ADC0Value ) ;

    if( ui32ADC0Value < TEMP_LO_THRESHOLD ) 
    {
       state = COLD ;
    } 
    else if( temp > TEMP_HI_THRESHOLD ) 
    {
        state = HOT ;
    } 
    else 
    {
        state = IDLE ;
    }
}

noting too that ui32ADC0Value need not be global since it is only used in the ISR.

And your main() loop:

    ADC0_Init();
    IntMasterEnable(); // globally enable interrupt
    
    for(;;)
    {
        if( !ADCBusy() )
        {
            ADCProcessorTrigger( ADC0_BASE, 3 ) ;
        }

        GPIO_PORTF_DATA_R &= ~(RED_LED_MASK | BLUE_LED_MASK | GREEN_LED_MASK);

        switch( state )
        {
            case COLD :
            {
                GPIO_PORTF_DATA_R |= RED_LED_MASK ;
            }
            break ;
            
            case HOT : 
            {
                GPIO_PORTF_DATA_R |= BLUE_LED_MASK ;
            }
            break ;
            
            default :
            {
                GPIO_PORTF_DATA_R |= GREEN_LED_MASK ;
            }
            break ;
        } 
    }

moving the ADC trigger inside the loop, so that at least one loop iteration will process between conversions.

Now given this change, it is apparent that in this particular case, when there is no other work to be done while the ADC conversion completes and the ADC conversion is in any event probably very fast in relation to the interrupt context switch overhead, you might conclude that the interrupt is not necessary in any case. Almost certainly true in this case. In which case the ISR can be removed (and the interrupt configuration in ADC0_Init() too) and simply wait for the conversion to complete in the main() context:

    ADC0_Init();

    for(;;)
    {
        ADCProcessorTrigger(ADC0_BASE, 3) ;
        while( ADCBusy() )
        {
            // busy-wait for conversion
        }
        
        ADCSequenceDataGet(ADC0_BASE, 3, &ui32ADC0Value); // Load the captured data from FIFO

        GPIO_PORTF_DATA_R &= ~(RED_LED_MASK | BLUE_LED_MASK | GREEN_LED_MASK);

        if( ui32ADC0Value < TEMP_LO_THRESHOLD ) 
        {
            GPIO_PORTF_DATA_R |= RED_LED_MASK ;
        } 
        else if( temp > TEMP_HI_THRESHOLD ) 
        {
            GPIO_PORTF_DATA_R |= BLUE_LED_MASK ;
        } 
        else 
        {
            GPIO_PORTF_DATA_R |= GREEN_LED_MASK ;
        }
    }

noting that the enum is now not required either (although So you should seriously consider whether you need to use the ADC interrupt since in this case there are few advantages. In general ADC conversion times are so small that an interrupt for a single conversion is prohibitive.

In cases where you need a rock-solid deterministic sample interval, a timer trigger would be appropriate. I am not familiar with this part, but I would imagine timer triggered ADC conversion is an option. If not you might have a timer ISR trigger the ADC conversion, but that you would cause two interrupts in rapid succession - the timer then the ADC. Either way, you then have an interrupt rate determined by the application requirements rather than the ADC conversion time.

In signal processing applications you would instead capture multiple samples in a block using DMA, with the DMA interrupt triggering when the DMA buffer was full, you then get an interrupt rate at some rate lower then the sample rate and process a block of samples in one go which is significantly more efficient.

So the final code without the probably unnecessary use of ADC interrupts (and what I suspect are unnecessary headers removed) might look like:

#include <stdint.h>
#include "inc/tm4c123gh6pm.h"
#include "driverlib/sysctl.h"
#include "driverlib/adc.h"
#include "driverlib/gpio.h"

// bit masks for leds
#define RED_LED_MASK 0x02  
#define BLUE_LED_MASK 0x04
#define GREEN_LED_MASK 0x08

// Temperature ADC thresholds
#define TEMP_LO_THRESHOLD 869   // 68degF in ADC units
#define TEMP_HI_THRESHOLD 883   // 70degF in ADC units

//ADC0 initialization
void ADC0_Init(void) 
{
    SysCtlClockSet(SYSCTL_SYSDIV_5 | SYSCTL_USE_PLL | SYSCTL_OSC_MAIN | SYSCTL_XTAL_16MHZ); // configure the system clock to be 40MHz
    SysCtlPeripheralEnable(SYSCTL_PERIPH_ADC0); // activate the clock of ADC0
    SysCtlDelay(2); // insert a few cycles after enabling the peripheral to allow the clock to be fully activated.

    ADCSequenceDisable(ADC0_BASE, 3); // disable ADC0 before the configuration is complete
    ADCSequenceConfigure(ADC0_BASE, 3, ADC_TRIGGER_PROCESSOR, 0); // will use ADC0, SS1, processor-trigger, priority 0
        
    // ADC0 SS3 Step 3, sample from external temp sensor
    ADCSequenceStepConfigure(ADC0_BASE, 3, 0, ADC_CTL_CH0 | ADC_CTL_IE | ADC_CTL_END); 
    ADCSequenceEnable(ADC0_BASE, 3); // enable ADC0
}
        

void PortFunctionInit(void) 
{
    // ENABLE THE PORT CLOCK
    SYSCTL_RCGC2_R |= SYSCTL_RCGC2_GPIOF; 
    
    // dummy read to insert a few 
    volatile uint32_t ui32Loop = SYSCTL_RCGC2_R; 
    
    GPIO_PORTF_LOCK_R = 0x4C4F434B;   // unlock 
    GPIO_PORTF_CR_R |= 0x01; // allow changes 
    
    GPIO_PORTF_AFSEL_R |= 0x00; // 
    
    GPIO_PORTF_DEN_R |= RED_LED_MASK; // enable pin1 on port f
    GPIO_PORTF_DIR_R |= RED_LED_MASK; // make output pin

    GPIO_PORTF_DEN_R |= BLUE_LED_MASK; // enable pin2 on port f
    GPIO_PORTF_DIR_R |= BLUE_LED_MASK; // make output pin
    
    GPIO_PORTF_DEN_R |= GREEN_LED_MASK;// enable pin3 on port f
    GPIO_PORTF_DIR_R |= GREEN_LED_MASK; // make output pin
}

int main(void) 
{
    SysCtlPeripheralEnable(SYSCTL_PERIPH_GPIOE);
    GPIOPinTypeADC(GPIO_PORTE_BASE, GPIO_PIN_3);
    
    // initialize the GPIO port
    PortFunctionInit();
        
    ADC0_Init();

    for(;;)
    {
        ADCProcessorTrigger(ADC0_BASE, 3) ;
        while( ADCBusy() )
        {
            // busy-wait for conversion
        }
        
        ADCSequenceDataGet(ADC0_BASE, 3, &ui32ADC0Value); // Load the captured data from FIFO

        GPIO_PORTF_DATA_R &= ~(RED_LED_MASK | BLUE_LED_MASK | GREEN_LED_MASK);

        if( ui32ADC0Value < TEMP_LO_THRESHOLD ) 
        {
            GPIO_PORTF_DATA_R |= RED_LED_MASK ;
        } 
        else if( temp > TEMP_HI_THRESHOLD ) 
        {
            GPIO_PORTF_DATA_R |= BLUE_LED_MASK ;
        } 
        else 
        {
            GPIO_PORTF_DATA_R |= GREEN_LED_MASK ;
        }
    }
}