Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to build a custom firmware with my own reflow profiles but failing, what am I doing wrong? #226

Open
dburr opened this issue Mar 18, 2022 · 14 comments

Comments

@dburr
Copy link

dburr commented Mar 18, 2022

Hi, I'm trying to build and install a custom version of this firmware with my own reflow profiles "baked in" (pun intended ;-) ) Unfortunately so far I have not been successful. All my attempts result in a firmware that bootloops. If I take my changes out, and build the unmodified firmware as it exists in git, then my build is successful.

In reflow_profiles.c I create a new profile like this

static const profile mytestprofile = {
	"TEST", {
		 50, 50, 50, 60, 73, 86,100,113,126,140,143,147,150,154,157,161, // 0-150s
		164,168,171,175,179,183,195,207,215,207,195,183,168,154,140,125, // Adjust peak from 205 to 220C
		111, 97, 82, 68, 54,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0  // 320-470s
	}
};

then I add it to the static const profile* profiles[] array like so:

static const profile* profiles[] = {
	&mytestprofile,
	&nc31profile,
	&am4300profile,
#ifdef RAMPTEST
	&rampspeed_testprofile,
#endif
#ifdef PIDTEST
	&pidcontrol_testprofile,
#endif
	(profile*) &ee1,
	(profile*) &ee2
};

but like I said, any time I try this, I get a firmware that bootloops, and all I see on the serial console is

                                                       
T-962-controller open source firmware (v0.5.2-4-g217281c-dirty)
                                                            
See https://github.com/UnifiedEngineering/T-962-improvement for DABT@00000a38
T-962-controller open source firmware (v0.5.2-4-g217281c-dirty)
                                                            
See https://github.com/UnifiedEngineering/T-962-improvement for mDABT@00000a38
T-962-controller open source firmware (v0.5.2-4-g217281c-dirty)
                                                            
See https://github.com/UnifiedEngineering/T-962-improvement for mDABT@00000a38
T-962-controller open source firmware (v0.5.2-4-g217281c-dirty)

See https://github.com/UnifiedEngineering/T-962-improvement for mDABT@00000a38

[and so on, and so on...]

And like I said, if I take my changes out, and build the unmodified firmware as it exists in git, then my build is successful. So something I am doing code-wise is causing this. But I am completely at a loss as to what I could be doing wrong. What am I doing wrong?

@sevstels
Copy link

You can't add anything extra to the code. Because of the "childish" style of writing the program, the RAM memory is full of useless garbage. For example string constants. So if you add anything else to the RAM, it will overflow and crash the program. If you need just a little extra memory, delete unnecessary text messages.

In your case, just replace the existing profile with yours for minimal modification. You should not add any.

@xnk
Copy link
Member

xnk commented Mar 18, 2022 via email

@GitLang
Copy link

GitLang commented Mar 18, 2022

It should not be possible to allocate a static string to RAM. The only business it has in RAM is when it is an auto variable, and then only for the scope of the procedure it's declared in.

@mikeanton
Copy link

When I built a version for my oven the strings were not put into RAM. In fact, RAM only had about 4K+stack in it at the time (2018). However, I built it with GCC directly and not through LPCExpresso, so I don't know if that makes a difference. At the moment, I don't have GCC set up on my computer, or I'd attempt a build of the new version, and look at the map file. Maybe someone else could do this to verify how full the RAM is, and what is stored there. The map file breaks down how much space is used for each file, so it is pretty easy to determine where the large blocks are being consumed.

The linker file very clearly puts the .rodata, and .constdata sections into flash, so I'd be pretty sure that the constants are not also stored in RAM, unless something very odd has happened.

@xnk
Copy link
Member

xnk commented Mar 18, 2022 via email

@mikeanton
Copy link

I just pulled the current master build from git, and compiled it. Here are the results:
Flash: 65544 bytes
RAM: 3472 bytes
So, I'm not sure why data aborts would be seen, given that there is over 12KB of stack space, unless there are some very large local objects placed on the stack somewhere.

Reflow profiles are definitely allocated to flash in .rodata, and that is where the addresses reside.
While the strings in main.c are not declared as const, it doesn't appear that they are in the .data section. In fact, there are only 384 bytes allocated to the initialized data section, and the rest of the RAM is mostly in the .bss section.

So, my conclusion is that @sevstels comments are probably not all that relevant, and that the observed issue is elsewhere. This is likely not due to a lack of RAM, or string constants maybe being located there, which doesn't appear likely.

@dburr
Copy link
Author

dburr commented Mar 19, 2022

OK, I think I've figured it out, and believe it or not, I think it's down to a buffer overflow of some sort.

While perusing the files, I noticed that the Makefile was embedding the build version into the compiled firmware using the command git describe --tag --always --dirty. Well, since I was making my changes locally, and hadn't committed anything yet, this command resulted in a version string like v0.5.2-6-g7c3a4fa-dirty. This seemed a bit long to me -- like it wouldn't fit in the space alootted for it on the About screen. So on a hunch I created my own branch and committed my changes to it, which resulted in a more sane looking (not to mention shorter) version string v0.5.2-7-gcd926ef. And, lo and behold, when I flashed this firmware to my oven, it worked! So, believe it or not, it seems to me that the extra -dirty in the version string embed appears to exceed some boundary (buffer overflow?)

@mikeanton
Copy link

@dburr If that's the case, it seems a bit odd. The sed command that is invoked during that command just builds a version.c file with the version string in it, and returns it as a pointer. There isn't any fixed length storage that is used to allocate that string, and it should also be located in flash. That resulting string is then used as a parameter in a printf call, that results in a longer string, but one that is still far shorter than others, so unless there is a buffer overflow happening in printf then that isn't it. It would be far more of a problem if the terminating null was for some reason missing in that string, but that shouldn't be the case either.

I have seen odd bugs in ARM7 code that show up when code alignment changes. Those are usually caused by something else in the code though, rather than the code that was modified, though that sort of occurrence does indicate that there is a problem somewhere.

@sevstels
Copy link

@mikeanton
I use the commercial IAR compiler and it shows a lot of text variables in the code was placed in RAM. After some tweaking I got the following results:

Optimize: Release, max speed

51477 bytes of readonly code memory
7 649 bytes of readwrite data memory

Errors: none
Warnings: none

Without modifying the code, the ram memory was crammed with string variables. But I didn't clean it all out, I was too lazy.
You can see the ported project > here

@mikeanton
Copy link

Well, that would explain it. On GCC anything declared as const, or a constant string generally gets put into flash. This makes sense. Since IAR handles this differently, it's not really a fault of the code design as you were initially implying. Even with the optimizations you did, it is still worse for RAM usage than a GCC build, though better from a ROM standpoint.

The takeaway here is that different compilers treat the same code differently. Don't assume that the code is "childish" because your compiler treats the code differently than what the compiler it was originally designed for. IAR seems to use a the __flash type address space identifier, or a linker option to force strings into flash. This isn't required with GCC, as it does it in a more standard way for most targets. The concept of a address space identifier like __flash likely comes from the AVR world (and similar targets) where ROM accesses have to be treated very differently (and GCC supports it there as well). This isn't the case with ARM, as it is essentially a flat memory model, so no special accesses are required.

@sevstels
Copy link

You misunderstand the essence of optimization. If optimization is done in terms of speed, then the maximum possible filling of RAM with procedures or variables, which will be accessed faster than from FLASH. That's why in this case the RAM fill is bigger. Not because IAR is worse. On the contrary, commercial compilers, like any commercial thing, are far superior to anything made by artisans. And it's silly not to understand this. ;)

@mikeanton
Copy link

@sevstels Well, to each his own I suppose. I used GCC for many years, and had good luck with it. I've also used a number of commercial compilers. Most commercial compilers have not been remarkably than GCC from my experience. Maybe IAR is one of the exceptions. I think given that this is an open source project, it makes sense that an open source compiler was also used...

I have not looked to see what optimization level was applied in the original makefile. Perhaps it could be better. I do know that one of the large differences between compilers depends on how the library code has been written. There are some much better libraries for use with GCC compared to the standard ones. I've heard that the CrossWorks libraries are very good, but even though it uses GCC as the compiler it would be considered a commercial option.

@sevstels
Copy link

mikeanton, why we should argue about the obvious? Commercial tools - the way to success. I stopped using GCC 25 years ago and don't regret it for a minute. :)

@mikeanton
Copy link

@sevstels I was the opposite. Used commercial tools for over 20 years, and have used GCC for the last 17 (now I just feel old). I found bugs in both commercial and non-commercial products. I'd never go back... :-)

One thing I insist on is using a commercial editor (CodeWright for me even though it's a dead product). I don't know how anyone can stand Eclipse...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants