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

HKG: Car Port for Hyundai Palisade and Kia Telluride 2023-24 (non-HDA2) #1273

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

@github-actions github-actions bot added car related to opendbc/car/ hyundai DBC signals labels Sep 24, 2024
@jyoung8607 jyoung8607 self-assigned this Sep 29, 2024
Copy link
Collaborator

@jyoung8607 jyoung8607 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one could use some preparatory refactors (in separate PRs) before going forward.

  • The CanBus class should move from hyundaicanfd to values, and then just switch to always using that for bus selection on all cars, getting rid of a lot of extra conditional branches.
  • The CAN packer functions should gain a bus argument and the bus should be passed down from carcontroller. This will allow a lot of cleanup, especially the duplicate lkas11 packer.

I can help with some of the refactors later.

@@ -113,11 +113,12 @@ def update(self, cp, cp_cam, *_) -> structs.CarState:
ret.cruiseState.standstill = False
ret.cruiseState.nonAdaptive = False
else:
ret.cruiseState.available = cp_cruise.vl["SCC11"]["MainMode_ACC"] == 1
scc_bus = "SCC12" if self.CP.flags & HyundaiFlags.CAN_CANFD_HYBRID else "SCC11"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scc_bus is a misnomer, it's really scc_msg.

@jyoung8607 jyoung8607 marked this pull request as draft September 29, 2024 16:54
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.

2 participants