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

fix cluster displayed set speed skipping certain mph values #33030

Closed
wants to merge 12 commits into from

Conversation

gregjhogan
Copy link
Member

@gregjhogan gregjhogan commented Jul 20, 2024

When incrementing set speed, convert to display units, increment in display units, then convert back to original units.

No clusters display set speed units with decimal places, and this ensures when you convert the stored value to any units and round to the nearest integer you always get the desired set speed value.

  • Hyundai simply requires we send the correct value in display units, and previously the higher the set speed the further off the conversion back to mph gets until it rounded down to the incorrect value. (e.g. set speeds above 85 mph)
  • Honda requires the kph value we send divided by 1.6 to round to the mph value, and previously all the conversions going on introduced error (e.g. it would never show a set speed of 65 mph)
  • Volkswagen also has issues based on this comment?
    # FIXME: follow the recent displayed-speed updates, also use mph_kmh toggle to fix display rounding problem?

@gregjhogan
Copy link
Member Author

Tested on 2016 civic, 2018 accord, and 2022 ev6 with hda2 using openpilot longitudinal and the set speed seems to always be correct now.

@sshane
Copy link
Contributor

sshane commented Jul 23, 2024

@jyoung8607 can you try this?

@gregjhogan gregjhogan force-pushed the fix-hud-mph-display branch 2 times, most recently from d9a86eb to b17b152 Compare July 30, 2024 00:35
@jyoung8607
Copy link
Collaborator

@jyoung8607 can you try this?

I will be test-driving some harness samples in the next couple days, I'll validate this at the same time.

Copy link
Contributor

This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.

@github-actions github-actions bot added the stale label Aug 14, 2024
Copy link
Contributor

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

@github-actions github-actions bot closed this Aug 17, 2024
@adeebshihadeh adeebshihadeh deleted the fix-hud-mph-display branch August 28, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants