[C][ARM] changing variables also accessed by an interrupt routine

[Simon]

Atomic writes to a single variable won't cause issues, but probably you're updating more than a single variable (an array of few values, or a text string), thus it behaves very much like a non-atomic operation and it's very probable that at some point the interrupt gets weird data.

Nobody said anything, about what I posted earlier. What's the problem with simply blocking interrupts during these few instructions writing to the variables?

If you can't do that for whatever reason, a flag would do the job, ex. "buffer_busy", set before writing to the LCD buffer, and clear after.
The flag should be checked by the LCD interrupt, and skip LCD update if active.

I have the flag but it has not helped. I have some other issue somewhere.

[DavidAlfa]

Are you updating the buffer between interrupts, or after all chars are sent? Otherwise you might start sending a string,  get updated in the middle, and as result the LCD shows a mix of different strings.
So you might need to set a 4 level status: idle, busy, pending and transferring.
So:
- The update process only executes if in idle state, sets busy before transfer and pending after done.
- Interrupt only proceeds if in pending or transferring state. If pending, sets transferring state. Counts the sent chars, when all were sent It sets idle state to allow new update.

That way there's no way you send random data.

[Simon]

No it's something different and very subtle.

So I use an up/down and select buttons to navigate my interface. I have things like a variable that keeps track of which page I am currently displaying. Now if I press dowwn from page 0 to page 1 that's fine. If I press up I go back to page 0. But if I press the up or down button twice or more I get something odd. the page does not go beyond where it should (array variable so I would get any jibberish), no, but when I go back it takes two presses to go back instead of one. This is no matter how many times more than once I pressed the button. Often when I press the first time I get a flash of the new other page but then it reverts straight back to the current page before this has loaded and I have to press again.

I also get this when I display a variable on the screen that I want to edit. I have a maximum and minimum for that wariable, if I try to exceed the end stop it takes to presses of the oposite button to go back and I sometimes see the incremented or decremented value appear, but then it goes back to the last one and I have to press again.

This is very strange. On the one hand it is the variables themselves that are tested to guard against them being edited. On the other hand these variables are being changed by something like they regain an old value. I really cannot fathom it.

It's not interrupt related and I have turned off all code optimization.

[Siwastaja]

You just need to systematically debug the problem.

For that, you need to add visibility; possible with a debugger, or by mechanisms like storing values in logging variables and print them out.

It sounds your button press detection logic has a bug. Possibly with debouncing.

[nctnico]

Maybe even test the code on a PC combined with using a debugger. That would take the embedded hardware with interrupts etc. out of the loop.

[Siwastaja]

On PC, you can just write test vectors like

int button_a[] = {0,0,0,0,0,0,0,0,0,0,1,1,1,1,0,1,0,1,0,0,0,0,0,0,0,0,0};

Then printf() the heck out of every if-else branch with all variables of interest, and see what happens.

Or you can single-step it in a debugger, no big difference.

Doing it sucks, but then you get the aha! moment and fix it.

Though, the ultimate target would be to learn to write more robust code, so that you actually understand what happens by just looking at that code. I rarely need to resort to tedious brute-force head-banging debug sessions of my program logic / algorithms anymore, although of course it sometimes happens.

[Simon]

Yea, I need to learn to use the debugging. The little I have found just wants to show examples and results rather than describe the tools.

I don't think there are switch bouncing issues:

pushbutton.c

Code: [Select]


const uint8_t longpress = 50 ;              // Long press threshold counts
const uint8_t shortpress = 1 ;              // Short press threshold counts

volatile uint8_t buttonPressedCounts[64] ;           // loop counts button was held down for, one variable per pin
volatile uint32_t shortpressed[2], longpressed[2] ;  // short and long button states, one array variable per port

#define setbit( x, y ) ( x |= ( 0x1 << y ) )
#define clrbit( x, y )  ( x &= ~( 0x1 << y ) )
#define retbit( x, y )  ( x & ( 0x1 << y ) )

void buttonRead(uint8_t button)
{
// If button is pressed increment button pressed counter
if ( !pinRead( button ) )             
{
buttonPressedCounts[button]++ ;
}

// If "the button has been released" && "the press count is between" 0 && "the long press limit"
// reset the press counter and set a button pressed state.

if ( pinRead( button ) && ( buttonPressedCounts[button] <= longpress ) && ( buttonPressedCounts[button] >= shortpress ) )
{
shortpressed[(button >> 5 )] |=  0x1 << (button & 31U) ;
buttonPressedCounts[button] = 0 ;
}

if ( pinRead( button ) && ( buttonPressedCounts[button] >= longpress )  )
{
setbit( longpressed[(button >> 5 )], ( button & 31U ) ) ;
buttonPressedCounts[button] = 0 ;
}
}

uint8_t button_short_press(uint8_t button)
{
if ( shortpressed[(button >> 5 )] & (0x1 << ( button & 31U ) ) )
{
return 1 ; // return 1 for short press
}
else return 0 ;
}


extract from 100ms task routine

Code: [Select]

void page_scroll()
{
if(editMode == PAGE_SCROLL)
{
if ( (button_short_press(BT_DN)) && (currentDisplayPage < 1U) )
{
button_short_press_reset(BT_DN) ;

currentDisplayPage ++ ;

if (currentDisplayPage == MAIN_PAGE)
{
print_current_values() ;
}
}

if ( (button_short_press(BT_UP)) && (currentDisplayPage > 0U) )
{
button_short_press_reset(BT_UP) ;

currentDisplayPage -- ;

if (currentDisplayPage == MAIN_PAGE)
{
print_current_values() ;
}
}
}

if (currentDisplayPage == SETTINGS_1_PAGE) // check to see if display is on edit settings page
{
display_u16(oilPressureThreshold, 3 , OIL_PRESSURE_THRESHOLD_SETTING_ROW , 14) ;
}
}

So "currentDisplayPage" cannot be modified any further, it is it's own guard. But if I press the button more than once when I go the other way it takes 2 presses and "currentDisplayPage " is changing all by itself.

[Simon]

The only think that comes to mind is, is my stack too small? I hardly know what I am talking about here but could I be falling off the edge of something?

[cv007]

Code: [Select]

if ( (button_short_press(BT_DN)) && (currentDisplayPage < 1U) )
{
button_short_press_reset(BT_DN) ;

currentDisplayPage ++ ;

if (currentDisplayPage == MAIN_PAGE)
{
print_current_values() ;
}
}I would presume button_short_press_reset() clears the 'press' bit, so you are not clearing the presses when you are at page limits. Your presses are then stored for later, and then end up using them later, inadvertently.

[Simon]

Yes that is it! The button pressed is held onto and when the other button tries to reverse the action suddenly the condition for the previous press has something to do.

I suppose I need to reset all the button pressed states at the end of the whole 100ms task sequence as if it was not used then it needs discarding. The code is written such that only one thing can use up the button pressed, so it also shortens the code.

[tggzzz]

Divide the solution into two concepts: button presses and what to do when a button is pressed.

Implement and test those two concepts in isolation. Encode the concept of a button being pressed as an "event". Encode why to do when and event occurs as an FSM.

Test the FSM on a PC using synthetic events. Test the events on the target hardware,to see that there is a reliable correspondence between key presses and event generation.

Then couple the events to the FSM on the target hardware. If that doesn't work as expected, the cause ought to be easy to locate.

Don't try to skip those stages.

[Simon]

It is what cv007 said, I just needed a second pair of eyes to see it quicker than me after the week I have spent on writing code. The problem is that I am writing multiple bits of functionality at the same time and as you say they need dealing with in isolation.

What I also needed to establish was the button functionality, do I need multi-press or not, the answer is no so when I write the code totally separately as you suggest I should lock other button presses out by testing the flags to see if any have already been pressed before setting a flag for that button. That way the push button priority is on the first one pressed by the user and not what the program happens to dictate at that time.

[brucehoult]

In my hunt for sample code when working with the non volatile memory controller I found what I think is some microchip code that seems to include it's own version named with a capital M:

Code: [Select]

void * Memcpy(void* dst, const void* src, unsigned int cnt)
{
char *pszDest = (char *)dst;
const char *pszSource =( const char*)src;
if((pszDest!= NULL) && (pszSource!= NULL))
{
while(cnt) //till cnt
{
//Copy byte by byte
*(pszDest++)= *(pszSource++);
--cnt;
}
}
return dst;
}


An absolutely bog-standard small but slow[1] memcpy(). Other than the checking for null pointers, for no obvious reason. No harm.

I assume this is returning a pointer (void * ) it is because "dst" is returned presumably as a pointer to the last location? maybe as a means of calling code checking it ended in the right place?

dst is, quite plainly, still pointing to the first location.

I'm thinking of using that maybe with the returned value removed.

Fine.

Can compiler code optimization cause any issues?

No. For sure not if the function isn't inlined. If it is inlined, then no more so than with whatever code you've got now.

[1] especially without "restrict"

[Simon]

Can compiler code optimization cause any issues?

No. For sure not if the function isn't inlined. If it is inlined, then no more so than with whatever code you've got now.

[1] especially without "restrict"

So is in-lined code an issue with compiler optimizations? I call any inline function a "static inline" and put it in a header file. I usually do it when the contents of the function is one or two lines but the function is just easier reading than the machinations contained. And of course it means I can write hardware specific code for different devices but if I use the same function names it makes life easier when reusing higher level code or just for memory sake of what to write.

What is "restrict"?

[brucehoult]

So is in-lined code an issue with compiler optimizations?

Only to the same extent as if you wrote the same code in the same place by hand. No more and no less. The compiler is, as always, free to remove apparently useless code.

For example code inside an if() where the test is always false, or replace repeated reads of an ordinary memory location by a copy of the first value read, or replace repeated writes of an ordinary memory location by the last value written.

The usual things designed to clean up sloppy code, especially generic code generated from macros or inlined functions that can be simplified in the current situation.

What is "restrict"?

A promise that no other variable or pointer will access the same memory locations as the pointer with "restrict" on it.