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
New advance ignition calculations #781
Open
DeionSi
wants to merge
28
commits into
noisymime:master
Choose a base branch
from
DeionSi:newAdvanceCombined5
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… are applied once for add and multiply secondary table. Advance1 and advance2 are only retrieved/calculated if required. Performance improvements in some circumstances.
…all for some spark 2 tables modes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a combination of #702, #703 and #684 with additional code to support calculating in the old method.
Increases firmware size by 194 bytes. Performance for single table is down by ~0,25%. Performance for multiply (and probably added mode too) is up by ~5-3%. Tested on 0.4.3d board with Ardustim, seems to be working well.
The base tune must be updated to set "Corrected multiply/added mode" to "Yes".
Changes
The advance calculations are rewritten and moved into one function for table 1 and 2 (adds getAdvance, removes calculateSecondarySpark). All advance related functions are moved from speeduino.ino and secondaryTables.ino to advance.cpp (except ignition corrections).
currentStatus.advance1 is only set when the secondary table is not in use, otherwise it is 0. currentStatus.advance2 is only set when the secondary table is in use, otherwise it is 0. currentStatus.advance is always the current advance.
The correctionsIgn function has been rewritten to not overflow at 127 degrees. Also it returns whether the resulting degrees are fixed or relative. (ref1)
Fixed advance corrections will now be honored for multiply and added secondary table. Previously this value would be used afterwards in calculations removing the fixed value. (ref2)
Multiplications above 127% no longer overflow to 0. All the way up to 215% will work. (ref3)
Corrected multiply/added mode
The secondary spark table option ”Corrected multiply/added mode” refers to if the new mode (Yes) or old mode (No) is used. The only reason for old mode (No) is for the advance to not change on old tunes.
A new eeprom version is used to set ”Corrected multiply/added mode” to false to tunes utilizing the secondary spark table in added or multiply mode, otherwise it will be set to true.
Yes: Corrections are only applied once.
No: Corrections are applied twice.
This option is intended to mimic the behavior of the multiply/added mode calculations before this pull request. However the fixes listed above also affect these settings (ref 1,2,3). The validity of this mode has been confirmed by comparing outputs of the tests below between master and test-groups 16 and 17. The values that differ are all due to the fixes (ref 1,2,3).
Ignition advance tests
Test are included for the ignition advance calculations. The code comments explain the basic idea on how these tests work. The purpose of these tests is not to test individual corrections but rather the overall calculation.
All 1120 tests pass with this change.
The tests can be run on master by changing the references to configPage10.spark2correctedMultiplyAddedAdvance in test_advanceDC.cpp. The first should be commented and the second replaced with the rvalue of the commented line.