Optimization help!

I’m trying to use an arduino IC to help me run a megasqirt ECU on my honda s2000.

The Honda uses an awkward cam and crank signal to tell the ECU where it is

As standard the engine sends out:

Cam pulse
6* crank pulses
Cam pulse
6* crank pulses
Cam pulse
12* crank pulses

Then repeat

But the megasquirt can only handle

Cam pulse
24* crank pulses

So I need to use an arduino IC (only the IC, I’ve not got room for the full board). to count the crank pulses, reset on cam pulses, then since only once in the full engine cycle i’'ll reach 10+ crank pulses if resetting with every cam signal, send a pin high for a couple of milliseconds for the ecu to read

I’m already using interrupts, but i’m not sure if my if functions are an efficient use of the IC’s time, or if there’s a better way to write what i’ve written.

Cheers, chris.

S2000_decoder.ino (1.38 KB)

When the engine sends a cam pulse, why do you reset the number of crank pulses to 0?

When there are supposed to be 24 crank pulses, why do you reset when you get 10 or more crank pulses?

Why is crank NOT volatile, when it is can be changed outside of code that loop() calls?

Sorry, i didn't make myself as clear as I should have.

This decoder is only to provide a single cam input trigger to the ECU, the ecu can't understand the standard trigger setup, so this routine is to provide 1 can pulse per cam revolution (2 crank revolutions). The arduino is only using the crank as a reference, the ECU deals with all the
engine speed and timing calculations (from the crank wheel) but needs 1 signal per cam revolution to index itself so it knows which cylinder is where in their strokes.

I reset the crank count every time the cam pulse rises because I only want the crank int to reach 10 once in the engine's cycle (2 crank revolutions), to give the ECU the indexing cam trigger it needs.

Hopefully I've explained that a little less drunkenly than last night :slight_smile:

When you say crank is not volatile is that because I've declared:

Int crank = 0;

Before the setup loop? I only wanted to declare it, I thought I had to give it a number, can I delete the =0?

Definitely an arduino n00b here sorry.

The code does work, but because it'll be getting aLOT of crank/cam signals very quickly I don't want it to have a more difficult time than it should running the loop.

Volatile is an important keyword. It allows the "big" C++ compiler to work with microprocessors that don't behave like a big computer.

Specifically, the compiler knows exactly what each function does. It can see that the linear execution of the code never changes crank. So it can do some optimization to remove any reference to that variable and just insert a constant.

But the microprocessor executes the code non-linearly. The interrupts can change crank. So you need to let the compiler know that this variable is special.

Note that the Arduino is an 8-bit processor so it takes 2 instructions to read or write a value to the 16-bit crank. An interrupt may occur between those two instructions. That will really screw things up. use noInterrupts() and interrupts() on each side of any attempt to read or write to crank. (Unless you are inside the interrupt itself, where you know there won't be any interrupts.)

If you expect that you will never see a value greater than 255 for crank, then make it a byte or uint8_t (unsigned integer 8-bit type), which makes sure that you can read and write without an interrupt happening in the middle.

When you say crank is not volatile is that because I've declared:

Int crank = 0;


You need to use:

volatile int crank = 0;

Without the volatile keyword, the compile has no way of knowing that the value in crank can change between lines of code. So, it will cache the value, and not check every time it needs the value, correctly assuming that the cached value will always be the latest value.

With the volatile keyword, the compiler knows not to cache the value, since it can change outside of loop()'s control.

I thought I had to give it a number, can I delete the =0?

You can, but you shouldn't. It is always better to explicitly value variables, even though the compiler will assign global variables a default value (of 0, in the case of ints).

Big thank you to you both, I'll make those changes when I get home from work.

If the crank gets any bigger than 12 before being reset by the cam pulse my engine will already be in many many pieces so it can definitely be a byte :slight_smile:

Any other pointers would be useful :slight_smile:

Any other pointers would be useful

char *pUseful = "An altogether useless pointer";

Well, maybe not.