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

Move car.capnp to opendbc #33646

Closed
2 tasks
sshane opened this issue Sep 24, 2024 · 1 comment
Closed
2 tasks

Move car.capnp to opendbc #33646

sshane opened this issue Sep 24, 2024 · 1 comment

Comments

@sshane
Copy link
Contributor

sshane commented Sep 24, 2024

@deanlee interested in this? We want to replace the dataclasses with car.capnp again, but it can live in opendbc. The duplication of the structs is starting to get annoying and is error-prone if one place is forgotten to be updated.

There are some issues we need to resolve:

  • OnroadEvent should be in openpilot but is used in carState.events (deprecated field now). We can make a copy of that and call the struct OnroadEventDEPRECATED.
  • log.capnp uses some structs from car.capnp such as radar errors, audble alerts, safety model. We can duplicate most, except the safety model shouldn't need to be edited in both places. Maybe we should turn it into an int from pandaStates? Or can we just include the opendbc struct from cereal and call it a day?
    Edit: I don't think so, you will get a duplicate ID error when you import both car.capnp in opendbc and the duplicate included one in log.capnp (from a symlink)
@deanlee
Copy link
Contributor

deanlee commented Sep 29, 2024

commaai/opendbc#1288

The approach of this PR is to move car.capnp to opendbc/car while keeping all other capnp files unchanged. In opendbc, the following code is used to avoid the duplicate ID error:

try:
  from cereal import car
except ImportError:
  capnp.remove_import_hook()
  car = capnp.load(os.path.join(OPENDBC_CAR_PATH, "car.capnp"))

This ensures that car.capnp can be imported properly without conflicts

@sshane sshane closed this as completed Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants