Your dream ECU

Reply Subscribe

Thread Tools

Search this Thread

Mar 15, 2013 | 03:00 AM

#61

rb26dett

Junior Member

Joined: Feb 2007

Posts: 269

Total Cats: 92

From: 11368 miles from where i would like to be

Quote:

Originally Posted by Jeff_Ciesielski

What I will say is that in my experience, if the maintainers are diligent, community driven projects actually tend to produce MUCH better code than commercial solutions.

<snip>

Having 150 people tell you that your code looks like **** on the internet makes you shape up quick if you want it to be included

There are a lot of problems with the MS codebase.

1) I shouldn't have to dig through 1300 lines of global variables and memory map structures to get to the definition of main() in a file called ms2_extra_main.c.<snip>

2) WHY THE **** ARE THERE SO MANY GLOBAL VARIABLES.<snip>

3) Your main loop shouldn't be 2000 lines long. Y U NO FUNCTION!?! <snip>

4) There is no layering. Good embedded software is layered (in fact, all good software is layered). <snip>

5) They ~~over~~use GOTO statements.<snip>

6) They use assembly in completely unnecessary places.<snip>

BIG +1, you very clearly know what you're talking about! I appreciate that. Most don't.

Standard ms function sig these days is void abc(void) :-) Hilarious. Unless you're a maintainer, or user.

Fred.

Mar 15, 2013 | 03:20 AM

#62

Joe Perez

Boost Pope

iTrader: (8)

Joined: Sep 2005

Posts: 34,402

Total Cats: 7,523

From: Chicago. (The less-murder part.)

Quote:

Originally Posted by Jeff_Ciesielski

6) They use assembly in completely unnecessary places. I get why you might use it in a timing critical interrupt, but there is some stuff that just doesn't require it.

Irony: my feeble old mind finds the MS1 code much easier to comprehend than the new stuff, since it's all written in assembly as a single, monolithic listing. No need to go chasing through a dozen different .c files while attempting to trace the flow of the code through a typical cycle.

Modular high-level code is easy to create and a nightmare to debug.

Mar 15, 2013 | 03:42 AM

#63

blaen99

Elite Member

iTrader: (6)

Joined: Sep 2010

Posts: 3,611

Total Cats: 25

From: Seattle, WA

Quote:

Originally Posted by Joe Perez

I dunno if I'd call C "high-level" code anymore Joe, but you are right about one thing - the rise of modern languages seem to have primarily given a new generation of programmers entirely new and novel ways to write spaghetti code.

Mar 15, 2013 | 11:36 AM

#64

JasonC SBB

Elite Member

Joined: Jul 2005

Posts: 6,420

Total Cats: 84

Quote:

Originally Posted by Jeff_Ciesielski

There are a lot of problems with the MS codebase.

1) I shouldn't have to dig through 1300 lines of global variables and memory map structures to get to the definition of main()

...

2) WHY THE **** ARE THERE SO MANY GLOBAL VARIABLES.

3) Your main loop shouldn't be 2000 lines long. Y U NO FUNCTION!?!

4) There is no layering.

5) They overuse GOTO statements.

6) They use assembly in completely unnecessary places.

I never delved into the MS code, so I didn't realize the above. Early in my working life I've programmed in C for embedded systems. I'm a hardware, not a software weenie (my wares are hard), but your list describes newbie mistakes (I knew enough to avoid), or the classic "hardware engineers writing code" problem.

- Why do you think the devs fell into that?
- Maybe the MS3 is much better?

Mar 15, 2013 | 11:51 AM

#65

vtjballeng

Junior Member

iTrader: (1)

Joined: Feb 2011

Posts: 342

Total Cats: 24

From: Richmond, VA

Quote:

Originally Posted by blaen99

Haha, Simulink (or other visual abstraction) -> C -> Assembly is the high level tool that is both intriguing and messifying at the same time depending on the assembler. The problem I usually see when someone can't work with the lower level code to be able to determine timing for various events. Will that water pump interrupt delay an ignition event? That is something you need to know and deal with or you just get lucky and it still works or the periodic offset is either insignificant or never identified. Who would notice if you lose 10 degrees of timing for less than a second every time the electronic water pump cuts on?

Quote:

Originally Posted by Reverant

I agree, however the problem with CAN is that its not suitable for time critical applications. For example, if you want to activate a high-current device at a specific crank angle, you can't. Or to put more scientifically, you can try, but you are not guaranteed to apply it in the correct time frame.

That is part of why those devices have switched inputs which allows for direct io triggering. So that problem has been dealt with. New boxes had to be developed for gdi as an example because of the new injector requirements. Usually you put the PDM on it's own bus or a bus with very low % utilization for the exactly the reason you stated. It is too bad CAN is not deterministic and Flex-ray or some deterministic bus is still not widely adopted.

Quote:

Originally Posted by Jeff_Ciesielski

I can't comment on that as all of the community projects I'm involved in are related to embedded systems. What I will say is that in my experience, if the maintainers are diligent, community driven projects actually tend to produce MUCH better code than commercial solutions. People tend to get in a tight feedback loop in their company, with no external criticism, and commit bullshit. Having 150 people tell you that your code looks like **** on the internet makes you shape up quick if you want it to be included

I think embedded and places where you have heavy professional involvement (Linux) tend to clean up issues for the above stated reasons. Also, anyone working in embedded is already working at a higher level than many web coders imo. My worst case example is osCommerce where just anyone starts dumping code into modules and there is no good feedback loop for poor coding because there is so little professional involvement. I could go on for a while on this one but basically a lot of people who have never coded anything before in their lives start to dump into things like credit card modules or how to store customer data leading to an absolute clusterfuck at every level.

Mar 15, 2013 | 12:41 PM

#66

codrus

Elite Member

Joined: Mar 2007

Posts: 5,298

Total Cats: 884

From: Santa Clara, CA

Quote:

Originally Posted by blaen99

C was originally described as a "portable assembly language". It's low-enough level that it is possible to write systems code in it (because it gives you raw access to read and write arbitrary values to arbitrary memory locations, unlike things like Java) but it's not tied to any one particular processor architecture. It's the reason why operating systems that run on more than one type of processor exist.

I've looked at the MS code once or twice, and yeah, it's pretty ugly.

--Ian

Mar 15, 2013 | 02:10 PM

#67

Jeff_Ciesielski

Elite Member

iTrader: (10)

Joined: Oct 2008

Posts: 1,770

Total Cats: 31

From: Rhode Island

Quote:

Originally Posted by JasonC SBB

Software weenie? PSHHHHH.

Quote:

- Why do you think the devs fell into that?

Well without being privy to the history of the development aside from the sparse changelog at the top of the file, I can't say exactly (that's another pet peeve, they should really think about GIT or something...). But from what it looks like, MS2 started as a 1:1 port of MS1, which was in assembly. I'd be willing to bet money that they just ported it in the most exact fashion possible, which would be to use goto statements and large blocks of readily accessible data at the top of the file. This would speed up development since its easy to feature test ("does it work the same? Yep"), but it hurt maintainability in the long run.

Can't speak to MS3 as I've not seen the code.

Quote:

That's more of an issue of tooling IMO. Emacs + cscope = easy symbol lookup. Its easy to jump around and trace the flow of code. Most people's brains only work in small chunks, much easier to understand a short function and move on than to try figuring out how that large column of assembly.
Example: on startup, a cortex M3 walks through the data section of the binary and copies the contents from flash to ram.

This is the standard STM CMSIS implementation:

Code:

Reset_Handler:	

/* Copy the data segment initializers from flash to SRAM */  
  movs	r1, #0
  b	LoopCopyDataInit

CopyDataInit:
	ldr	r3, =_sidata
	ldr	r3, [r3, r1]
	str	r3, [r0, r1]
	adds	r1, r1, #4
    
LoopCopyDataInit:
	ldr	r0, =_sdata
	ldr	r3, =_edata
	adds	r2, r0, r1
	cmp	r2, r3
	bcc	CopyDataInit
	ldr	r2, =_sbss
	b	LoopFillZerobss
/* Zero fill the bss segment. */  
FillZerobss:
	movs	r3, #0
	str	r3, [r2], #4
    
LoopFillZerobss:
	ldr	r3, = _ebss
	cmp	r2, r3
	bcc	FillZerobss
/* Call the clock system intitialization function.*/
  bl  SystemInit 	
/* Call the application's entry point.*/
	bl	main
	bx	lr

And this is mine:

Code:

void reset_handler(void)
{
	volatile unsigned *src, *dest;

	for (src = &_data_loadaddr, dest = &_data; dest < &_edata; src++, dest++)
		*dest = *src;

	while (dest < &_ebss)
		*dest++ = 0;

	/* Call the application's entry point. */
	main();
}

Assembly is fine when you need to do something that doesn't have a compiler intrinsic (getting the stack pointer on a cortex M3 for example) but in modern microcontrollers that have very complicated peripherals, dma, etc. you pretty much need to layer or perish. This aint your daddy's 8051

Quote:

Modular high-level code is easy to create and a nightmare to debug.

I disagree. Properly layering code means that you can easily isolate problems in their specific layer and can fix them in a way that fixes it universally across the codebase. This is a place where proper documentation of your code is key (and I don't mean comments). Having proper state diagrams and architecture layouts is key to understanding the layer separation and fixing problems quickly.