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

iso-tp: more checks #1487

Merged
merged 6 commits into from Aug 26, 2023
Merged

iso-tp: more checks #1487

merged 6 commits into from Aug 26, 2023

Conversation

sshane
Copy link
Contributor

@sshane sshane commented Jun 30, 2023

asserts we can't get multiple single or first frames after calling send on IsoTpMessage. with this, we can extend timeouts in openpilot after every response, not just consecutive frames.

@gregjhogan do you think it is a good idea to make the user of the class to need to call send(b'', setup_only=True) (or similar reset function) every time if they just want to receive?

right now you can continually call iso_tp_message.recv() and continually get first or single frames, not sure if that's the intended behavior

@sshane sshane linked an issue Jun 30, 2023 that may be closed by this pull request
@gregjhogan
Copy link
Member

@gregjhogan do you think it is a good idea to make the user of the class to need to call send(b'', setup_only=True) (or similar reset function) every time if they just want to receive?

I think this is fine, sending the initial messages out of band is definitely not the normal situation and I don't have any better ideas on how to handle it.

right now you can continually call iso_tp_message.recv() and continually get first or single frames, not sure if that's the intended behavior

The asserts are good, something has gone wrong if either get hit.

This reverts commit ba4c4f1.
@sshane sshane merged commit 01db9e4 into master Aug 26, 2023
16 checks passed
@sshane sshane deleted the more-iso-tp-message-checks branch August 26, 2023 13:35
@sshane sshane added the bugfix label Aug 26, 2023
sshane added a commit that referenced this pull request Aug 29, 2023
sshane added a commit that referenced this pull request Aug 29, 2023
* Revert "iso-tp: return if updated (#1610)"

This reverts commit 0eb04fa.

* Revert "iso-tp: more sanity checks (#1487)"

This reverts commit 01db9e4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IsoTpMessage: more sanity checks
2 participants