Search code examples
cavratmega

C program stops execution when assigning variable


I'm not sure if this is happening when assigning a variable specifically but when debugging the assembly code, the compiler executes RJMP $+0000 where it hangs the program.

EDIT: I added included libraries if that's relevant

#define __DELAY_BACKWARD_COMPATIBLE__
#define F_CPU 8000000UL
#include <avr/io.h>
#include <avr/delay.h>
#include <stdint.h> 

void ReadTemp(uint8_t address){
    
    ADCSRA = ADCSRA | 0b10000111;       //enable ADC, CLK/128 conversion speed
    ADMUX  = ADMUX | 0b01000000;        //Use internal 2.56V Vref and PA0 as input, right-hand justified
        
    ADCSRA |= (1 << ADSC);              //start conversion
        
    while(!(ADCSRA & (1 << ADIF))) {}   // wait until process is finished;

    uint8_t  low_value = ADC & 0x00FF;
// or low_value = ADCL;
    uint8_t high_value = ADC & 0xFF00;   //problem here
...
}

enter image description here


Solution

  • There is something somewhat suboptimal here:

    uint8_t low_value = ADC & 0x00FF;
    

    You are reading ADC, which is a 16-bit register, implemented as a pair of 8-bit registers. As shown in the disassembly, this requires two in instructions, one per byte. And then you are just throwing away one of those bytes. You may think that the compiler is smart enough to avoid reading the byte it is going to discard right away. Unfortunately, it cannot do that, as I/O registers are declared as volatile. The compiler is forced to access the register as many times as the source code does.

    If you just want the low byte, you should read only that byte:

    uint8_t low_value = ADCL;
    

    You then wrote:

    uint8_t high_value = ADC & 0xFF00;
    

    As explained in the previous answer, high_value will be zero. Yet the compiler will have to read the two bytes again, because the I/O register is volatile. If you want to read the high byte, read ADCH.

    But why would you want to read those two bytes one by one? Is it to put them back together into a 16-bit variable? If this is the case, then there is no need to read them separately. Instead, just read the 16-bit register in the most straight-forward fashion:

    uint16_t value = ADC;
    

    A long time ago, gcc didn't know how to handle 16-bit registers, and people had to resort to reading the bytes one by one, and then gluing them together. You may still find very old example code on the Internet that does that. Today, there is absolutely no reason to continue programming this way.

    Then you wrote:

    //problem here
    

    Nope, the problem is not here. That is not what generated the rjmp instruction. The problem is probably right after that line, in the code you have chosen not to post. You have some bug that manifests itself only when optimizations are turned on. This is typical of code that produces undefined behavior: works as expected with optimizations off, then does weird “unexplainable” things when you enable optimizations.