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

selfdrive/car: ban cereal and capnp #33208

Merged
merged 142 commits into from
Aug 16, 2024
Merged

selfdrive/car: ban cereal and capnp #33208

merged 142 commits into from
Aug 16, 2024

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Aug 6, 2024

instead of #33197

while thinking about it, I realized we might not need to switch to a full blown class in the first iteration. will try to reduce touching things, and just convert log.CanData to a custom dataclass with generic send/receive functions passed into fingerprinting/car interface code

@sshane sshane changed the title ban cereal and msgq selfdriv/car: ban cereal and msgq Aug 6, 2024
@sshane sshane added refactor car vehicle-specific labels Aug 6, 2024
@sshane sshane changed the title selfdriv/car: ban cereal and msgq selfdrive/car: ban cereal and msgq Aug 6, 2024
@commaai commaai deleted a comment from github-actions bot Aug 9, 2024
@sshane
Copy link
Contributor Author

sshane commented Aug 9, 2024

We have builder, automatic defaults, and proper static analysis type checking now! Operates similarly to Enum's auto() object.

This plus the custom enum helper is only +37 lines for capnp struct behavior but with static linting, I think that's worth it.

@auto_dataclass
class TestClass:
  a: int = auto_field()
  b: float = auto_field()
  c: str = auto_field()
  d: bytes = auto_field()
  e: list[int] = auto_field()
  f: tuple[int] = auto_field()
  g: set[int] = auto_field()
  h: dict[str, int] = auto_field()
  i: bool = auto_field()
  ecu: CarParams.Ecu = auto_field()
  carFw: CarParams.CarFw = auto_field()
  carFwList: list[CarParams.CarFw] = auto_field()

TestClass()
Out[3]: TestClass(a=0, b=0.0, c='', d=b'', e=[], f=(), g=set(), h={}, i=False, ecu=<Ecu.eps: 'eps'>, carFw=CarParams.CarFw(ecu=<Ecu.unknown: 'unknown'>, fwVersion=b'', address=0, subAddress=0, responseAddress=0, request=[], brand='', bus=0, logging=False, obdMultiplexing=False), carFwList=[])

@sshane
Copy link
Contributor Author

sshane commented Aug 16, 2024

process replay diffs:

A lot are minute differences due to switching from float32 precision to float64 using Python's float type. Nissan calculates mean of floats to get steering pressed for example. Toyota rate limit being different was because its CarControllerParams didn't call which as a function, and the <torque which-enum> defines a __str__/__eq__

I made a Which StrEnum that supports __call__ which is more robust:

CP.lateralTuning.which == 'torque'
Out[6]: True
CP.lateralTuning.which() == 'torque'
Out[7]: True

Previously, which would return a method.


Here's a sample from the Subaru for example:

	('change', 'carState.steeringRateDeg', (-1.0849952697753906, -1.0850000381469727))
	('change', 'carState.aEgo', (0.37458106875419617, 0.37458622455596924))

Setting NUMPY_TOLERANCE to 7e-6 2e-5 resolves all diffs (Subaru's angle rate calculation is the highest diff).

@sshane
Copy link
Contributor Author

sshane commented Aug 16, 2024

@adeebshihadeh I've reviewed the car interface code, can you review card.py and structs.py? For the car tests touched here, I am going to leave them in openpilot for now, but I can see splitting them up like I did for the docs test in a follow up PR (not going to do here since they're all so huge and require some more thought).

Comment on lines +64 to +88
def is_dataclass(obj):
"""Similar to dataclasses.is_dataclass without instance type check checking"""
return hasattr(obj, _FIELDS)


def asdictref(obj) -> dict[str, Any]:
"""
Similar to dataclasses.asdict without recursive type checking and copy.deepcopy
Note that the resulting dict will contain references to the original struct as a result
"""
if not is_dataclass(obj):
raise TypeError("asdictref() should be called on dataclass instances")

def _asdictref_inner(obj) -> dict[str, Any] | Any:
if is_dataclass(obj):
ret = {}
for field in getattr(obj, _FIELDS): # similar to dataclasses.fields()
ret[field] = _asdictref_inner(getattr(obj, field))
return ret
elif isinstance(obj, (tuple, list)):
return type(obj)(_asdictref_inner(v) for v in obj)
else:
return obj

return _asdictref_inner(obj)
Copy link
Contributor Author

@sshane sshane Aug 16, 2024

Choose a reason for hiding this comment

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

We unfortunately have to rewrite these to take advantage of referencing vs. deepcopying and avoiding two additional type()/isinstance() on every single field recursively, which take around ~7-8% total vs 3-4% with this

@sshane sshane marked this pull request as ready for review August 16, 2024 22:12
@sshane sshane merged commit 6a15c42 into master Aug 16, 2024
17 of 18 checks passed
@sshane sshane deleted the no-capnp-cars-simple branch August 16, 2024 22:13
Edison-CBS pushed a commit to Edison-CBS/openpilot that referenced this pull request Sep 15, 2024
* ban cereal and msgq

* common too

* do toyota/values.py

* do all fingerprints

* example without builder

* this still works, but no type checking anymore

* stash

* wtf, how does this work

* okay actually not bad

* safe

* epic!

* stash data_structures.py

* some clean up

* hell yeah

* clean up old file

* add to delete

* delete

This reverts commit 90239b7.

* switch more CarParams stuff over

remove unused

* fix car tests by removing cereal! mypy forgets about dataclass if we wrap it :(

* fix this too

* fix this too

* remove more cereal and add some good hyundai tests

* bunch more typing

* override default with 20hz radar

* temp capnp converter helper

* more lateralTuning

* small union replicator is better than what i was trying, and fixes mypy dynamic typing issues

* can keep all this the same now!

* type ret: CarParams, add more missing structs, revert lateralTuning changes (smaller diff!)

* revert more

* get first enum automatically, but ofc mypy doesn't pick up the new metaclass so can't use :(

would have been `CarParams.NetworkLocation()`

* Revert "get first enum automatically, but ofc mypy doesn't pick up the new metaclass so can't use :("

This reverts commit bb28b22.

* remove cereal from car_helpers (TODO: caching)

* remove a bunch of temp lines

* use dataclass_transform!

* remove some car.CarParams from the interfaces

* remove rest of car.CarParams from the interfaces

* same which() API

* sort

* from cereal/cache from fingerprinting!

* more typing

* dataclass to capnp helper for CarParams, cached it since it's kinda slow

* (partial) fix process replay fingerprintig for new API

* latcontrollers take capnp

* forgot this

* fix test_models

* fix unit tests

* not here

* VehicleModel and controller still takes capnp CP since they get it from Params()

* fix modeld test

* more fix

* need to namespace to structs, since CarState is both class and struct

* this was never in the base class?!

* clean that up again

* fix import error

fix import error

* cmts and more structs

* remove some more cereal from toyota + convert CarState to capnp

* bruh this was wrong

* replace more cereal

* EventName is one of the last things...

* replace a bunch more cereal.car

* missing imports

* more

* can fix this typing now

* proper toyota+others CS typing!

* mypy can detect return type of CS.update() now

* fix redeclaration of cruise_buttons type

* mypy is only complaining about events now

* temp fix

* add carControl struct

* replace CarControl

i hope there's no circular imports in hyundai's CC

* fine now

* lol this was wrong too

* fix crash

* include my failed attempts at recursively converting to dataclass (doesn't implicitly convert types/recursively :( )

but attrs does, maybe will switch in the future

* clean up

* try out attr.s for its converter (doesn't work recursively yet, but interesting!)

* Revert "try out attr.s for its converter (doesn't work recursively yet, but interesting!)"

This reverts commit ff2434f.

* test processes doesn't fail anymore (on toyota)!

* fix honda crash

* stash

* Revert "stash"

This reverts commit c1762af.

* remove a bunch more cereal!

* LET'S GOOO

* fix these tests

* and these

* and that

* stash, something is wrong with hyundai enable

* Revert "stash, something is wrong with hyundai enable"

This reverts commit 39cf327.

* forgot these

* remove cereal from fw_versions

* Revert "remove cereal from fw_versions"

This reverts commit 232b37c.

* remove rest of the cereal exceptions!

* fix that

* add typing to radard since I didn't realize RI.update() switched from cereal to structs

* and here too!

* add TODO for slots

* needed CS to be capnp, fix comparisons, and type hint car_specific so it's easier to catch type issues (capnp isn't detected by mypy :( )

* remove the struct converter

* save ~4-5% CPU at 100hz, we don't modify after so no need to deepcopy

btw pickle.loads(pickle.dumps()) is faster by ~1% CPU

* deepcopy -> copy: we can technically make a reference, but copy is almost free and less error-prone

saves ~1% CPU

* add non-copying asdict function

* should save ~3% CPU (still 4% above baseline)

* fix that, no dict support

* ~27% decrease in time for 20k iterations on 3X (3.37857 -> 2.4821s)

* give a better name

* fix

* dont support none, capitalize

* sheesh, this called type() on every field

* remove CS.events, clean up

* bump card %

* this was a bug on master!

* add a which enum

* default to pid

* revert

* update refs

* not needed, but consistent

* just Ecu

* don't need to do this in this pr

* clean up

* no cast

* consistent typing

* rm

* fix

* can do this if we're desperate for the last few %

* Revert "can do this if we're desperate for the last few %"

This reverts commit 18e11ac.

* type this

* don't need to convert carControl

* i guess don't support set either

* fix CP type hint

* simplify that
old-commit-hash: 6a15c42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
car vehicle-specific refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants