1
\$\begingroup\$

I'm using ATmega328P with Atmel Studio. In the following code I declare almost all the variables inside the while loop:

#include <Arduino.h>//this is to use the Serial library
#include <util/delay.h>
#include <avr/io.h>
#define F_CPU   16000000UL

const int DATA_PIN = 6;        

int main(void) {

    DDRD = B0100000;
    DDRD |= 1<<5;
    Serial.begin(57600);

    while (1) {      

        unsigned long data = 0;
        uint8_t val;

        for (int i=0; i<25; i++) {
            data <<= 1; 
            PORTD &= ~(1 << 5);  
            _delay_us(2);    
            PORTD |= (1 << 5); 
            _delay_us(2); 

            val = 1 & (PIND >> PIND6);

            data |= val;     
        }  

        PORTD &= ~(1 << 5);  
        _delay_us(2);    
        PORTD |= (1 << 5); 
        _delay_us(2);

        alarm = 1 & (PIND >> PIND6);

        //rest of the code....

        unsigned long angle_count = data & 0x1FFF;
        unsigned long turn_count = data >> 14;
        float angle = (angle_count * 360UL) / 8192.0;   

        Serial.print(angle);
        Serial.print(";");
        Serial.print(turn_count); 
        Serial.print(";");  
        Serial.println(alarm);  
    }
}

In terms of speed and efficiency, I cannot be sure where to declare them. There are two more alternative places such as in the main but outside the while loop or right at the very beginning of the code after #define and %include libraries. Also how can we make judgement whether we need to use volatile or not?

\$\endgroup\$
  • \$\begingroup\$ volatile: Do you have some specific scenario or question? \$\endgroup\$ – the busybee Jan 10 at 10:43
  • \$\begingroup\$ I added the whole code. What else needed? \$\endgroup\$ – ty_1917 Jan 10 at 10:44
  • \$\begingroup\$ I want stability as good as possible. \$\endgroup\$ – ty_1917 Jan 10 at 10:44
  • \$\begingroup\$ Do you think my code is fine to declare variables inside the while loop? \$\endgroup\$ – ty_1917 Jan 10 at 10:44
  • 1
    \$\begingroup\$ Yes. Indentation can be better, though. ;-) \$\endgroup\$ – the busybee Jan 10 at 10:46
4
\$\begingroup\$

In terms of speed and efficiency, I cannot be sure where to declare them.

For clean code the scope must be as small as possible.
Use the static keyword if you have to make it persistent.

For speed you may want to look into how and where you use it. Often automatic (local) scope variables are very fast since they fit in R0-R15, then follow relative operations on stack, and the slowest are absolute (global) or external variables which also require a full pointer to be loaded before read/write.

Also how can we make judgement whether we need to use volatile or not?

"An object that has volatile-qualified type may be modified in ways unknown to the implementation or have other unknown side effects."

Volatile is only required when the object changes outside program flow the compiler can predict. Such as peripheral registers or variables shared with interrupts or threads.
Volatile is expensive! Each expression with volatile is evaluated separately.


I also see some blobs of code that do have an obvious function to the reader. You may want to move these into a function. eg:

void sendDataByBitBanging(unsigned long data){
    uint8_t val;
    for (int i = 0; i < 25; i++){
        data <<= 1;
        PORTD &= ~(1 << 5);
        _delay_us(2);
        PORTD |= (1 << 5);
        _delay_us(2);

        val = 1 & (PIND >> PIND6);

        data |= val;
    }
}

Although a function might costs you a stackframe and a call/return instruction. Many modern compilers can inline functions when side-effects are limited.
As Micheal said below, you shouldn’t sacrifice readability or maintainability for performance.

As embedded developer you always have to make a compromise between abstraction, readability and raw performance due to the limited environment the software has to work on.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ The reason I avoided making function, that I thought function calls requires interrupts or sort of extra clock to the uC. Am I WRONG? \$\endgroup\$ – ty_1917 Jan 10 at 10:57
  • 2
    \$\begingroup\$ The compiler can inline functions to avoid the penalty of a function call. Unless speed is extremely important you shouldn’t sacrifice readability or maintainability for it. Premature optimisation is the root of all evil and all that… \$\endgroup\$ – Michael Jan 10 at 11:06
  • \$\begingroup\$ I had one interesting case (compiler: GCC, platform: AVR) of the need to use a function: Because in an interrupt service routine with a switch one case used a lot of registers, and so the compiler generated code to push and pop all these registers for each interrupt. The run-time got too long! But because the case was taken only in 1 of multiple interrupts, we put the statements of this case in a separate function marked as "don't inline." The registers were now saved only in this function. Now the run-time was acceptable, and also the jitter, when the special case was taken. \$\endgroup\$ – the busybee Jan 10 at 11:24
  • 1
    \$\begingroup\$ @ty_1917 Function calls do not require interrupts - study what interrupts are: they are kind of function calls themselves, but called by the hardware rather than from your program. Function call overhead is negligible in most cases, particularly if using modern gcc that is quite capable of effective inlining. The exception is when passing a lot of large parameters by value - 8 bitters don't like that at all, though I believe the AVR gcc calling convention allows fairly liberal use of registers to store parameters, so it is unlikely to be much of a performance issue. \$\endgroup\$ – Lundin Jan 10 at 12:32
4
\$\begingroup\$

Generally speaking, you should strive to give variables as local a scope as possible. This makes it easier to find them, eliminates bugs caused by namespace collisions and can possibly reduce the overall stack use.

Local variables declared inside a {} scope may also be easier for the compiler to allocate them in registers, which is the ideal place both in terms of speed and memory use.

However, for constrained embedded systems, declaring everything at the most local scope is not necessarily wise. 8-bitters have a very limited stack size, which is very valuable. It is hard to keep track of stack use and easy to get stack overflows.

File scope variables ("globals") aren't allocated on the stack however, so they don't contribute to stack size. Also, they have a fixed address so you can have a look at them with a debugger at any point, since they are persistent throughout the program execution. For this reason, you'll want to allocate large variables such as arrays, buffers, structs etc at file scope (with static). This goes completely against "as local as possible".


Also how can we make judgement whether we need to use volatile or not?

See this.


In terms of speed and efficiency...

You have already killed everything that resembles efficiency elsewhere.

  • Here: Serial.print.
  • Here: unsigned long data
  • And here: float angle = (angle_count * 360UL) / 8192.0

These are huge bottlenecks that you should spend your programmer energy on, rather than pondering about minor details such as at which scope to allocate variables.

Serial.print is a blocking call waiting on some lousy UART. Instead of busy-waiting for that line, your little AVR could be executing many thousands instructions. If you have debug/lab stuff like this present in your program, don't even bother to think about efficiency.

As for unsigned long, the AVR is one of the least efficient MCUs still produced. It is not a PC. Like any 8 bitter, it will produce large amounts of overhead code whenever it has to deal with 32 bit numbers. We are talking about hundreds of machine instructions per operation. Therefore you should avoid 32 bit numbers when possible.

Similarly, the AVR does not have a FPU, so using floating point is a huge mistake. All floating point operations have to be carried out with software floating point libraries. If you thought 32 bit numbers yielded tons of overhead, that's nothing compared to software floating point. You should never use floating point on anything less than a Cortex M3, or you picked the wrong MCU for the task.

Because contrary to popular PC programmer belief, floating point does not mean "I need decimals". It means: I need advanced mathematical calculations such as trigonometry, square roots, non-linear math, exponential/logarithmic etc.

A calculation such as (angle_count * 360UL) / 8192.0; could be carried out on fixed point by multiplying the left operand with the desired resolution, then dividing with an integer. For example (10000ul * angle_count * 360ul) / 8192ul; gives you a 5 digit resolution. If you for some reason need to print a decimal comma to some user after a few digits, then divide/modulo the number accordingly.

All professional small embedded systems works in this way - with fixed point arithmetic.

If you actually need more advanced math, you picked the wrong MCU for the task - should have been a Cortex M3 or heavier. Yes you can do advanced stuff with 8 bitters... I have written things like radio signal decoding and PID regulator systems with 8 bitters, all in fixed point. And it was needlessly complicated and painful compared to using the right tool for the task. 32 bitters used to be expensive, but that argument died some 10 years ago.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I would even go as far as writing (10000ul * angle_count * 360ul) / 8192ul; as (3600000ul * angle_count) >> 13;. Though maybe current compilers to that by themselves. \$\endgroup\$ – Swedgin Jan 10 at 15:56
  • \$\begingroup\$ @Swedgin If your optimizer cannot fold constants and reduce the strength of unsigned division by powers of 2 into bit shifts, then you need to throw away that compiler and get a new one. Optimizers have been able to this for decades; it's not a new feature. \$\endgroup\$ – Cody Gray Jan 11 at 1:33
  • \$\begingroup\$ "File scope variables ("globals") aren't allocated on the stack " That's true, but they contribute to the heap, which grows from the other direction of the memory and thus also limits the maximum stack size \$\endgroup\$ – yar Jan 11 at 2:25
  • \$\begingroup\$ @Lundin The reason I use long because I need an array of bits which can store 26 bits. That was the only variable type. I could use Serial.write or send as binary but Im restricted to send the data in ASCII format. And the float is necessary because two decimals precision needed. I wish I could have time to learn and try M3 of ARM. \$\endgroup\$ – ty_1917 Jan 11 at 14:19
  • 1
    \$\begingroup\$ @ty_1917 uint8_t [4] works just fine for that and will yield much better code. \$\endgroup\$ – Lundin Jan 13 at 7:52
1
\$\begingroup\$

Note: Wars were led about the "correct" placement of variables. ;-)

Anyway, some common sense went into style guides and coding regulations. To answer your questions:

  • Place variables at the most inner place possible. Reasons:
    • It is hard to grasp when you need to look at two (or more) distant places in the sources.
    • The compiler might have more chances to optimize: speed and efficiency.
  • You need volatile if you don't want the compiler to take shortcuts and cache the read value. (This is one good reason, but there is more to volatile.)

The placement of variable declarations has no impact on stability.

To judge about different styles concerning speed and efficiency, there is just one safe way: Try every alternative, look at the resulting machine code, test, take measurements. After a while you'll get a "gut feeling" for your given compiler and target platform. However, this might not hold true for other compilers and platforms.

| improve this answer | |
\$\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.