Skip to content
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

Deduplicate ignition scheduler code #686

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

DeionSi
Copy link
Contributor

@DeionSi DeionSi commented Oct 17, 2021

  • Refactor IgnitionSchedule1-8 variables into IgnitionSchedule[] array.
  • Add compare, counter, timerEnable, timerDisable, startAngle, endAngle, degreesAfterTDC1 (previously channelXIgnDegrees) and channel to Schedule struct.
  • The changes to the Schedule struct has the benefit of deduplicating multiple functions/code segments:
    • refreshIgnitionTiming
    • setIgnitionSchedule
    • ignitionScheduleInterrupt
    • checkIgnitionSchedule
    • calculateIgnitionAngle

This is a direct continuation of #648 and that pull request is used as base. It is also related to #578.

This change more or less only refactors code. The logic changes are that the scheduler callbacks are only set once in initialiseAll and a small optimization into calculateIgnitionAngleWithSpecificEndAngle for rotary.

ignitionSchedule[i].member calls seem to be slowed than ignitionSchedule->member calls. This is why most functions pass a pointer to the array member rather than the number of the ignition channel.

Tested on mega2560. Loops/sec reduced by 0,5% (from Tunerstudio). Size reduced by 1,8% and memory by 0,72% (PlatformIO compile info). I've done some basic testing on the bench and the ignition events appear to happen at the correct time (scope tested).

There are other areas where code can be deduplicated as a continuation of this change.

@DeionSi
Copy link
Contributor Author

DeionSi commented Nov 7, 2021

Fixed the decoder and scheduler tests. Both are successful.

TBAMax added a commit to TBAMax/speeduino that referenced this pull request Nov 27, 2021
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants