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

configure DFCO MAP threshold to vary based on RPM #485

Merged
merged 6 commits into from
Sep 13, 2024

Conversation

alrijleh
Copy link
Contributor

@alrijleh alrijleh commented Sep 6, 2024

Addresses my idea proposed in #478

I did a brief test drive in my car to verify that it works. Feel free to reject if this seems like it would contribute unnecessary bloat to the project.
Capture

@mck1117
Copy link
Collaborator

mck1117 commented Sep 7, 2024

  1. looks like a merge conflict in the TS template
  2. I'd love to see a unit test for this new behavior!

@alrijleh
Copy link
Contributor Author

alrijleh commented Sep 7, 2024

On it.
Do you think it would make sense to also add a field to set hysterics for the MAP threshold?

@alrijleh
Copy link
Contributor Author

alrijleh commented Sep 7, 2024

Also, do you think the “ MAP Mode” drop-down selector is needed? Fixed is the same as setting a flat table, but it allows the existing tune settings to work when updating to the new firmware for the first time.

@mck1117
Copy link
Collaborator

mck1117 commented Sep 13, 2024

Also, do you think the “ MAP Mode” drop-down selector is needed? Fixed is the same as setting a flat table, but it allows the existing tune settings to work when updating to the new firmware for the first time.

Yes - breaking back compat is sort of unacceptable, so the dropdown is required.

@mck1117 mck1117 merged commit 86c1c52 into FOME-Tech:master Sep 13, 2024
23 checks passed
@alrijleh alrijleh deleted the dfco-map-with-rpm branch September 16, 2024 00:08
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.

2 participants