Struct error help

31 May 2011

Hey everyone.

I am having a strange error that I found a work-a-round to get by but its puzzling me and maybe I'm missing something.

I declare a structure before the main() in my mbed project as shown below.

struct vars {
    float A;
    float B;
    float C;
};

vars Coeff;

Then in the main() section before the while(1){} I declare their values as shown below.

Coeff.A= 9.21735E-04;
Coeff.B= 2.23015E-04;
Coeff.C= 1.21607E-07;

Then in an SPI interupt I use the values Coeff.A, Coeff.B, Coeff.C and it works fine until I get a serial RX interrupt and then it somehow changed the values of my Coeff.A, Coeff.B, Coeff.C and the results from the calculation in the SPI function is wrong based on the changed values.

If I put the following code IN my SPI interrupt routine then everything works as should and values are always correct even after serial Rx interrups.

void SerialInt(){
    Coeff.A= 9.21735E-04;
    Coeff.B= 2.23015E-04;
    Coeff.C= 1.21607E-07;

....etc
31 May 2011

Or if I put the following inside the while(1) it works also every time a serial interrupt happens.

    Coeff.A= 9.21735E-04;
    Coeff.B= 2.23015E-04;
    Coeff.C= 1.21607E-07;
31 May 2011

Interrupts are always fun, use volatile :-

volatile vars Coeff;
31 May 2011

Andy

volatile did not allow me to set the coefficient values in the main and have it work either. There is absolutely nothing else in the program that modifies the Coefficients so I have no idea why they change when a serial RX interrupt happens.

Eventually the values of Coefficients will be changeable so I dont think I can go volatile anyway.

Any other thoughts?

31 May 2011

Matt,

Try this:

struct vars { const float A; const float B; const float C;

vars() : A(9.21735E-04), B(2.23015E-04), C(1.21607E-07){} };

vars Coeff;

Harry

01 Jun 2011

Hi

Volatile is a keyword that instructs the compiler to NOT cache the value in a register. It is mostly used when vars are memory mapped onto hardware and the compiler cannot know the value can change outside the knowledge of the app.

For example if you optimize a construct like while (port) {} where port is actually a i/o port, declaring it volatile means that the port will be read instead of the value being optimized (and not changing anymore because it's cached in a register). So volatile would be used for things like the handshake lines of serial comms etc.

Normally you only need to use volatile if the compiler optimizes a lot (but it won't hurt).

As for your problem it almost seems like a buffer overrun in your RX routine leading to overwriting data. Perhaps you should padd your struct with a couple of Kb arrays on either side and see if it behave better (or examine the RX interrupt code carefully).

Declaring it const is probably only fighting symtomes, not removing the cause. If it's an overrun problem it will hapily popup in the next variable in memory.

Wim

02 Jun 2011

I had a similar overrun type thing happen once before on an STM103 ARM where my code was clobbering memory. I fixed it by increasing the stack size in the startup.s (<-think that was the file). Anyway it seems strange that an overrun of a serial interrupt would spill into other memory.

Here is all that is done in my interrupt.

void Serialint() {
....
}

and in my main() I call a parse routine when the sFlag=1 as below

void parse() {
    sFlag=0;
    char *pos;
....code code code here

     for (int q=0;q<21;q++) {
        buff[q]='\0';
    }
}

I think the last for loop looks suspitious since the buff[] variable only has 9 in it. Thats probably the problem.

01 Jun 2011

Hi Matt,

Your code is potentially very dangerous for a interrupt. Some remarks:

1) If you do not get a CR within the length of the buffer you will go outside the buffer.

You should not only check on a CR but also on the value of sB!

if (pc.readable()) {
        buff[sB]= pc.getc();
        if (buff[sB]==13) {
            sFlag=1;
            buff[sB]='\0';
            sB=0;
        } else {
            //sB++; 
            //BETTER WOULD BE 
            if (sB<sizeof(buffer)-1) {sB++;}
        }
    }

This way you always get your CR/13 but might loose the characters that are not fitting the buffer.

2) If you do not disable/re-enable the irq you could in theory have nested irq calls which is also bad for this kind of routines. A real serial port is probably not fast enough but the USB port is different matter.

3) Finally if you receive a CR and you start parsing the buffer could in theory already be overwritten with new data.

4) C is a language known for speed, not for safety checks on array bounds etc. So spilling does not suprise me in C.

Wim

01 Jun 2011

The problem was the for loop going beyond the buff[9] with the buff[q] where q was up to 20. I'm not sure why the compiler didnt flag this as an error since the buff[] was defined as only having 10 elements so why did it let me write to elements 10-20 in the for loop. Oh well mystery solved!!

Thanks everyone.

01 Jun 2011

Thanks Wim

I will add your suggestion to protect against going over the buffersize.

So you are saying that I SHOULD have the disable irq at the start of the routine and re-enable at the end of the routine?

Matt

01 Jun 2011

Hi,

Normally diabling or not it should be no problem if interrupt handlers are much faster than interrupts occur.

If you disable you could miss an inteerupt but do not have re-entrancy problems.

I do not use the serial interrupt but use good old polling in the main loop. Other interrupts I do not disable either (just looked at my code).

As for the C compiler not flagging the overrun, it's run-time When it happens and not compile time. The compiler cannot know you type a string so long it does not fit.

You could however try to run good old c-lint on it and see it's complaints.

Again C does not do much run-time checking to keep it fast (this in contrast to some other languages). Personally I prefere both a complaining compiler and run-time checks.

But mBed already left out C++ Exceptions so I guess the rest is gone too.