Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

use new stream decoder API #264

Merged
merged 2 commits into from
Sep 27, 2021
Merged

use new stream decoder API #264

merged 2 commits into from
Sep 27, 2021

Conversation

japaric
Copy link
Member

@japaric japaric commented Sep 27, 2021

fixes knurling-rs/defmt#575

marking as a draft because I want to update the manual snapshot tests before merging this

also include source code so it's easier to regenerate them in the future
also reduce the size of the stack overflow snapshot
@japaric japaric marked this pull request as ready for review September 27, 2021 11:16
@jonas-schievink
Copy link
Contributor

I think it would be good to set up the unit tests to always be built from source, that makes it easier to modify them

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 27, 2021

Build succeeded:

@bors bors bot merged commit a9cec34 into main Sep 27, 2021
@bors bors bot deleted the stream-decoder branch September 27, 2021 12:09
@japaric
Copy link
Member Author

japaric commented Sep 27, 2021

I think it would be good to set up the unit tests to always be built from source, that makes it easier to modify them

generally +1 to the idea of having the source in-tree. always rebuilding the snapshot binaries does have the complication that stuff can vary between systems and break the tests e.g. file paths, binary sizes, etc. resilience against that would require sort of wildcards in the .snap files (flashing program ({digit*}.{digit*} KiB))) which I'm not sure insta supports?

}

Err(defmt_decoder::DecodeError::UnexpectedEof) => break,

Err(defmt_decoder::DecodeError::Malformed) => {
log::error!("failed to decode defmt data: {:x?}", buffer);
log::error!("failed to decode defmt data");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by the way, @Urhengulas this looked like the right place to modify to implement #249 (I think)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the pointer!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using probe-run with encoding-rzcobs results in decoding error
3 participants