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

Add option to verify written flash #353

Merged
merged 4 commits into from
Sep 28, 2022
Merged

Add option to verify written flash #353

merged 4 commits into from
Sep 28, 2022

Conversation

suyashb95
Copy link
Contributor

@suyashb95
Copy link
Contributor Author

@Urhengulas I've got an ST Link JTAG programmer and an ESP32, would love some tips on testing this out!

@Urhengulas Urhengulas marked this pull request as draft September 13, 2022 09:13
Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

@Urhengulas I've got an ST Link JTAG programmer and an ESP32, would love some tips on testing this out!

Sure!

  1. Install your local probe-run
    $ cd path/to/probe-run
    $ cargo install --path .
  2. Set up the https://github.com/knurling-rs/app-template for your board
  3. Add the --verify option to the runner in .cargo/config.toml (link)
  4. Then just run a few of the examples, e.g.:
    $ cargo rb hello
  5. Try it in combination with some of the other configuration flags, e.g. --measure-stack, --no-flash, --connect-under-reset
  6. If that behaves as expected you can mark the PR as ready for review.

PS: Please add the change to the CHANGELOG.md.

src/cli.rs Show resolved Hide resolved
@suyashb95
Copy link
Contributor Author

@Urhengulas I've got an ST Link JTAG programmer and an ESP32, would love some tips on testing this out!

Sure!

  1. Install your local probe-run
    $ cd path/to/probe-run
    $ cargo install --path .
    
  2. Set up the https://github.com/knurling-rs/app-template for your board
  3. Add the --verify option to the runner in .cargo/config.toml (link)
  4. Then just run a few of the examples, e.g.:
    $ cargo rb hello
    
  5. Try it in combination with some of the other configuration flags, e.g. --measure-stack, --no-flash, --connect-under-reset
  6. If that behaves as expected you can mark the PR as ready for review.

PS: Please add the change to the CHANGELOG.md.

@Urhengulas Turns out the ST Link debuggers don't work with the ESP32
https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/jtag-debugging/index.html#jtag-debugging-selecting-jtag-adapter

Would it be possible to test this out on your end?

Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

I tested it and it seems to verify it. At least the probe-rs logs say so.

Looking into the code I realized that the --verify option would be skipped, if used together with --no-flash. Can you therefore please change the attribute of no_flash (not verify!) to following:

/// Skip writing the application binary to flash.
-    #[structopt(long, conflicts_with = "defmt")]
+    #[structopt(
+        long,
+        conflicts_with = "disable-double-buffering",
+        conflicts_with = "verify"
+    )]
    pub no_flash: bool,

(defmt isn't needed anymore, and disable-double-buffering has no effect with --no-flash as well)

Other than that it looks good to me!

@Urhengulas Urhengulas marked this pull request as ready for review September 19, 2022 11:46
@Urhengulas Urhengulas changed the title [WIP] add option to verify written flash Add option to verify written flash Sep 19, 2022
@Urhengulas
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Sep 19, 2022
353: Add option to verify written flash r=Urhengulas a=Suyash458

 - add option to verify written flash
 - fixes #236

Co-authored-by: suyash458 <suyash.behera458@gmail.com>
@bors
Copy link
Contributor

bors bot commented Sep 19, 2022

Build failed:

Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

Can you please run cargo fmt?
Afterwards you can use bors r+ to merge.

bors d+

@bors
Copy link
Contributor

bors bot commented Sep 20, 2022

✌️ Suyash458 can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@suyashb95
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 28, 2022

Build succeeded:

@bors bors bot merged commit f5dd134 into knurling-rs:main Sep 28, 2022
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.

Add option to verify flash
2 participants