There you go, I tweaked the code a bit, but I don't have the arduino at hand to try and debug it, so no guarantees here.
There you go, I tweaked the code a bit, but I don't have the arduino at hand to try and debug it, so no guarantees here.
Thanks, is really good stuff! A little difficult for me to read but that's my shortcoming, and one that is completely dwarfed by how the compiler is handling the math for the table generation. Very cool.
I'd like to incorporate most of your changes but I've made a few significant changes myself so I'll have to spend some time combining these. I'll upload 0.9.6 to github after I can test tonight but it may be several days before I can get these two merged.
Quick question - it should probably be obvious but why are you using these read functions 'pgm_read_word(&rpmReference[rpmIndex])' to read the table cells?
One of the issues I found was an oversight on my part to use the correct typing in the microseconds to degrees calculation. When I fixed it I found a notable amount of "clipping" on the higher rpm, lower voltage (upper right in this illustration) corner of the dwell table when I used dwell data for the LS1 / D580 coils.
I was starting to think this wasn't worth doing correctly because these are some fairly dumb operating ranges (< 10.49 volts, > 5600 rpm), but making the dwell table a 16 bit uint only uses another ~300 bytes of flash but I've also found a few things I can optimize and a few routines that can be eliminated so it will only cost a hundred bytes or so.Code:// for ls1 / D580 coils #define DWELL_TGT 6.1 // dwell target in milliseconds - edit to your liking #define CRANK_DWELL_TGT 3.6 // dwell target for cranking / startup #define ACCEL_COMP 0.6 // for accell comp table const uint8_t PROGMEM dwellTable[VOLT_DIVS][RPM_DIVS] = { { 1, 2, 3, 4, 5, 5, 6, 7, 14, 31, 41, 51, 62, 71, 82, 92, 102, 113, 123, 133, 144, 155, 163, 173, 185, 198, 207, 230, 250, 255, 255, 255, 255, 255 }, << last cell should be 346.9 { 1, 2, 2, 3, 4, 5, 5, 6, 12, 27, 37, 46, 55, 64, 73, 83, 92, 101, 110, 119, 129, 138, 146, 155, 165, 177, 185, 205, 224, 238, 255, 255, 255, 255 }, { 1, 1, 2, 3, 3, 4, 5, 5, 11, 25, 34, 42, 51, 59, 67, 76, 84, 93, 101, 109, 119, 127, 135, 143, 152, 163, 171, 189, 206, 219, 241, 255, 255, 255 }, { 1, 1, 2, 2, 3, 4, 4, 5, 10, 23, 31, 39, 47, 55, 63, 71, 78, 87, 94, 102, 110, 118, 125, 133, 141, 151, 159, 176, 191, 203, 224, 241, 250, 255 }, << last cell should be 265.3 { 1, 1, 2, 2, 3, 3, 4, 4, 9, 22, 29, 37, 44, 51, 59, 66, 73, 81, 88, 95, 103, 111, 117, 124, 133, 142, 149, 165, 179, 191, 210, 226, 235, 254 }, { 1, 1, 2, 2, 3, 3, 4, 4, 8, 21, 28, 36, 43, 50, 57, 64, 71, 79, 86, 92, 100, 107, 113, 120, 128, 137, 144, 159, 174, 184, 203, 219, 227, 246 }, { 1, 1, 1, 2, 2, 3, 3, 4, 7, 20, 27, 34, 41, 47, 54, 61, 67, 75, 81, 88, 95, 102, 108, 114, 122, 130, 137, 151, 165, 175, 193, 207, 215, 233 }, { 1, 1, 1, 2, 2, 3, 3, 3, 7, 19, 26, 33, 39, 45, 52, 59, 65, 72, 78, 84, 92, 98, 104, 110, 117, 126, 132, 146, 159, 169, 186, 200, 208, 225 }, { 1, 1, 1, 2, 2, 2, 3, 3, 6, 19, 25, 31, 38, 44, 50, 57, 63, 69, 75, 81, 88, 95, 100, 106, 113, 121, 127, 141, 153, 163, 179, 193, 200, 217 }, { 1, 1, 1, 2, 2, 2, 3, 3, 6, 18, 25, 31, 37, 43, 49, 55, 61, 68, 74, 80, 86, 93, 98, 104, 111, 119, 124, 138, 150, 159, 176, 189, 196, 213 } };
Yep, I have to agree it is most triumphant, since I was going to ask where the dwell variable was so I could change it for whatever coils I had available.
This will allow me to set up ion sense ignition with 1 degree accuracy. I just need to sift through the code and find the cylinder specific degree counters in it, so I can have the ion sense subsystem target 16 deg. ATDC, or at the very least, tell me hit or miss so I can narrow down the changes I will make in the timing tables. But that, is another project for another thread.
Now that does sound cool!Originally Posted by vilefly
OOI is that value the same for both banks (I presume 350 chevy) and would that change if the stroke is altered?
Mitch
'95 Z28 M6 -Just the odd mod.
'80 350 A3 C3 Corvette - recent addition.
According to my research, peak pressure must occur at 16 deg. ATDC, regardless of stroke, but I do wonder about this with different fuels and such. Might have to play with it, but I have no dynometer.....the researchers had one. Just one thing, it lost the ion sense signal above 3500 rpm. Should still be useful, since racing engines are at max timing 3000rpm and up.
Hi
can you not just scale the values in the table?
I can see the end of the 1st line reads: should be 346.9 which is the highest value ithink in your table.
If you scaled the table by 2 it would allow 512 range of units in 2's.
I don't know how that would affect the lower values.
Or if you need the fine control use values under 128 *1 and over 128 *2
Just a thought.
Mitch
'95 Z28 M6 -Just the odd mod.
'80 350 A3 C3 Corvette - recent addition.
The issue here is that I defined the table with 8 bit cell values in hopes of saving memory, which for unsigned values limits the maximum value to 255 (binary 11111111) = 256. It's not a problem - what I said at the end is I can make the table store each cell in 16 bit 'buckets' and that will fix the clipping - B1111111111111111 = 65536.
I don't want to get lost on a wild goose chase here, but if you can give me a birds-eye-view of what you're wanting to do here or point me to where you explained it / posted info that would help. If I can at least try to wrap my head around what you're needing to do electrically and logically it would help. If I have a handle on what you need to support your goal it will help so that the next round of optimizations doesn't eliminate the possibility of meeting those goals.
Incidentally, ATDC isn't currently tracked, nor is angular velocity at any resolution higher than every 90 degrees. As the code stands now, it counts down from 90 after each TDC (low res rising edge) signal. So 16 ATDC would be 74 degrees BTDC (of the next cylinder).
Well, technically all I need is an output pulse every 16*ATDC (74*BTDC) as a single point of reference for all the cylinders. I will either manually scope or "arduino-scope" the ion sense signal to find out if peak pressure occurs at the right time using a second arduino-pro to do so. Eventually, it may evolve into a closed loop system that can more accurately detect knock as well. It might be worth something to others to make that output timing changeable so that they can enact their own evil plans triggered by this pulse.
ion sense ignition waveform.jpg
ion-sense-ignition.png
The ion-sense power supply/circuitry might change to the Delphi method of capturing the 400v kickback and using it to power up the spark plug, but for now, this is a crude representation that has already worked for me in the past......but my 300v power supply was very noisy last time, and I will see about regulating it and smoothing it out.
delphi ion-sense ignition.jpg
Last edited by vilefly; 01-23-2018 at 08:30 AM. Reason: another picture
I don't know the chip architecture but I thought i'd read this was speed ctritical.The issue here is that I defined the table with 8 bit cell values in hopes of saving memory, which for unsigned values limits the maximum value to 255 (binary 11111111) = 256.
It's not a problem - what I said at the end is I can make the table store each cell in 16 bit 'buckets' and that will fix the clipping - B1111111111111111 = 65536.
Is the chip 16 / 32 bit on the data bus?
Is reading a single 8 bit and multiply noticably faster than reading a 16 bit value or not? I don't know.
Older 8 bit micros could take half a dozen cycles to calculate a 16bit no.
Mitch
'95 Z28 M6 -Just the odd mod.
'80 350 A3 C3 Corvette - recent addition.
Glad I could help :)
When you place data in program (flash) memory (PROGMEM attribute), you have to inform the code that tries to read it, that the data is in fact in flash, not in RAM. That's why we use pgm_read_xxx functions. Without this, the program would fail as the code would try to read from RAM using FLASH addressing.
Important thing I noticed and changed. There even were warnings about it during compilation. When using pointers to port addresses in "cylinders" structure, you MUST retain the volatile attribute for port addresses:
Without volatile, compiler could optimize out some output pin operations.Code:typedef struct { int cylNo; // user readable cylinder no int armed; // set to 1 = armed for dwell, set to 0 = est signal caught - prevents dwell routine from dwelling recently fired coil volatile uint8_t *portAddr; // used at a pointer to the output registers so coil drivers can be controlled directly (faster than using digital[write/read]() arduino functions) byte bitMask; // the bit mask of each output pin - see the ATmega datasheet for more info on this and timers } cylinders;
One more thing, of course I mislabelled RPM_TO_MS macro - it should be called RPM_TO_US.
Last edited by dzidaV8; 01-23-2018 at 01:33 PM.
That's easy enough - I'll add something for this to take over the tach output pin b/c I don't think it will be needed on any of the flash based cars. Or there are four other spare outputs.
Fair warning, the arduino only has one ADC that uses a multiplexer. The ADC itself is fairly slow to stabilize after you switch the multiplexer input from one analog pin to another. So if you need to look at all eight cylinders in real time it might necessitate 8 AVRs each polling one analog pin.
This is 8 bit all the way. You make a valid point. The only math that's happening to this are comparisons and additions. I'll have to look into the instruction set to see how many cycles a 16 bit add consumes. At 16 mhz each cpu tick takes 0.0625 us.
What really gets ugly is doing floating point operations, which is one of the reasons I didn't try any smoothing / interpolation on table lookups.
I noticed this immediately when I tried accessing the array directly. Oddly I didn't notice anything like this when using a static initializer on my 0.9.6 version, but it's possible I didn't test thoroughly.
Thanks for mentioning this as I probably would have overlooked it. I guess I need to look at the compiler options in arduino and make sure it's not suppressing any. Either way it's nice to have another set of eyes on my work.
This could explain why my stall detection wasn't turning the coils off.
We won't sweat the small stuff. Also that formula was off by a power of 10. This gives results similar to what the populate routine was outputting.
Sunday I found a small rounding error caused by casting the float on the right hand side of the formula into an int.Code:#define RPM_TO_US(x) (( 250000UL * 60UL / ( x ) ))
This gave more exact results.Code:dwellDegrees = uint16_t (((CRANK_DWELL_TGT + VOLT_COMP_8) * 1000) / (rpmTable[i] / 90));
What I did to test all these is to convert the populate routines to dump routines so I could see what the mcu was seeing.Code:dwellDegrees = uint16_t (((CRANK_DWELL_TGT + VOLT_COMP_8) * 1000.0) / (float (rpmTable[i] / 90)) + 0.5);
This is what I'm expecting (output of my previous populate routine).Code:dump of rpmTable[RPM_DIVS] = { 600000, 300000, 200000, 150000, 120000, 100000, 85714, 75000, 37500, 25000, 18750, 15000, 12500, 10714, 9375, 8333, 7500, 6818, 6250, 5769, 5357, 5000, 4687, 4411, 4166, 3947, 3750, 3409, 3125, 2884, 2678, 2500, 2343, 2205 }; dump of dwellTable[RPM_DIVS][VOLT_DIVS] = { { 0, 0, 0, 1, 1, 1, 2, 2, 5, 12, 16, 21, 25, 29, 33, 37, 42, 46, 50, 54, 58, 63, 67, 71, 75, 79, 84, 92, 100, 109, 117, 126, 134, 142 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } };
I can live without the extra rounding fix, but unless I'm doing something dumb iterating the array it looks like the array is only getting populated in one dimension / subscript.Code:const uint64_t rpmTable[RPM_DIVS] PROGMEM= { 600000, 300000, 199999, 150000, 119999, 99999, 85714, 75000, 37500, 24999, 18750, 14999, 12499, 10714, 9375, 8333, 7499, 6818, 6249, 5769, 5357, 4999, 4687, 4411, 4166, 3947, 3749, 3409, 3124, 2884, 2678, 2499, 2343, 2205 }; const uint8_t dwellTable[VOLT_DIVS][RPM_DIVS] PROGMEM = { { 1, 2, 3, 3, 4, 5, 6, 7, 13, 25, 33, 42, 50, 58, 66, 75, 83, 92, 100, 108, 117, 125, 133, 141, 150, 160, 168, 186, 203, 216, 238, 255, 255, 255 }, { 1, 1, 2, 3, 4, 4, 5, 6, 11, 22, 29, 36, 43, 50, 58, 65, 72, 80, 87, 94, 102, 109, 115, 122, 130, 140, 146, 162, 176, 188, 207, 222, 231, 250 }, { 1, 1, 2, 2, 3, 4, 4, 5, 10, 19, 26, 33, 39, 45, 52, 59, 65, 72, 78, 84, 92, 98, 104, 110, 117, 126, 132, 146, 159, 169, 186, 200, 208, 225 }, { 1, 1, 2, 2, 3, 3, 4, 4, 9, 18, 24, 30, 36, 41, 47, 53, 59, 65, 71, 77, 83, 89, 94, 100, 107, 114, 120, 132, 144, 153, 169, 181, 188, 204 }, { 1, 1, 1, 2, 2, 3, 3, 4, 8, 16, 22, 27, 33, 38, 43, 49, 54, 60, 65, 70, 76, 82, 87, 92, 98, 105, 110, 122, 132, 141, 155, 167, 173, 188 }, { 1, 1, 1, 2, 2, 3, 3, 4, 7, 16, 21, 26, 31, 36, 41, 47, 52, 57, 62, 67, 73, 78, 83, 88, 93, 100, 105, 116, 126, 134, 148, 159, 165, 179 }, { 1, 1, 1, 2, 2, 2, 3, 3, 6, 14, 19, 24, 29, 34, 38, 43, 48, 53, 58, 63, 68, 73, 77, 82, 87, 93, 98, 108, 118, 125, 138, 148, 154, 167 }, { 1, 1, 1, 2, 2, 2, 3, 3, 6, 14, 18, 23, 28, 32, 37, 41, 46, 51, 55, 59, 64, 69, 73, 78, 83, 88, 93, 103, 112, 119, 131, 141, 146, 158 }, { 1, 1, 1, 1, 2, 2, 2, 3, 6, 13, 17, 22, 26, 30, 35, 39, 43, 48, 52, 56, 61, 65, 69, 73, 78, 84, 88, 97, 106, 113, 124, 133, 138, 150 }, { 1, 1, 1, 1, 2, 2, 2, 3, 5, 13, 17, 21, 25, 29, 34, 38, 42, 47, 51, 55, 59, 64, 67, 71, 76, 81, 85, 95, 103, 109, 121, 130, 135, 146 } };
In case your'e wondering about the zeros in the first thee cells of the 17v subscript - I had the populate routine artificially rounding those up. But I can change the getDwell() routine to handle turning zeros into ones.
Snippet of dump routine - sorry, I know it's terrible spaghetti.
Code:// print out the dwell table to uart Serial.println("dump of dwellTable[RPM_DIVS][VOLT_DIVS] = {"); Serial.print(" { "); for (int i=0; i<RPM_DIVS; i++) { Serial.print(pgm_read_word(&dwellTable[i][0])); if (i < RPM_DIVS - 1) Serial.print(", "); } Serial.println(" },"); Serial.print(" { "); for (int i=0; i<RPM_DIVS; i++) { Serial.print(pgm_read_word(&dwellTable[i][1])); if (i < RPM_DIVS - 1) Serial.print(", "); } Serial.println(" },");
Last edited by spfautsch; 01-23-2018 at 03:48 PM.
This is solved. There were too many parens around the fill macro definition.
Replaced this:
With this:Code:#define FILL_DWELL_COLUMN(rpm) {(((CALC_DWELL(rpm, 8)), \ (CALC_DWELL(rpm, 9)), \ (CALC_DWELL(rpm, 10)), \ (CALC_DWELL(rpm, 11)), \ (CALC_DWELL(rpm, 12)), \ (CALC_DWELL(rpm, 13)), \ (CALC_DWELL(rpm, 14)), \ (CALC_DWELL(rpm, 15)), \ (CALC_DWELL(rpm, 16)), \ (CALC_DWELL(rpm, 17))))}
Thanks for this - I'd never in a million years figured out how to do this with the precompiler!Code:#define FILL_DWELL_COLUMN(rpm) { (CALC_DWELL(rpm, 8)), \ (CALC_DWELL(rpm, 9)), \ (CALC_DWELL(rpm, 10)), \ (CALC_DWELL(rpm, 11)), \ (CALC_DWELL(rpm, 12)), \ (CALC_DWELL(rpm, 13)), \ (CALC_DWELL(rpm, 14)), \ (CALC_DWELL(rpm, 15)), \ (CALC_DWELL(rpm, 16)), \ (CALC_DWELL(rpm, 17))}
I just need to convert the accel comp table to using this method and test.
Found this in arduino/preferences.txt: compiler.warning_level=none Switch none to all and lots of ugliness popped up. Very helpful.
I think I will keep the tach output to "arm" the ion sensing circuitry, and the separate 16*BTDC output to disarm/reference it.
That'll be easy enough as well. Will try to work on that before the weekend.
0.9.7 tested and uploaded to github (link in sig).
Also updated partial schematic (also in sig).
dzidav8: Thanks again for the help with the precompiler macros / table generation. I was able to take what you put together and improve on it slightly (i.e. populating any cells in the dwell table that were rounding to zero with 1). I was surprised at how difficult it was to find good info about how to handle stuff like modulo and simple decision trees this way. This seems to be one of the dark arts of code compilation. Also thanks for the hint to look into un-muting the compiler warnings in the arduino ide.
I have a laundry list of items to test and debug such as whether the system will restart after a stall condition without removing power (switching the key off). Also if increasing the size of the dwell table to 16 bits, and adding an accel compensation table has had a measurable impact on speed. I'd also like to implement a scheduling state machine for the main loop. Lastly testing at higher RPMs and in real-world driving conditions. I do have a short log I made over the past weekend where I blipped the throttle to 4400. I also had the audacity to stomp the pedal all the way down after a few glasses of liquid courage / stupidity, and I'm not sure if I hit the rev limiter (6800) or if the controller crashed (I turned the ignition off immediately). Disclaimer: I was running an experimental build of 0.9.6 at the time that could have had a serious bug in the way table data was being accessed (once again, thanks dzidav8).
When I feel confident that this project isn't going to crash and burn (i.e. after some real-word testing) I have a list of credits for the source.
But...
I also have an oil leak I'd like to fix, and about a quart of Mobil 1 to scrub off that's been caramelized onto my exhaust and floorpan. U-joints, ball joints and tie rod ends that are original to the car. You get the picture. So other than vilefly's ion sense trigger I probably won't be doing a lot of coding for a few weeks (at least that I'll be able to test). I also need to yank the rotor out of my opti, make coil brackets and remove all the opti wire routing parts. Incidentally, if anyone wants to buy a nearly complete wire retainer set they're only a year old.
Last edited by spfautsch; 01-25-2018 at 04:56 AM.
The meteorologists were talking up the forecast for today so I postponed tear-down with the anticipation of taking a short test drive.
Duct tape holding the breadboard to the windshield, I managed to get these logs.
There is a serious problem that presents as hard hesitation right around several RPM bands, starting around 1800-2200. In the controller log I noticed several lines around 2200 RPM like:
L56 is indicating that dwell started 56 degrees later than requested - possibly for a few microseconds before the EST signal was caught, if at all.Code:R2200:S33:D56:C2:V13.42:E0:L56
I found that if I accelerate through this range the problem went away. Watching my tach this seemed to be starting around 1800 so I suspect my tach reads lower than actual.
Managed to get a short pass at my "private drag strip" - peak RPM in the .eedata file was 6329 @ frame 2590 / timestamp 246.6. I wanted to push it all the way to the rev limiter but had a run-in with johnny law 15 minutes earlier when I was running late for a hair cut appoint (41 in a 35), so I thought it best not to push my luck. I felt the same type of hesitation again in a higher (4000-5000 rpm) band but I haven't looked for this in the log yet.
I'm feeling optimistic this could be an issue introduced with the most recent changes, but there's really no telling until I have time to test thoroughly which will unfortunately be several weeks out. It could however have been always present because don't recall revving slowly through the first 2200 rpm "problem" band when testing previously. Whatever the case, I'm still going to pull my rotor out out of the opti when I have the engine out. If that doesn't give me ample motivation to fix this nothing will.
Last edited by spfautsch; 01-25-2018 at 09:56 PM.
Posting Permissions
Reply With Quote
Bookmarks