Car Port: Kia Carnival 2025 {HEV,ICE} {HDA1,HDA2} #1580

ccdunder · 2024-12-14T23:55:45Z

Checklist

added entry to CAR in selfdrive/car/*/values.py and ran selfdrive/car/docs.py to generate new docs
test route added to routes.py
route with openpilot:
- HEV: 6c0069dcd5bbb6c1/00000020--6b95507969
- ICE: 481f55600340bebb/00000095--49fb7431a0
route with stock system:
- HEV: 6c0069dcd5bbb6c1/00000021--e6c25dde2d
- ICE: f1a99071580b5733/00000007--260128fdab
car harness used (if comma doesn't sell it, put N/A):
- HDA1: Hyundai K
- HDA2: Hyundai Q

Dependencies:

HKG: ccNC messages #1671

Changes:

Adds new car KIA_CARNIVAL_HEV_4TH_GEN. This is the first year the Carnival is available as a hybrid.
Adds new car KIA_CARNIVAL_2025. While it's ostensibly "4th gen", it's very different from previous 4th gen Carnivals. In terms of static flags alone, it doesn't have RADAR_SCC, has ccNC, and gets incorrectly dynamically flagged HYBRID.

Adds support for ALT_BUTTONS with HDA2. Otherwise, the following error is logged continuously and can never becomes valid:

"filename": "opendbc_repo/opendbc/can/parser.cc",
"funcname": "UpdateValid",
"levelnum": 40,
"lineno": ' '230,
"msg": "0x1CF \'CRUISE_BUTTONS\' NOT SEEN"}',

Adds support for cruise resume from standstill with ALT_BUTTONS.
- This is needed for some other ccNC models as well.
Adds support for lane change detection with ALT_LAMPS.
- This is needed for some other ccNC models as well.
Adds support for ICE vehicles with all-zero messages on the hybrid accel address. This includes:
i. Adds a static ICE flag to prevent incorrect dynamic detection as HYBRID.
Alternative considered: We could check if not 0x100 in fingerprint[CAN.ECAN]. That passed testing on all routes, but explicit configuration is much easier to maintain (case in point).
ii. Changes safety RX check config to add only the relevant accelerator check (rather than all 3 (GAS, HYBRID, EV)).
Adds support for HDA2 long on ccNC models by populating new requisite messages (MSG_161, MSG_162).
Refactors the safety code and tests in order to support the many combinations of HKG flags. See the commit message for further details and alternatives considered.

TODO (before final review, but there's a lot I'd like to get an initial review on before dotting I's and crossing T's):

Shave down changes to hyundaicanfd.py to absolute minimum and not modify existing behavior.
Trigger rx check invalid flag (see TODO in the refactor).

Parent PR: commaai/openpilot#34662

github-actions

Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:

Convert your PR to a draft unless it's ready to review
Read the contributing docs
Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant

sunnyhaibin · 2024-12-15T15:07:09Z

This PR is a duplicate of #1558.

ccdunder · 2024-12-15T19:19:41Z

This PR is a duplicate of #1558.

Sure, the titles are similar but the content is very different. #1558 appears to be a stale draft with fingerprints only. This PR has fixes, routes, and has been verified to work.

jyoung8607 · 2024-12-15T20:39:39Z

Disclaimer: While I have the ability to merge one of these PRs, I don't set bounty policy or make bounty decisions.

@ccdunder: Welcome to opendbc and openpilot! We like having new contributors. That said, if you're aware there are other potentially duplicate PRs, it's good practice to explicitly mention them and explain why it's necessary to file a separate one. I wouldn't really call that other PR stale, conversations and testing are clearly in progress.

@sunnyhaibin: Haven't reviewed these in depth, but there are definitely differences between these PRs, starting with the existence of Panda changes in this one. Neither of these PRs are ready to merge. To get one of them merge-ready, this would be a great time to onboard and collaborate with a first-time contributor and HKG owner.

ccdunder · 2024-12-16T01:26:48Z

@sunnyhaibin my sincere apologies for my overly critical reply. I heard what sounded like "we don't want your work" and got confused and defensive. My sincere thanks @jyoung8607 for the warm welcome and cool moderation 🙏 Jason and I are collaborating in discord now.

To avoid duplicating efforts, I'm going to focus this set of PRs on the HEV specifically (i.e. drop the ICE fingerprints). There is an additional issue that only affects the ICE model and I only have an HEV to verify with anyhow.

This is necessary in order to support the 2025 Kia Carnival w/ HDA2, which uses ALT_BUTTONS. This seems exceedingly unlikely to cause a regression. If any car wasn't hitting this branch and now does, it means it was already broken (trying to use BUTTONS when they don't exist).

This is needed for the Kia Caravan '25.

TODO: Minimize changes to create_adrv_messages and make the rest conditional. Credit to royjr@ for MSG_161 & MSG_162 signal mapping.

As a newish contributor to this codebase, I should make clear up front that I haven't taken up this refactoring lightly or without good reason. I would love to avoid it but it seems necessary and less bad than all the other options. If there is another way to accomplish these goals I'm all ears. Root problem: HKG flags combine to produce combinatoric configurations. Supporting this requires tradeoffs between maintainability, convention, and quality. Alternatives considered: 1. Continue making RX & TX allowlists overly permissive. This seems antithetical to the whole point of safety flags. 2. Continue Copy-pasting nested branches with 1-line differences. It seems like this has already been done to the limit of reasonable maintainability (3-4 conditions deep). Also, to support the new conditions here would mean duplicating the code 8x. Callout: MISRA I am not a MISRA expert but AFAIU this conforms. Happy to discuss and iterate if this is incorrect. There are other options that trade readability for reduced dynamism. But at a certain point making everything static for the sake of MISRA seems to miss the forest for the trees. LMK what you think. I'd love to hear if there's a better way to solve this.

This was removed when safety was moved from panda to opendbc. This is needed in order for memcpy to be available for safety tests as it is in production.

ccdunder · 2025-02-21T22:29:52Z

Plan is to land this as part of #1625

adeebshihadeh

This is quite big. Can we break this up at all into smaller self-contained PRs? At least break out the two different car models

adeebshihadeh · 2024-12-18T22:14:13Z

opendbc/car/hyundai/carstate.py

+    left_blinker_sig, right_blinker_sig = (
+      ("LEFT_LAMP_ALT", "RIGHT_LAMP_ALT")
+      if cp.vl['BLINKERS']['USE_ALT_LAMP']
+      else ("LEFT_LAMP", "RIGHT_LAMP")
+    )


I think the old way this was written is more readable

adeebshihadeh · 2025-02-22T04:34:23Z

opendbc/safety/board/libc.h

adeebshihadeh · 2025-02-22T04:40:27Z

opendbc/car/hyundai/interface.py

    if candidate == CAR.KIA_OPTIMA_G4_FL:
      ret.steerActuatorDelay = 0.2
+    elif candidate == CAR.KIA_CARNIVAL_HEV_4TH_GEN:
+      ret.steerActuatorDelay = 0.35


this is incredibly high. are you sure? how'd you determine this?

adeebshihadeh · 2025-02-22T05:56:18Z

opendbc/dbc/hyundai_canfd.dbc

wanna split this out? we can just merge this to reduce the diff a bit

adeebshihadeh · 2025-02-22T06:03:05Z

opendbc/safety/tests/libsafety/safety_helpers.h

+      printf("i %d seen %d lagging %d valid checksum %d wrong counters %d valid quality flag %d\n",
+      i, addr.status.msg_seen, addr.status.lagging, addr.status.valid_checksum, addr.status.wrong_counters, addr.status.valid_quality_flag);
+      for (int j = 0; j < MAX_ADDR_CHECK_MSGS; j++) {
+        const CanMsgCheck msg = addr.msg[j];
+        if (msg.addr == 0) {
+          break;
+        }
+        printf("i %d j %d bus %d addr 0x%x len %d\n",
+        i, j, msg.bus, msg.addr, msg.len);
+      }


if you split this out and put it behind an environment variable (like LOGPRINT=debug), we can merge this

FW for 2025 HDA2 (KOR)

github-actions · 2025-06-03T02:13:16Z

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

github-actions · 2025-06-10T02:13:57Z

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

github-actions · 2025-09-22T02:10:43Z

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

github-actions · 2025-10-01T02:12:11Z

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

github-actions bot added car related to opendbc/car/ hyundai labels Dec 14, 2024

github-actions bot reviewed Dec 14, 2024

View reviewed changes

This was referenced Dec 14, 2024

HKG: HDA2 ALT_BUTTON support commaai/panda#2109

Closed

Car Port: Kia Carnival HEV 2025 (HDA2) commaai/openpilot#34246

Closed

ccdunder force-pushed the kia-carnival-25 branch from 63f9308 to a89a450 Compare December 15, 2024 05:05

ccdunder changed the title ~~Car Port: Kia Carnival 2025 HDA2 (Hybrid & ICE)~~ Car Port: Kia Carnival HEV 2025 (HDA2) Dec 16, 2024

ccdunder force-pushed the kia-carnival-25 branch 4 times, most recently from 6b73dc8 to 300e557 Compare December 17, 2024 18:08

ccdunder mentioned this pull request Dec 19, 2024

HKG: ALT_LAMPS support #1593

Draft

ccdunder marked this pull request as draft December 19, 2024 21:00

ccdunder added 6 commits February 20, 2025 20:35

Add Kia Carnival Hybrid 2025.

5073f77

Add route for Kia Carnival Hybrid 2025.

7ad0643

Fix cruise resume from standstill when ALT_BUTTONS are used.

98227ec

This is needed for the Kia Caravan '25.

Fix lane change detection when ALT_LAMPS are used.

c1c7a72

This is needed for the Kia Caravan '25.

Add fingerprints for Kia Carnival '25 ICE.

763459b

ccdunder force-pushed the kia-carnival-25 branch from 300e557 to d7a1032 Compare February 21, 2025 00:21

github-actions bot added the DBC signals label Feb 21, 2025

ccdunder force-pushed the kia-carnival-25 branch 3 times, most recently from 1523991 to 398e585 Compare February 21, 2025 04:12

ccdunder changed the title ~~Car Port: Kia Carnival HEV 2025 (HDA2)~~ Car Port: Kia Carnival 2025 Feb 21, 2025

ccdunder changed the title ~~Car Port: Kia Carnival 2025~~ Car Port: Kia Carnival 2025 {HEV,ICE} {HDA1,HDA2} Feb 21, 2025

ccdunder added 2 commits February 21, 2025 21:39

tweak carnival torque based on observed values

105756d

Tune actuator lag on KIA_CARNIVAL_HEV_4TH_GEN.

7927cc1

ccdunder force-pushed the kia-carnival-25 branch from 9c093d5 to e6d3a5b Compare February 21, 2025 21:56

ccdunder added 8 commits February 21, 2025 22:10

Fix Kia Carnival HEV HDA1 can error.

87b1642

Make long work.

8c26468

TODO: Minimize changes to create_adrv_messages and make the rest conditional. Credit to royjr@ for MSG_161 & MSG_162 signal mapping.

cleanup to make merge-ready

4b68fc5

Add ICE route.

36e7929

Reorder HEV & ICE

8297fb4

gen docs

84987a5

Bring libc.h over from panda to safety.

d950f67

This was removed when safety was moved from panda to opendbc. This is needed in order for memcpy to be available for safety tests as it is in production.

ccdunder force-pushed the kia-carnival-25 branch from e6d3a5b to d950f67 Compare February 21, 2025 22:13

adeebshihadeh reviewed Feb 22, 2025

View reviewed changes

opendbc/dbc/hyundai_canfd.dbc

Copy link

Contributor

adeebshihadeh Feb 22, 2025

Choose a reason for hiding this comment

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

wanna split this out? we can just merge this to reduce the diff a bit

adeebshihadeh reviewed Feb 22, 2025

View reviewed changes

sunnyhaibin mentioned this pull request Mar 2, 2025

Hyundai CAN FD: Accurate hybrid powertrain detection #1891

Merged

sunnyhaibin and others added 2 commits March 15, 2025 10:57

FW for 2025 HDA2 (KOR)

4f4e8db

Merge pull request #1 from sunnyhaibin/patch-1

16669d8

FW for 2025 HDA2 (KOR)

github-actions bot added the stale label Jun 3, 2025

github-actions bot closed this Jun 10, 2025

adeebshihadeh reopened this Jun 10, 2025

github-actions bot removed the stale label Jun 11, 2025

github-actions bot added the stale label Sep 22, 2025

github-actions bot closed this Oct 1, 2025

nussdk mentioned this pull request Nov 10, 2025

add 2026 kia carnival SX fingerprints #2885

Closed

Car Port: Kia Carnival 2025 {HEV,ICE} {HDA1,HDA2} #1580

Car Port: Kia Carnival 2025 {HEV,ICE} {HDA1,HDA2} #1580

Uh oh!

Conversation

ccdunder commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

sunnyhaibin commented Dec 15, 2024

Uh oh!

ccdunder commented Dec 15, 2024

Uh oh!

jyoung8607 commented Dec 15, 2024

Uh oh!

ccdunder commented Dec 16, 2024

Uh oh!

ccdunder commented Feb 21, 2025

Uh oh!

adeebshihadeh left a comment

Choose a reason for hiding this comment

Uh oh!

adeebshihadeh Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

adeebshihadeh Feb 22, 2025

Choose a reason for hiding this comment

Uh oh!

adeebshihadeh Feb 22, 2025

Choose a reason for hiding this comment

Uh oh!

adeebshihadeh Feb 22, 2025

Choose a reason for hiding this comment

Uh oh!

adeebshihadeh Feb 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 3, 2025

Uh oh!

github-actions bot commented Jun 10, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ccdunder commented Dec 14, 2024 •

edited

Loading