uint8_t my_data[256000]; Compiles?

It might well be that you have some kind of memory leak somewhere in your code.
When you allocate some memory - explicitly or implicitly - and fail to free it up propperly once you’re done using it, then your mem just keeps melting away bit by bit, till you run into some kind of error.

For this reason it is crucial to know what happens behind the szenes - especially when things happen implicitly.

Great thread everyone. One of the bigger issues this raises for the Spark team is: how much should we protect the user from themselves?

As many have suggested on this thread, embedded programming has lots of pitfalls, and to some extent it is up to the developer to watch out for them. At the same time, there is an expectation of simplicity, and man, wouldn’t it be nice to live in a world where, if you seriously f*** up your code, it won’t kill the Core and your ability to reprogram it wirelessly?

This is especially true once people start managing fleets of devices out in the field. If pushing out a firmware update to the fleet could mean bricking the whole fleet, that would create a lot of fear and apprehension about updates, but the world we want to see is one where the embedded/hardware world looks more like the web world, and products can become a little more fluid and constantly upgraded.

There are a number of possible solutions here - virtual machines, watchdog timers, RTOSes - that have been discussed on other threads, and that each have their own pros and cons. I’m not sure what the right answer is, but it is something that we are actively thinking about on the team.

3 Likes

@ScruffR, here is the strange thing about it. When i store the font data in processors flash and pull it from there it runs fine. when i try to pull it from the external onboard flash is when it gets wiggy. The only difference in the code it that i am swapping / adding this 2 two lines to get the data from external rather from internal. I think i have to stop talking about this here before someone yells at me because i am talking about something else. I will create a new post for this possibly when i have more info.

uint8_t my_data[128];
sFLASH_ReadBuffer(my_data,data_pointer,64);

For dead updates one additional feature looks not too hard to implement and would solve lot of cases cases: add button combination for “revert to latest working firmware” where working firmware is detected by configurable parameter “if code is not dead after N seconds, this new firmware is marked OK”

Current firmware reset feature is doing too much - it erases wifi credentials(most critical part here) and older firmware as well.

I would not really protect user too much :smiley: More software checks in core firmware mean less space for user code and less flexibility.

@seulater, I have not looked into sFLASH_ReadBuffer, but does your problem occure immediately or only after several calls to the function containing these lines of code?

As for my foo functions above one possible problem roots in the fact that passing the pointer to an automatic might either prevent implicit freeing of the allocated memory or receiving a pointer the now points to som memory that might actually already contain some other functions data.

If one of the above is the case with your code it’ll be hard to nail down without full examination of the code.

@ScruffR, Its seemingly random. Sometimes it will be a couple seconds after its starts and other times it will run for a minute. In the loop() i just have a counter that increments and i print that value to the LCD. Every time it stops the LCD number is different. Just so you know the call i showed "sFLASH_ReadBuffer(my_data,data_pointer,64);"
i am not passing a pointer in there. ‘data_pointer’ is uint32_t data_pointer; User flash starts @ 0x80000, so on start data_pointer is set to this value and then incremented such as data_pointer += 64.

I know without seeing the whole code it’s just about impossible to help me. If i do one of two things it will not lock up.

A) comment out the “sFLASH_ReadBuffer(my_data,data_pointer,64);” in my LCD routine.
or
B) change main.cpp so it will bypass all the wifi stuff as i shown before.

@seulater, sorry but I can’t really see what’s going on there - not even after having looked into sFLASH_ReadBuffer :frowning:

But just by the beginning of this thread, I suspect that you might have your uint8t my_data[128]; line inside of loop. While this should not be a problem - since loop() should not be called recursivly or multiple instances live parallel at any time - it still might be worth a try to declare a global buffer and reuse this instead.

Have you got any idea if your code stalls always at the same place or completely random?

I did try a couple things with no better results. First i made uint8t my_data[128]; static, then i moved it outside the loop making it global with still no difference.
This is what my loop function looks like.

char lcdstring[50];

void loop()
{
    if(++seconds == 60)
    {
        seconds = 0;
        if(++minutes==60)
        {
            minutes=0;
            hours++;
        }
    }

    sprintf(lcdstring,"%02u:%02u:%02u",hours,minutes,seconds);

    Serial.println("A");
    LCD_String(lcdstring,0x80000,20,120,7,0xF4df,0x0000);
    Serial.println("B");
}

You will notice that i put a serial.print before my ‘LCD_String’ call and after. every time it has locked up it been after i see ‘B’ in the terminal window. Its not locking up in my function but after it leaves the loop().

Looking at your snippet I could not see any reason for crashing - assuming all variables have been set propperly in setup(). For my part I’d rather test >= 60 rather than == 60 to avoid missing 60 if seconds/minutes might get changed somwhere else - it might be slightly slower but code resilience goes first for me.

On the other hand the fact that you see the “B” output does not necessarily proof that there is no problem in your LCD_String() function.
As mentioned above, having stray pointers might cause corrupted bytes in “someone elses” memory location that can cause otherwise clean code to stumble.
So without having the whole picture there is no way of tracking this down.
Sometimes it’s just a case of a one iteration overshoot in a loop or the wrong data type on pointer increment.
Maybe you add some more println to check start/end states of loops and pointers against their expected values.

I agree with everything you mentioned. As for the test>=60, yes on a released product that is the only way to go. For this quick and dirty test its not critical. The source for the LCD code I imported has been proven for many years in the field. Its not something that i just whipped up for this. This same code has been run on many different platforms even ones with less ram than the spark core has. I know its solid code. As you can see by the products we make. www.versamodule.com. I realize that for anyone else here it is impossible for them to diagnose the problem without seeing the full LCD source code. I do thank everyone for trying though. I have been placing more println in their code for tracking down their call in main of SPARK_WLAN_Loop(); What i really need to do is get a debugger working.

The only thing changed in the LCD source is me reading the data from a different source aka “sFLASH_ReadBuffer(my_data,data_pointer,64);” my (now) global variable of uint8t my_data[128]; is double what i can read from sFLASH_ReadBuffer which is fixed at 64 bytes. data_pointer variable is a uint32t and is incremented +=64 on every next pass. even if that value went off into the abyss somehow i would just read wrong values from flash. Like i mentioned, if i comment out this one line of sFLASH_ReadBuffer it does run fine. or if i comment out their SPARK_WLAN_Loop(); I just need to track it down as time permits.

seulater, any chance you can share the code for LCD_String()? Reading from external flash may be conflicting with the core firmware access to the flash so that when loop() hands off controls to the core firmware, it manifests itself as a hang.

Here is the bulk of the code. FWIW i just discovered that if i use this in my code it does not hang up right away it now runs for somewhere in the neighborhood of a couple minutes, then starts updaing the screen slower and slower until finally it stops. whereas before it would only run for about 1-30 seconds. This is not the right way to do it, but its now causing me to look at what other IRQ’s may be firing causing it to break away from my code.

NVIC_DisableIRQ(CC3000_WIFI_INT_EXTI_IRQn);
LCD_Strings(lcdstring,0x80000,20,120,7,0xF4df,0x0000);
NVIC_EnableIRQ(CC3000_WIFI_INT_EXTI_IRQn);

uint16_t hours,minutes,seconds;

// Used for string data to lcd
char lcdstring[50];

//*******************************************************
// 	Name:		GotoXY(x,y)
// 	Copyright:	Free to use at will & at own risk.
// 	Date:		10.01.12
// 	Description:    To set x,y location registers
// 	Usage:		GotoXY(param 1,param 2)
//
//			param 1: x location on screen
//			param 2: y location on screen
//
//	Example:        GotoXY(x,y)
//  Notes:		No bounds checking
//******************************************************
void GotoXY(uint16_t x, uint16_t y)
{
    SPI_WriteCMD(0x2a);    // column set
    SPI_WriteDAT((uint8_t) (x >> 8));
    SPI_WriteDAT((uint8_t) x );
    SPI_WriteDAT((uint8_t) ((DISPLAY_WIDTH-1) >>8));
    SPI_WriteDAT((uint8_t) (DISPLAY_WIDTH-1));


    SPI_WriteCMD(0x2b);   // page address set
    SPI_WriteDAT((uint8_t) (y >> 8));
    SPI_WriteDAT((uint8_t) y);
    SPI_WriteDAT((uint8_t) (DISPLAY_HEIGHT-1) >>8);
    SPI_WriteDAT((uint8_t) (DISPLAY_HEIGHT-1));

}

//*******************************************************************
//  Name:       LCD_String
//  Copyright:  Free to use at will & at own risk.
//  Date:	10.01.12
//  Desc:    	To print a string of characters to LCD
//  Usage:	LCD_String(param 1,param 2,param 3,param 4,param 5,param 6,param 7,param)
//
//              param 1: String data
//              param 2: Location in external flash.
//              param 3: x start direction: 0-x max
//              param 4: y start direction: 0-y max
//		param 5: char to char spacing in pixels. Pads extra pixels between chars to space them apart
//      	param 6: Font Color, 5-6-5 format
//          	param 7: Background color, 5-6-5 format
//
//  Example:    sprintf(lcdstring,"1234567890");
//		LCD_String(lcdstring,0x80000,10,10,5,0x04df,0x0000);
//
//
// NOTES:            No x & y bounds checking
//
//*******************************************************************
void LCD_Strings(char *lcd_string, uint32_t font_pointer, uint16_t ux, uint16_t uy, uint8_t char_spacing, uint16_t fcolor, uint16_t bcolor)
{

	uint16_t y, y_total, mask, x, data_start, height, width, po;
	uint8_t l, m, h, px, dt_start;
	uint32_t data_pointer = font_pointer;
	static uint8_t my_data[512];

	// Get the height value for the font
	data_pointer +=6;

	sFLASH_ReadBuffer(my_data,data_pointer,1);
	height = my_data[0];

	// Initialize y
	y=0;

	do
	{
		// Check to see if we received a space char
		dt_start = (*lcd_string - 32);

		// Point to the start of the font data in the table
		data_pointer = font_pointer;

		// point to the data in the table for this font chars information
		data_pointer += (dt_start * 4) + 7;

		// get the font information
		sFLASH_ReadBuffer(my_data,data_pointer,4);
		h = my_data[0];
		width = my_data[1];
		l = my_data[2];
		m = my_data[3];


		// Find out where in the table the actual data is for this font char
		data_start = (h << 16) | (m << 8) | l;

		// Now point to the start of the actual data for this font char
		data_pointer = font_pointer + data_start;


		for(y_total=0;y_total<height;y_total++)
		{
			// Initialize the Mask bit for the data
			mask = 0x01;

			// Set new x,y location for LCD
			GotoXY(ux,(uy + y));

			// Tell LCD we are going to be sending data now
			SPI_WriteCMD(0x2c);

			po=0;
			sFLASH_ReadBuffer(my_data,data_pointer,width);
	
			// Bring CS pin low
			PIN_MAP[SS].gpio_peripheral->BRR = PIN_MAP[SS].gpio_pin;


			// Set or clear each pixel on the LCD in the x axis
			for(x=0;x<width;x++)
			{
				// if pixel data then put dot on lcd
				if(my_data[po] & mask)
				{
					SPI.transfer((uint8_t) (fcolor >> 8) );
					SPI.transfer((uint8_t) fcolor );
				}
				else	// else use background color
				{
					SPI.transfer((uint8_t) (bcolor >> 8) );
					SPI.transfer((uint8_t) bcolor );
				}

				// Shift up one bit
				mask<<=1;

				// If we have done all 8 bits then reset back to 1
				if(mask == 0x100)
				{
					mask=0x01;

					// Also point to the next data byte
					data_pointer++;

					po++;
				}
		}

		// This keeps the background color in-between chars
		for(px=0;px<char_spacing;px++)
		{
			SPI.transfer((uint8_t) (bcolor >> 8) );
			SPI.transfer((uint8_t) bcolor );
		}

		PIN_MAP[SS].gpio_peripheral->BSRR = PIN_MAP[SS].gpio_pin;

		// whenever x winds up being the same as a byte size (I.E. 8,16,24,32,40...) then
		// then the if(mask == 0x100) increments data_pointer on the exit
		// when we dont want it to. so this backs it up if that was the case.
		if(x % 8 != 0 )
		{
			// Set pointer for next data byte
			data_pointer++;
		}

		// Move down one pixel the y axis
		y++;
	}

		// Add the width of this font char + the user pixel spacing
		ux += width + char_spacing;

		// Thus char is done so reset the y counter
		y=0;

		// next character in string
		lcd_string++;

	}while(*lcd_string !='\0');	// keep spitting chars out until end of string


}

//******************************************************
//******************************************************
//******************************************************
void loop()
{

    if(++seconds == 60)
{
		seconds = 0;
	    if(++minutes==60)
	    {
			minutes=0;
		    hours++;
	    }
	}

	sprintf(lcdstring,"%02u:%02u:%02u",hours,minutes,seconds);

//      NVIC_DisableIRQ(CC3000_WIFI_INT_EXTI_IRQn);
    LCD_Strings(lcdstring,0x80000,20,120,7,0xF4df,0x0000);
//      NVIC_EnableIRQ(CC3000_WIFI_INT_EXTI_IRQn);


}
1 Like

I’m not sure if this might actually be related to your problem, but I have no good feeling about the line

dt_start = (*lcd_string - 32); 

as I can’t see any code ensuring that dt_start doesn’t results in a negative number (e.g. for \n, \r or so) for further pointer arithmetic, resulting in reading locations where you won’t find the expected data.

i could change it to this

if(*lcd_string >= 32)
{
	dt_start = (*lcd_string - 32);	
}

but it wont go negative because its a unsigned int. Also, even it it did read data from the wrong point in external flash ram it would just print gibberish on the lcd. I am leaning toward some IRQ firing in the middle of a read or write is the culprit.

Oops :blush: I missed the unsigned bit.

But to make a complete fool of me :wink: I’d like to put my finger on one other thing that does not have anything to do with your problem, but might result in some unwanted behaviour of LCD_String().
When you happen to pass in a pointer to a supposedly empty string (which starts with \0) the do { } while will enter and therefore produce unwanted output - especially in case of an automatic variable there will be unpredictably many non-zero bytes that will be spat out.

Ahhh… no sweat, i should have wrote it better right there anyway, i.e. should not write the code that uses signed or unsigned as a catch all for problems.

seulater, you can just rewrite the do-while to just a while as ScruffR pointed out. Looking at your code, can you tell me what your character metrics “l” (el) and “m” are? I am trying to figure out how you calculate your font offset. I also noticed there are no bounds check for that data_start calculation.

I noticed in loop(), you refer to seconds and minutes and incremented on each iteration of the loop, which obviously is not every second. Another thing is that you have no delay in the loop whatsoever so you call sprintf/LCD_Strings as fast as the loop() runs. This may overwhelm the display unit and possibly the SPI, since it is shared with the CC3000. You may want to put a non-blocking delay to slow down the calls. :smile:

I can, but keep in mind that none of these are issues here for this example. Since I am sending it the string there is no concern of these conditions. Granted if someone else took this code and send the function with a null that is another thing. Likewise with data bounds check for data_start even if somehow it was off the LCD would just print gibberish.

[quote="peekay123, post:37, topic:3308"]
I noticed in loop(), you refer to seconds and minutes and incremented on each iteration of the loop, which obviously is not every second. Another thing is that you have no delay in the loop whatsoever so you call sprintf/LCD_Strings as fast as the loop() runs.. [/quote]

Yes, this is the result of stripping off code to get to the bottom of things. seconds, minutes & hours being off has no barring on anything i was just using it to increment things on the lcd so i can see a change. There is no delay in loop as i want to do this as fast as i can. as for the overwhelm the LCD, the clock rate of the SPI is not even at 1/2 the max clock rate it can take.

If this all sounds a bit off, please dont take it that way. Its been a very long and frustrating day at work.

HOLD THE BOAT! are you sure about this ? If this is true, i was un-aware of this which changes the whole game plan.

Hi @seulater

As I read the Spark core schematic, the CC3000 Wifi module and the external FLASH share one SPI bus. The user SPI bus is on separate pins. That said, while the new CC3000 driver improves the situation with interrupts and the CC3000, there can still be problems if you are interrupted in the middle of a long transfer from the external flash. I am not sure (and @peekay123 is the expert here) about which timer/prescalers the SPI busses use in core–it could be shared–I’m not sure.

I think of users using sFLASH_ReadBuffer() as experimental at this point: you are blazing a new path!

Maybe you could try disabling the CC3000 interrupts only around the calls to sFLASH_ReadBuffer(); and see if that helps?

Maybe you could also try using const data which comes from the ARM internal FLASH and see if that has the same problems?

I think i got it. Its been running now for 20 minutes whereas before i could not get even a minute of run time. I did the following in the loop(), It’s not a solution i like or even want to use, but it may help in diagnosis.

NVIC_DisableIRQ(CC3000_WIFI_INT_EXTI_IRQn);
NVIC_DisableIRQ(CC3000_SPI_RX_DMA_IRQn);
NVIC_DisableIRQ(CC3000_SPI_TX_DMA_IRQn);

LCD_Strings(lcdstring,0x80000,20,120,7,0xF4df,0x0000);

NVIC_EnableIRQ(CC3000_WIFI_INT_EXTI_IRQn);
NVIC_EnableIRQ(CC3000_SPI_RX_DMA_IRQn);
NVIC_EnableIRQ(CC3000_SPI_TX_DMA_IRQn);