This repository has been archived by the owner on Dec 15, 2021. It is now read-only.
add support for all ITM packet types and replace Decoder with Stream parser #24
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…parser as this is a breaking change and requires a minor version bump, this commit also: - sets MSRV to 1.31.0 - changes the crate edition to 2018 - removes `itmdump`
6 tasks
|
Sorry for the late reply! Overall I like this update, I have had strange things happen with ITM before but never looked into it. With this change we should be getting closer to production ready tools in this area, great job! |
korken89
approved these changes
Feb 5, 2019
|
@rust-embedded/tools any thoughts about this PR? |
Disasm
added
T-tools
S-waiting-on-team
|
I don't have time to test ATM but it looks good. |
therealprof
approved these changes
Mar 6, 2019
|
@rust-embedded/tools time to move this forward? |
|
bors r+ |
bors bot
added a commit
that referenced
this pull request
Aug 10, 2019
24: add support for all ITM packet types and replace Decoder with Stream parser r=therealprof a=japaric as this is a breaking change and requires a minor version bump, this commit also: - sets MSRV to 1.31.0 - changes the crate edition to 2018 - removes `itmdump` (more info below) --- The rationale for dropping the existing `Decoder` API is that it doesn't handle well streams of data that contain errors, like missing bytes (occurs in ~0.2% of packets according to my unscientific measurements). Namely, `Decoder` can't backtrack the stream because it has no internal buffer; meaning that once a byte is lost it will continue to parse packets with an offset of one (i.e. the first byte of the payload will be treated as the packet header) leading to a cascade of errors. The `Stream` API has an internal buffer so when it encounters a packet error due to a missing byte it can backtrack the stream and retry parsing from the most likely place. Also, thanks to the internal buffer `Stream` greatly reduces the number of read syscalls -- the `Decoder` API was doing 1-byte read syscalls in some places. (For historic purposes, I have pushed [the branch] where I was adding full parsing support to the `Decoder` API) [the branch]: https://github.com/rust-embedded/itm/tree/decoder With this new API and support for all the different ITM packets I have created a set of tools ([`itm-tools`]) for analyzing ITM traces. One of the tools, `port-demux`, is a more general version of `itmdump` and should be preferred over it. The other reason for removing `itmdump` and not providing tools in this crate is that it makes version management easier: breaking changes in the library don't necessarily mean breaking changes in the tool CLI. [`itm-tools`]: https://github.com/japaric/itm-tools r? @rust-embedded/tools cc @rust-embedded/cortex-m Co-authored-by: Jorge Aparicio <jorge@japaric.io>
Build succeeded |
10 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
as this is a breaking change and requires a minor version bump, this commit
also:
itmdump(more info below)The rationale for dropping the existing
DecoderAPI is that it doesn't handlewell streams of data that contain errors, like missing bytes (occurs in ~0.2% of
packets according to my unscientific measurements). Namely,
Decodercan'tbacktrack the stream because it has no internal buffer; meaning that once a byte
is lost it will continue to parse packets with an offset of one (i.e. the first
byte of the payload will be treated as the packet header) leading to a cascade
of errors.
The
StreamAPI has an internal buffer so when it encounters a packet error dueto a missing byte it can backtrack the stream and retry parsing from the most
likely place. Also, thanks to the internal buffer
Streamgreatly reduces thenumber of read syscalls -- the
DecoderAPI was doing 1-byte read syscalls insome places.
(For historic purposes, I have pushed the branch where I was adding full
parsing support to the
DecoderAPI)With this new API and support for all the different ITM packets I have created a
set of tools (
itm-tools) for analyzing ITM traces. One of the tools,port-demux, is a more general version ofitmdumpand should be preferredover it. The other reason for removing
itmdumpand not providing tools in thiscrate is that it makes version management easier: breaking changes in the
library don't necessarily mean breaking changes in the tool CLI.
r? @rust-embedded/tools
cc @rust-embedded/cortex-m