-
Notifications
You must be signed in to change notification settings - Fork 9k
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
Conversation
b823374
to
1d3e059
Compare
We have builder, automatic defaults, and proper static analysis type checking now! Operates similarly to Enum's 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=[]) |
remove unused
1f2ed22
to
e54ad02
Compare
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 I made a CP.lateralTuning.which == 'torque'
Out[6]: True
CP.lateralTuning.which() == 'torque'
Out[7]: True Previously, Here's a sample from the Subaru for example:
Setting |
@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). |
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) |
There was a problem hiding this comment.
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
This reverts commit 18e11ac.
* 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
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