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

Reapply "update to new opendbc API (#32009)" #33151

Merged
merged 1 commit into from
Jul 31, 2024
Merged

Reapply "update to new opendbc API (#32009)" #33151

merged 1 commit into from
Jul 31, 2024

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Jul 31, 2024

There is an exception in the fingerprinting code, commaai/panda#1329 probably would have caught this but requires a lot of work to make it mergable. Will fully run IsoTpParallelQuery on real CAN data (or faked) to ensure this never happens again.

I'm not sure if it would have caught it, but we used to also run our fingerprinting tests un-mocked but did that to speed them up

@sshane
Copy link
Contributor Author

sshane commented Jul 31, 2024

I couldn't get the pytest caplogger working to catch the cloudlogs, but mocking it worked:

self = <panda.python.uds.CanClient object at 0x7fa261229490>, drain = False

    def _recv_buffer(self, drain: bool = False) -> None:
      while True:
        msgs = self.rx()
        if drain:
          if self.debug:
            print(f"CAN-RX: drain - {len(msgs)}")
          self.rx_buff.clear()
        else:
>         for rx_addr, _, rx_data, rx_bus in msgs or []:
E         ValueError: not enough values to unpack (expected 4, got 3)

../../../panda/python/uds.py:342: ValueError

@sshane sshane force-pushed the opendbc-new-api branch from 54cce49 to 9efca46 Compare July 31, 2024 22:21
@sshane sshane marked this pull request as ready for review July 31, 2024 22:21
@sshane sshane merged commit 7c11234 into master Jul 31, 2024
18 checks passed
@sshane sshane deleted the opendbc-new-api branch July 31, 2024 22:37
Edison-CBS pushed a commit to Edison-CBS/openpilot that referenced this pull request Sep 15, 2024
This reverts commit f2f01e3afaeaa267af61c8d7ab918da04d3411f2.
old-commit-hash: 7c11234
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant