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

Report flashing size using probe-rs's FlashProgress system. #281

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

jonathanpallant
Copy link
Contributor

We get an Initialized object at the start, and then one message per sector erase and one message per block write.

We get an Initialized object at the start, and then one message per
sector erase and one message per block write.
let fp = flashing::FlashProgress::new(|evt| {
match evt {
// The flash layout has been built and the flashing procedure was initialized.
flashing::ProgressEvent::Initialized { flash_layout, .. } => {
Copy link
Contributor

Choose a reason for hiding this comment

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

FlashLayout is not part of probe-rs' public API, are you sure we're supposed to use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In what sense is it not public? It's documented in https://docs.rs/probe-rs/0.11.0/probe_rs/flashing/enum.ProgressEvent.html#variant.Initialized.field.flash_layout as

The layout of the flash contents as it will be used by the flash procedure. This is an exact report of what the flashing procedure will do during the flashing process.

It's also the only field in ProgressEvent::Initialized so it seems like it's OK to use it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but the type isn't documented, and you're calling methods on it which also aren't documented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll raise a ticket on probe-rs and check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spoke with @Dirbaio on Matrix, and basically it's now de-facto public API. They would like to replace it with something nicer (i.e. simpler) but that will be a breaking-change version bump, so we are safe to rely on this as long as we are on probe-rs 0.11.

@jonas-schievink
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 24, 2021

Build succeeded:

@bors bors bot merged commit 3019fc1 into main Nov 24, 2021
@bors bors bot deleted the report_upload_size_correctly branch November 24, 2021 16:53
bors bot added a commit that referenced this pull request Nov 24, 2021
283: Update snapshot tests r=jonathanpallant a=jonas-schievink

To include the changed output from #281

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants