-
Notifications
You must be signed in to change notification settings - Fork 605
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
Apply asc format since format v8.1 #1641
Conversation
@Tian-Jionglu Are you still working on the tests? I cannot merge if the tests are failing. |
can/io/asc.py
Outdated
@@ -98,14 +115,26 @@ def _extract_header(self) -> None: | |||
self.timestamps_format = timestamp_format or "absolute" | |||
continue | |||
|
|||
if version_match: | |||
version = version_match.group("version") | |||
self.version = version |
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.
Consider parsing the version using our existing dependency packaging.
Commits to parse the ASC format version using |
The ASC tests are still failing. |
When diving into test example, i found my mistake at the start. I would close this PR in coming days, and start a new PR to inherit the change to fix absolute time in ASC format. |
|
||
break | ||
if trigger_match: |
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.
I think the files can contain multiple triggers. Parsing it as part of the header is probably not sufficient.
see PR #1788 |
A solution to solve issue #1531.
Mainly include 2 changes in
ASCReader
class:start_time
,version
,date
, etc before iterate the messages. I think this is necessary;_process_fd_can_frame_2
to process CANFD message in ASCFormat since V8.1;Remaining issue:
some tests can not pass, mainly because after the change, correct
absolute
timestamp is get when iterate the messages;