Ignition schedulers update #578
Conversation
Ignition timing changed to be calculated directly from ingnition end angle (previous art was endAngle -> startAngle -> endtime)
Other channels left as is at the moment.
Initial work on refactoring ignition schedules
Reduced code repetition.
Updated interrupt functions on all channels to be compatible with the previous changes to the interrupt schedulers. Reduced code repetition in ignition interrupt functions.
There was a problem hiding this comment.
Downloaded this version and I'm seeing issues with spark 1. Spark 2 to 4 look good. Run with ardustim generating data on 36-1 with single tooth cam. Spark 1 drops out for a few spark events, when it restarts it usually has a very large spark for the first event. Will post image of logic level output on slack of what i can see.
Further testing has show the issue moves between sparks depending on the engine speed and as it increases gets worse. Low speed (cranking) has poor timing (individual spark events have variable gaps between them). Details posted on Slack with images.
Only allow the overdwell protection in the timers.ino to run when it is enabled and with the new ignition mode(to increase performance). For old ignition mode incorporate the overdwell protection to the schedulers. Plus some comments tweaked in scheduler.ino
Rearranged the angleToTime in the setIgnitionShedule() only calculated when it is really needed. Now it is not calculated when corresponding schedule is already running, this saves some time and may help on atmega platvorm since those calculations include division.
Time window between staging and impulse starting interrupts was too small for the Atmega to keep up. Just add a little extra time. Plus make setIgnitionxCompare functions use COMPARE_TYPE, for possible future multi platvorm differences.
Adds extra parentheses around the macro argument.
|
Now the issues that was initially with running on Amega2560 platvorm are fixed. |
Put default envs back to the megaatmega2560. Should not have been changed in the first place, would be cleaner PR.
TBAMax
changed the title
Change some more variables type to COMPARE_TYPE. Currently all timer comparators are used with 16bit. So the COMPARE_TYPE is currently uint16_t. Anyway when possibly changing the COMPARE_TYPE in the future, great care must be taken to evaluate all effects. This just adds a little bit closer to make smooth changing of COMPARE_TYPE possible.
Organize variables to use COMPARE_TYPE definition in some more places. With intention to make schedulers compatible with different width timers comparators.
The inline specifier is only a compiler hint, and the compiler is free to completely ignore it. Compilers that we are using today actually are supposed to do the inlining automatically without this specifier too, but for good measure still add it.
For cleaner and shorter code, remove the multiple global variables and place the same functionality inside the existing struct. The ignitionSchedule struct now has new member. Basically done with intention to reduce arguments count of setIgnitionSchedule function after that.
reduce setIgnitionSchedule function arguments count by 1. channelIgnDegrees are now taken directly from corresponding ignitionSchedule struct
@VitorBoss sorry, what exactly do you mean also need to be COMPARE_TYPE? |
Convert small "getIgnCoutenter()" functions also to the COMPARE_TYPE.
|
Currently seems this has broken fixed cranking override on some decoders. Under investigation. |
Fix Fixed Cranking Override for all decoders. Previously there was excessice dwell and spark produced at trigger edge. Changed that so that now coil charging is started at trigger edge and normal dwell is used. This retards the timing slightly for starting-- should be a good thing. (For most decoders that use fixed timing for starting the trigger edge should be at 5degrees BTDC). Prevent per tooth ignition from running with fixed cranking ignition override Add decoderHasFixedCrankingTiming = true; on some decoders that had that missing.
|
fixed cranking override also now repaired. |
Do not allow per tooth ignition mode to run during the fixedcrankingtiming on all triggers that support both. Add decoderHasFixedCrankingTiming = true; on some decoders that had that missing.
Remove unused variables: int ignition1StartAngle int ignition2StartAngle int ignition3StartAngle int ignition4StartAngle int ignition5StartAngle int ignition6StartAngle int ignition7StartAngle int ignition8StartAngle All those angles are not used any more. All things are based on ignitionEndAngles(actual spark angle). affected files: speeduino.ino; globals.h
Cleaner code. Remove dwell angle from calculateIgnitionAngle() function arguments, since this is actually unused. Dwell is based directy on dwell time.
Remove "fixedCrankingOverride" variable because it it not used any more. It was previously used to regulate dwell during fixed cranking timing, but now the dwell is regulated in the schedulers.
Those function declarations were still not updated. Functions themselves were already updated earlier.
Use functions to set the ignition compare values with the perToothTiming. This fixes the some kind of type conversion issue that caused occassional very early spark events previously.
Add cast to COMPARE_TYPE also for idle. Similar issue of that cast missing was causing issues with the ignition timing. This is needed when 32bit timer is used in the 16bit overflow mode. So apply it here too.
Combine setIgnitionSchedule functions into a new generic function. This reduces code duplication in this area quite a lot. This is a continuation/alternative from noisymime#686 and noisymime#578. Only taking some(better) parts from the both and keeping changes in the manageable level. Only the code parts that are in the main loop are modified, interrupts are left as is at the moment, because do not want to add extra function calls to the interrupts. And also this would be to probably too much for single PR. It was discovered that ignitionSchedule[i].member calls seem to be slowed than ignitionSchedule->member calls so this uses the latter variant. Also this seems to be the safer option.
Move IgnitionStartFunction and IgnitionEndFunction to be with the ignition schedule struct. Reduced code repetition. 0.1% RAM saving, 0.1% Flash saving observed on black_F407VE
|
At the moment this is still a draft because some bugs were discovered and need to be fixed here. All the fixes are already done in the https://github.com/TBAMax/speeduino/tree/mainbranch but need to be brought over. Can not do this by simple cherry-pick but must manually copy paste over because in the mainbranch there are also changes to the fuel chedulers. Thinking of maybe doing a joint PR for new fuel and ignition both, but unsure if there is any interest at the moment. |
|
@mike501 left a good comments on the slack about how to test the ignition jitter with the Sigrock PulseView. Too bad it is gone from there by now. Need to do some tests and figure that out again. |
|
Replaced by #804 |
|
The comments may be gone, but I am not! Give me a shout on Discord or Slack and I'll figure out what I told you before. It will have been along the lines of set up a 'timing' decoder for each ignition channel, remove average, select 1 edge (usually falling) and delta. You can then see how much the individual ignition event moves on that specific channel by looking at the delta. To improve my debugging I also added some code to turn on a single specific pin every time any ignition event occurred & set a flag high. I then added code in the main loop to turn off the pin if the flag was high and reset the flag. This allowed me to monitor every ignition event compared to each ignoring the specific ignition channel via another 'timing' decoder. |
Made ignition timing be calculated in the schedulers from the IgnitionEndAngle. Should be more accurate because IgnitionEndAngle is directly the angle that the spark is suppose to be fired at.
Improvements to the dwell timing:
-Dwell time is directly applied to timers now without intermediate conversion to the dwell angle domain and back.
-Dwell time is controlled entirely in the interrupts, so even when main loop completely locks up, it do not cause changes to the dwell time.
Also reduced code repetition and made all ignition channels to use the same function. To get them all channels accept the changes with less effort.
Update: Tested, and works with per tooth ignition timing as well.